fix(find): dispatch q-ref gestures at the target rect + prefer the interactive node on ambiguous labels#26
Open
anilcancakir wants to merge 9 commits into
Open
fix(find): dispatch q-ref gestures at the target rect + prefer the interactive node on ambiguous labels#26anilcancakir wants to merge 9 commits into
anilcancakir wants to merge 9 commits into
Conversation
A q-ref (find/observe handle) anchored its RefEntry at the root element, so pointer dispatch (dispatchRectOf = _liveRectOf(entry.element)) re-resolved to the whole viewport and every q-ref tap fired at the viewport CENTER instead of the target. On a centered widget the two coincided (so existing tests passed); on any off-center control (a submit button, a checkbox, a sidebar item) the tap missed silently. Resolve the Element whose RenderBox contributes the node (debugSemantics identity, matching ext_observe) and anchor the entry there, so dispatch, the stable-rect gate, and EditableText focus all operate on the real target. Derive the rect from that element's localToGlobal (LOGICAL px); the _globalRectFromSemantics fallback now divides out the device pixel ratio so the degraded path stays logical on a DPR!=1 display. Adds a regression test: an off-center (top-left) target whose q-ref tap must land on it, which fails when the entry is anchored at the viewport.
…pattern) Locks that a q-ref resolves to the owning element and its pointer tap reaches the underlying GestureDetector for the WAnchor `semanticLabel != null` branch (Semantics(button, onTap, excludeSemantics: true) inside a Semantics(container, toggled) + MergeSemantics wrapper, exactly what WSwitch builds). Passes at DPR 1 and 2, confirming dusk's core handling of this structure is correct: the live MSSwitch-tap gap seen in uptizm is not a widget-test-reproducible dusk-core defect.
…in text find-by-label returned the first node with a matching label. When a visible label Text names an adjacent control (the settings-toggle / MSSwitch pattern: a WText 'Email' beside a switch whose semanticLabel is also 'Email') or a heading repeats a button's text (the login 'Sign In' heading vs the submit button), the inert text sits first in tree order, so the handle resolved to it and the tap landed on the label instead of the control -- silently doing nothing. Resolve to the first INTERACTIVE match (button / switch / text field / anything exposing SemanticsAction.tap) when the label collides across an interactive and a non-interactive node, falling back to the first match otherwise. Covers the live MSSwitch toggles (notification prefs, status-page monitor assignment) and removes the need for e-ref workarounds on ambiguous labels. Adds two regression tests; full suite 797 green.
There was a problem hiding this comment.
Pull request overview
This PR improves ext.dusk.find / q-ref accuracy by (1) anchoring q-ref gesture dispatch to the target widget’s rect (instead of the viewport) and (2) preferring interactive semantics nodes when a label collides with inert text, with accompanying regression tests and a changelog entry.
Changes:
- Update label matching to prefer the first interactive
SemanticsNodewhen multiple nodes share the same label. - Rework q-ref
RefEntrymaterialization to resolve the owning element/render box for correct live dispatch coordinates (and adjust semantics-rect fallback for DPR). - Add widget tests covering off-center q-ref taps,
excludeSemanticstap patterns, and ambiguous-label selection; updateCHANGELOG.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
lib/src/extensions/ext_find.dart |
Adjusts semantics-node selection and q-ref anchoring to improve tap accuracy and disambiguation. |
test/src/extensions/ext_find_test.dart |
Adds regression tests validating q-ref dispatch location and interactive-node preference under label collisions. |
CHANGELOG.md |
Documents the behavioral fixes under [Unreleased] > Fixed. |
Comment on lines
+453
to
+457
| final bool sized = renderObject is RenderBox && | ||
| renderObject.attached && | ||
| renderObject.hasSize; | ||
| final Rect resolvedRect = sized | ||
| ? renderObject.localToGlobal(Offset.zero) & renderObject.size |
Comment on lines
+25
to
+27
| - **`q<N>` (find / observe) taps now dispatch at the target's own rect instead of the viewport centre.** `_entryFromSemanticsNode` anchored the `RefEntry` at the root element, so `dispatchRectOf` returned `_liveRectOf(root)` (the whole viewport) and every find/observe gesture fired at screen centre. A centred target coincidentally worked; off-centre controls (a submit button, a checkbox, a sidebar item) were missed silently. The entry now resolves the element whose `RenderBox` contributes the node (via `debugSemantics` identity, matching `ext_observe`), so the gesture lands on the addressed widget. Touches `lib/src/extensions/ext_find.dart`; covered by `test/src/extensions/ext_find_test.dart`. | ||
|
|
||
| - **`find`-by-label now prefers the first INTERACTIVE match when a label collides with inert text.** A visible `Text` naming an adjacent control (a settings label beside a switch that shares its `semanticLabel`) or a heading repeating a button's text (a "Sign In" heading over the submit button) sits first in tree order, so the handle resolved to the inert node and the tap landed on the label. `find` now resolves to the first node exposing `SemanticsAction.tap` (button / switch / text field) when the label spans an interactive and a non-interactive node, falling back to the first match otherwise. `matchCount` / `diagnostic` still report the collision. Touches `lib/src/extensions/ext_find.dart`; covered by `test/src/extensions/ext_find_test.dart`. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
A separate sized boolean does not promote renderObject for the localToGlobal and size reads, so a reviewer reasonably read the RefEntry resolution as unsound. Bind the RenderBox to a single non-null local and share it between the rect calculation and the RefEntry.renderObject field.
The existing collision test uses a button node, which short-circuits at the isButton check. Add a tappable non-button row (a ListTile-style control exposing SemanticsAction.tap without the button flag) so the interactive preference exercises the tap-action path.
Sync the find command doc with the runtime: a mixed label collision now resolves to the first interactive node, and each q-ref gesture dispatches at the resolved node own on-screen rect rather than the viewport centre.
The fallback rect path runs only for a synthetic / non-RenderBox-owned node; every node matched through the find handler with a real widget tree resolves to its owning RenderBox (verified across scroll/merge/link/list structures), so it is unreachable via the public handler. Expose the helper with @VisibleForTesting and verify the transform-composition + DPR-correction math directly (a wrong DPR divide lands every fallback tap at N times the offset).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two related q-ref (find/observe) accuracy fixes plus their regression test.
1. Viewport-centre dispatch (
17bf0fb)_entryFromSemanticsNodeanchored theRefEntryat the root element, sodispatchRectOf= the whole viewport and every find/observe tap fired at screen centre. Centred targets worked by luck; off-centre controls were missed silently. Now resolves the element whose RenderBox contributes the node (debugSemanticsidentity, matchingext_observe).2. Ambiguous-label resolution (
5b3c584)When a label collides across an inert
Textand an interactive control (a settings label beside a switch with the samesemanticLabel, or a "Sign In" heading over the submit button),findresolved to the first (inert) node and the tap missed. It now prefers the first node exposingSemanticsAction.tap, falling back to first-match otherwise.matchCount/diagnosticstill report the collision.Notes
lib/src/extensions/ext_find.dart; covered bytest/src/extensions/ext_find_test.dart(incl. a WSwitch excludeSemantics regression test).[Unreleased] > Fixed.