Fix lower DragTarget not being recognized in overlapping targets#188979
Open
Aurelius51 wants to merge 1 commit into
Open
Fix lower DragTarget not being recognized in overlapping targets#188979Aurelius51 wants to merge 1 commit into
Aurelius51 wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the early-bail logic in _DragAvatar inside drag_target.dart by requiring an exact-length match of hit targets when no target has accepted the drag yet. This allows lower overlapping targets to be recognized when an upper target rejects the drag. Additionally, two test cases are added to draggable_test.dart to verify this new behavior and ensure that accepting upper targets are not bypassed. There are no review comments, so no further feedback is provided.
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.
Fixes #187543
Fixes #143156
_DragAvatar.updateDragbails early when the hit-tested targets share aprefix with
_enteredTargets. That optimization (the pass-through path,refined in #60174) is only correct once a target has accepted the drag
(
_activeTarget != null), because deeper targets below the active one areintentionally ignored. When nothing has accepted yet,
_enteredTargetscontains every hit target, so a newly-appearing lower target makes the hit
list longer while still prefix-matching, and the code bailed without
entering it. Dragging over an upper rejecting target and then into an
overlap therefore left the lower target unrecognized.
The fix only takes the early-bail when
_activeTarget != nullor the targetset is exactly unchanged, so the fast-path is preserved once a target has
accepted, while a newly-appearing lower target is re-evaluated when nothing
has accepted.
Tests
Added a regression test (over an upper rejecting target, then into the
overlap, the lower target now accepts; verified it fails without the fix and
passes with it) and a guard test (when the upper target accepts, the drag
does not fall through to a lower overlapping target). The full
draggable_test.dart suite passes.
Design note
When nothing has accepted yet and a new lower target appears, the existing
leave/re-enter path re-evaluates the targets, which fires
onLeaveonce onthe still-hovered upper target before re-entering it. This matches existing
behavior whenever the entered-target set changes and is intentionally
minimal; a plain
==check would instead defeat the accepted-targetfast-path.