Skip to content
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

[Windows] Tap Events Incorrectly passed Through Button - fix #26834

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

MartyIX
Copy link
Contributor

@MartyIX MartyIX commented Dec 26, 2024

Description of Change

#25311 makes it possible to register when an entry is tapped. However, it seems it breaks other controls.

This is, in a way, naive way to fix it by "special-casing" (see the third commit) #25311's behavior just for entries and not other controls.

Notes:

  • The first commit modifies test so one can check behavior "before" and "after"
  • The second commit is strictly to fix preexisting code style issues.
  • The third commit is the workaround.

Issues Fixed

Fixes #26640

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Dec 26, 2024
@MartyIX
Copy link
Contributor Author

MartyIX commented Dec 26, 2024

cc @NirmalKumarYuvaraj

@MartyIX MartyIX marked this pull request as ready for review December 26, 2024 23:14
@MartyIX MartyIX requested a review from a team as a code owner December 26, 2024 23:14
@MartyIX MartyIX changed the title Tap Events Incorrectly passed Through Button - fix [Windows] Tap Events Incorrectly passed Through Button - fix Dec 27, 2024
@MartyIX MartyIX force-pushed the feature/2024-12-26-Windows-Click-through branch from c1e1c2f to 5cc8f85 Compare December 27, 2024 10:01
@MartyIX MartyIX requested review from Foda and removed request for StephaneDelcroix and tj-devel709 December 27, 2024 10:01
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen added this to the .NET 9 SR3 milestone Dec 27, 2024
@PureWeen PureWeen added the p/0 Work that we can't release without label Dec 27, 2024
@PureWeen
Copy link
Member

PureWeen commented Dec 28, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@MartyIX MartyIX force-pushed the feature/2024-12-26-Windows-Click-through branch 2 times, most recently from 9872c1c to 52320be Compare December 29, 2024 09:55
@MartyIX MartyIX force-pushed the feature/2024-12-26-Windows-Click-through branch from 52320be to aa3c96d Compare December 29, 2024 13:05
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@MartyIX
Copy link
Contributor Author

MartyIX commented Dec 30, 2024

Failed tests look unrelated. I rerun locally ControlCanBeFocusedByUnfocusedEvent and it passes on Windows.

@MartyIX MartyIX force-pushed the feature/2024-12-26-Windows-Click-through branch from aa3c96d to 6aea414 Compare January 2, 2025 19:31
@MartyIX
Copy link
Contributor Author

MartyIX commented Jan 2, 2025

Pushed to fix merge conflict.

@@ -783,8 +864,17 @@ void UpdatingGestureRecognizers()
|| children?.GetChildGesturesFor<TapGestureRecognizer>(g => g.NumberOfTapsRequired == 1).Any() == true)
{
_subscriptionFlags |= SubscriptionFlags.ContainerTapAndRightTabEventSubscribed;
_tappedEventHandler = new TappedEventHandler(OnTap);
_container.AddHandler(FrameworkElement.TappedEvent,_tappedEventHandler, true);
Copy link
Contributor Author

@MartyIX MartyIX Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #26640 (comment). I tested setting this true to false but it did not work for me then. That's why I tried the "special-casing" approach.

https://learn.microsoft.com/en-us/windows/windows-app-sdk/api/winrt/microsoft.ui.xaml.uielement.addhandler?view=windows-app-sdk-1.6

@PureWeen
Copy link
Member

PureWeen commented Jan 3, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-gestures Gesture types community ✨ Community Contribution p/0 Work that we can't release without platform/windows 🪟
Projects
Status: Ready To Review
Development

Successfully merging this pull request may close these issues.

Tap Events Incorrectly passed Through Button
3 participants