fix(text-input): select the target field's editable by rect, not the first in the tree#25
Open
anilcancakir wants to merge 5 commits into
Open
fix(text-input): select the target field's editable by rect, not the first in the tree#25anilcancakir wants to merge 5 commits into
anilcancakir wants to merge 5 commits into
Conversation
…alToGlobal), not the first in the tree
There was a problem hiding this comment.
Pull request overview
This PR fixes ext.dusk.type (and aligns ext.dusk.clear) so typing/clearing targets the correct EditableText in multi-field forms by selecting the editable that best matches the ref’s on-screen rect, rather than defaulting to the first editable found in the widget tree.
Changes:
- Add rect-based editable selection to
typeIntoElementand use it fromext.dusk.type(andext.dusk.clear) via a SemanticsNode→localToGlobalrect mapping when available. - Add a widget test covering rect-based routing for
typeIntoElementon a two-field form. - Document the behavior change in
CHANGELOG.mdunder[Unreleased] > Fixed.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/src/extensions/ext_text_input.dart | Implements rect-based EditableText resolution for type/clear using overlap/nearest matching and a SemanticsNode localToGlobal rect derivation. |
| test/src/extensions/ext_text_input_test.dart | Adds a widget test ensuring rect-based selection writes into the targeted (non-first) field. |
| CHANGELOG.md | Notes the dusk:type targeting fix under Unreleased/Fixed. |
Comment on lines
+648
to
+652
| /// CONTAINS the target's center, falling back to the nearest center when none | ||
| /// contains it. This routes a `type`/`clear`/`fill` to the field the agent | ||
| /// actually targeted rather than the first editable in the tree, and prefers | ||
| /// the visible on-screen editable over any zero-sized off-stage accessibility | ||
| /// proxy (whose empty rect is skipped). |
|
|
||
| - **`dusk:doctor` check 3 (snapshot enrichers) now emits INFO when no enrichers are registered, instead of WARN.** Enrichers are opt-in; zero is a valid state, not a problem. The WARN reading alongside "integration wired" (check 5) created false contradiction. Touches `lib/src/commands/dusk_doctor_command.dart`; test case updated in `test/src/commands/dusk_doctor_command_test.dart`. | ||
|
|
||
| - **`dusk:type` now targets the editable inside the ref's own Semantics rect, not the first `EditableText` in the tree.** The handler resolved the field to type into by walking to the first editable under the isolate, so on a form with several inputs a `type` against `q3` (Password) could land in the first field (Email). It now maps the ref's `SemanticsNode` to its global rect (`localToGlobal`) and selects the editable whose render box contains that rect, so the value goes into the addressed field. Touches `lib/src/extensions/ext_text_input.dart`; covered by `test/src/extensions/ext_text_input_test.dart`. |
Comment on lines
+572
to
+576
| final Rect? clearRect = | ||
| _localToGlobalRectForNode(entry?.node) ?? entry?.rect; | ||
| final EditableTextState? state = | ||
| (clearRect != null ? _findEditableTextStateByRect(clearRect) : null) ?? | ||
| _resolveEditableTextState(element); |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…r rect path Address PR review: the docblock and CHANGELOG said the editable is chosen by center-containment, but it ranks by overlap area with a nearest-center fallback; reword both. Add a clear test that proves ext.dusk.clear routes to the field matching targetRect (not the first editable), covering the shared rect path on the clear handler as well as type.
The existing rect-clear test registers a ref with a null node, so the node-render-rect derivation (_localToGlobalRectForNode) never runs. Add the real find -> clear path: a q-ref carries the matched SemanticsNode, so clear resolves the target from the node own render object.
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
dusk:typeresolved which editable to type into by walking to the firstEditableTextin the isolate. On a form with several inputs, atypeagainstq3(Password) could land in the first field (Email), so multi-field fills silently went into the wrong box.Fix
Map the ref's
SemanticsNodeto its global rect vialocalToGlobal, then select the editable whose render box contains that rect. The value now goes into the addressed field.Notes
lib/src/extensions/ext_text_input.dart; covered bytest/src/extensions/ext_text_input_test.dart.[Unreleased] > Fixed.