DropArea: route Drop only to a previously-accepting target#11716
DropArea: route Drop only to a previously-accepting target#11716tronical wants to merge 2 commits into
Conversation
| /// The item stack of the DropArea that accepted the most recent DragMove, if any. | ||
| /// On Release we use this to route the resulting Drop event only to that DropArea, | ||
| /// matching how OS DnD pipelines deliver drops only to targets that previously accepted. | ||
| pub(crate) drop_target_stack: Option<Vec<(ItemWeak, InputEventFilterResult)>>, |
There was a problem hiding this comment.
What is it a stack?
I'd say only one should accept it.
There was a problem hiding this comment.
Ahh, that was when I first tried to cover this with nested touch areas. But indeed, we only need the last anyway.
The Drop arm fired `dropped` unconditionally, even when `can-drop` had returned false on the most recent DragMove. Track on `MouseInputState.drop_target` whichever DropArea accepted that DragMove, and on Release only convert to `MouseEvent::Drop` if such a target was recorded. Otherwise tear the drag down via `MouseEvent::Exit` so neither the Drop nor the underlying Release reaches a non-accepting target. Adds a rejection scenario to dragarea_droparea.slint.
tronical
force-pushed
the
simon/dnd-no-drop-after-false-can-drop
branch
from
45fd8e8 to
504cb30
Compare
ogoffart
left a comment
ogoffart
left a comment
There was a problem hiding this comment.
We need to consider that the position of the release might not be the same position as the move.
I don't know if any of our backend do that. But this is not an invariant in our platform API that there must always be move event at the same position before the release event.
For that reason, i think it is the responsibility of the user's drop implementation to always check the validity of the drop before and not assume can-drop always returned true.
We could change that by always calling the can-drop before drop in the DropArea event handler.
| mouse_input_state.drag_data = None; | ||
| if mouse_input_state.drop_target.take().is_some() { | ||
| drop_event.position = crate::lengths::logical_position_to_api(*position); | ||
| event = MouseEvent::Drop(drop_event); |
There was a problem hiding this comment.
Note that you can still end up deliverying the event to a different item if the position changed from one item to another.
|
Agreed. This is then covered by #11720 where |
2 participants
droppedused to fire even aftercan-dropreturned false. Fix at therouting layer: track on
MouseInputState.drop_target_stackthe itemstack of the DropArea that accepted the most recent DragMove. On
Release, only dispatch a
Dropif the release point lies inside thatDropArea, and route it via the existing grab mechanism so non-accepting
DropAreas never see it. The full ancestry chain is stored (not just the
target's weak ref) so nested DropAreas with inner-reject + outer-accept
skip past the inner one that hit-test would reach first.
Outside that case the release is converted to
MouseEvent::Exitso thedrag tears down cleanly without the underlying Release registering as a
spurious click/hover on the hit-tested item.
Adds a rejection-path scenario to dragarea_droparea.slint.