-
Notifications
You must be signed in to change notification settings - Fork 868
fix: Cancel click on touch long-press menu #23596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,6 +29,10 @@ internal partial class ContextMenuProcessor | |||||||||||||||||||
| private DispatcherTimer? _contextMenuTimer; | ||||||||||||||||||||
| private Point _contextMenuOnHoldingTouchPoint = new(-1, -1); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Uno specific: the touch pointer that triggered the holding gesture, so its capture can be | ||||||||||||||||||||
| // released once the context menu is shown (see ReleaseContextMenuHoldingPointerCapture). | ||||||||||||||||||||
| private uint _contextMenuOnHoldingPointerId; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public ContextMenuProcessor(ContentRoot contentRoot) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| _contentRoot = contentRoot ?? throw new ArgumentNullException(nameof(contentRoot)); | ||||||||||||||||||||
|
|
@@ -95,6 +99,32 @@ public void RaiseContextRequestedEvent( | |||||||||||||||||||
| if (args.Handled && isTouchInput) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| _isContextMenuOnHolding = true; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // A context menu was shown in response to a touch press-and-hold. WinUI suppresses the | ||||||||||||||||||||
| // pending Click/Tapped because opening the menu's popup steals the OS pointer capture | ||||||||||||||||||||
| // (raising PointerCaptureLost on the pressing element). Uno's popups don't steal capture, | ||||||||||||||||||||
| // so release it explicitly here, matching that behavior across all targets. | ||||||||||||||||||||
| ReleaseContextMenuHoldingPointerCapture(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||
| /// Releases the explicit pointer capture held for the touch pointer that triggered the holding | ||||||||||||||||||||
| /// gesture, raising PointerCaptureLost so the pressing element (e.g. a Button) clears its pressed | ||||||||||||||||||||
| /// state and does not raise Click/Tapped on release (issue #22229). | ||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| private void ReleaseContextMenuHoldingPointerCapture() | ||||||||||||||||||||
| { | ||||||||||||||||||||
| if (PointerCapture.Any(out var captures)) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| foreach (var capture in captures) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| if (capture.Pointer.PointerId == _contextMenuOnHoldingPointerId | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When
Suggested change
|
||||||||||||||||||||
| && capture.ExplicitTarget is { } captureTarget) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| captureTarget.ReleasePointerCaptureForContextMenu(capture.Pointer); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -166,4 +196,9 @@ private static void OnContextRequestOnHoldingTimeout(object? sender, object e) | |||||||||||||||||||
| /// Sets the touch point for context menu on holding gesture. | ||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| public void SetContextMenuOnHoldingTouchPoint(Point point) => _contextMenuOnHoldingTouchPoint = point; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||
| /// Sets the touch pointer id that triggered the holding gesture. | ||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| public void SetContextMenuOnHoldingPointer(uint pointerId) => _contextMenuOnHoldingPointerId = pointerId; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -483,6 +483,9 @@ internal HitTestability GetHitTestVisibility() | |||||||||||
| // but ContextRequestedEventArgs.TryGetPosition expects global coordinates. | ||||||||||||
| var globalPosition = that.TransformToVisual(null).TransformPoint(args.Position); | ||||||||||||
| contentRoot.InputManager.ContextMenuProcessor.SetContextMenuOnHoldingTouchPoint(globalPosition); | ||||||||||||
| // Record the pointer so that, once the context menu is actually shown, its capture | ||||||||||||
| // can be released to cancel the pending Click/Tapped on the pressing element. | ||||||||||||
| contentRoot.InputManager.ContextMenuProcessor.SetContextMenuOnHoldingPointer(args.PointerId); | ||||||||||||
| contentRoot.InputManager.ContextMenuProcessor.ProcessContextRequestOnHoldingGesture(src); | ||||||||||||
| } | ||||||||||||
| else if (args.HoldingState == HoldingState.Canceled) | ||||||||||||
|
|
@@ -1641,6 +1644,33 @@ public void ReleasePointerCaptures() | |||||||||||
|
|
||||||||||||
| Release(PointerCaptureKind.Explicit); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /// <summary> | ||||||||||||
| /// Releases the explicit pointer capture and raises an <em>unhandled</em> PointerCaptureLost so the | ||||||||||||
| /// pressing control clears its pressed state. Used when a context menu is shown on a touch press-and-hold: | ||||||||||||
| /// WinUI clears the pressed state via the OS pointer-capture-changed raised when the menu's popup steals | ||||||||||||
| /// capture, but Uno's popups don't, so we do it explicitly (issue #22229). | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor style (AGENTS.md): "Don't reference the current task, fix, or callers ('handles the case from issue #123') since those belong in the PR description and rot as the codebase evolves." Remove
Suggested change
|
||||||||||||
| /// </summary> | ||||||||||||
| /// <remarks> | ||||||||||||
| /// The capture tracks the args from the PointerPressed, which the pressing control marks as Handled. | ||||||||||||
| /// PointerCaptureLost reuses those args, so we reset Handled first - otherwise the control's class handler | ||||||||||||
| /// (which only runs for unhandled events) never clears the pressed state. | ||||||||||||
| /// </remarks> | ||||||||||||
| internal void ReleasePointerCaptureForContextMenu(Pointer pointer) | ||||||||||||
| { | ||||||||||||
| if (PointerCapture.TryGet(pointer, out var capture)) | ||||||||||||
| { | ||||||||||||
| foreach (var target in capture.GetTargets(PointerCaptureKind.Explicit)) | ||||||||||||
| { | ||||||||||||
| if (target.LastDispatched is { } lastDispatched) | ||||||||||||
| { | ||||||||||||
| lastDispatched.Handled = false; | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| ReleasePointerCapture(pointer); | ||||||||||||
| } | ||||||||||||
| #endregion | ||||||||||||
|
|
||||||||||||
| private bool ValidateAndUpdateCapture(PointerRoutedEventArgs args) | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same minor style issue —
(issue #22229)in the doc comment body. Drop it; issue context lives in the commit message and PR description, not in code prose.