microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.28k stars 675 forks source link

Slider Tapped event doesn't always fire #4969

Closed pdecostervgls closed 7 months ago

pdecostervgls commented 3 years ago

Describe the bug When declaring a slider in XAML with a Tapped event, the resulting EventHandler doesn't always get called when the slider is clicked/tapped.

Steps to reproduce the bug

  1. Create a new WInUI on UWP project

  2. Add the following XAML to MainPage.xaml/MainWindow.xaml

    <StackPanel Orientation="Horizontal" HorizontalAlignment="Center" VerticalAlignment="Center">
    <Slider Tapped="Slider_Tapped" Width="400"/>
    <TextBlock x:Name="tb" Text="0 times tapped"/>
    </StackPanel>
  3. Add the following code to MainPage.xaml.cs/MainWindows.xaml.cs

    
    private int timesTapped = 0;

private void Slider_Tapped(object sender, TappedRoutedEventArgs e) { timesTapped++; tb.Text = String.Format("{0} times tapped", timesTapped); }



4. Run the project
5. Start Tapping on the slider, on different positions of the track
6. Notice the tap count doesn't always get incremented.

**Expected behavior**
The `Tapped` event should be called with every click/tap on the slider.

**Screenshots**
<!-- If applicable, add screenshots here to help explain your problem -->

**Version Info**
<!-- Please enter your WinUI NuGet package version, Windows app type (when using WinUI 3+), OS version(s), and form factor(s) -->

NuGet package version: 
WinUI 3 - Project Reunion 0.5 Preview: 0.5.0-prerelease

<!-- If you are using WinUI 3, please specify for which Windows app type you have encountered the issue. Leave blank if you didn't try that app type. -->
Windows app type:
| UWP              | Win32            |
| :--------------- | :--------------- |
| Yes |  Yes |

<!-- Which Windows versions did you see the issue on? Leave blank if you didn't try that version. -->
| Windows 10 version                  | Saw the problem? |
| :--------------------------------- | :-------------------- |
| Insider Build (xxxxx)              | <!-- Yes/No? --> |
| October 2020 Update (19042)        | Yes   |
| May 2020 Update (19041)            | <!-- Yes/No? -->   |
| November 2019 Update (18363)       | <!-- Yes/No? -->   |
| May 2019 Update (18362)            | <!-- Yes/No? -->   |
| October 2018 Update (17763)        | <!-- Yes/No? -->   |
| April 2018 Update (17134)          | <!-- Yes/No? -->   |
| Fall Creators Update (16299)       | <!-- Yes/No? -->   |
| Creators Update (15063)            | <!-- Yes/No? -->   |

<!-- Which device form factors did you see the issue on? Leave blank if you didn't try that device. -->
| Device form factor | Saw the problem? |
| :----------------- | :--------------- |
| Desktop            | Yes |
| Xbox               | <!-- Yes/No? --> |
| Surface Hub        | <!-- Yes/No? --> |
| IoT                | <!-- Yes/No? --> |

**Additional context**
This issue also occurs on the old UWP API
StephenLPeters commented 3 years ago

My guess is that something in the slider is handling certain pointer events. For instance, perhaps if you move the mouse/finger slightly during your tap the slider is handling the event (since it is moving the slider thumb) and thus you aren't getting your callback.

You could try, from code behind attaching using the event using the UIElment.AddHandler method. That method has a boolean you can use to have your handler fire even when the event is marked as handled. https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.uielement.addhandler?view=winrt-19041 That would let us know if my guess is right.

pdecostervgls commented 3 years ago

@StephenLPeters Thanks for your response. I added the AddHandler in code-behind, with the handledEventsToo boolean to true. I do however still miss tapped events.

ranjeshj commented 3 years ago

The slider could be handling the event as drag even for small movements that maybe are blocking the tapped events. @pdecostervgls what are you trying to do by handling the tapped event ?

pdecostervgls commented 3 years ago

@ranjeshj For now I just created a sample app to demonstrate the deficit in behaviour, but in my real-world project I have the slider value bound to a property on my ViewModel, and I need to known whether the user changed it or the value has been changed by the Two-Way binding somewhere else.

StephenLPeters commented 3 years ago

In that case I don't think tapped solves your case either. Since the user could use keyboard to change the slider value as well. It seems like you would like the slider's ValueChangedEventArgs to have a property that tells you the source of the change.

Regardless, this does seem like an issue.

zelaskjd commented 1 year ago

Is there any update on this bug such as a timeline for when it will be fixed or an official workaround? It appears to still be happening in WinUI 3 applications written in VS 2019 and 2022 targeting .NET 5.0 and 6.0.

zelaskjd commented 1 year ago

@pdecostervgls were you ultimately able to find a workaround for this issue?

@StephenLPeters with this still being an issue, is there any update on when it may be fixed or any changes that provide a dependable workaround?

eFail commented 1 year ago

I'm hitting this same issue. Create a slider control, add an event handler on the Tapped event, and then alternate between moving the mouse slightly to the left, and then slightly to the right over the slider. Click down after each move. Some of the clicks result in tapped events, some don't. Would be nice to get a fix for this, something basic is clearly broken.

I can confirm that this is not a side effect of the mouse moving ever so slightly while clicking. I'm being very precise and slow with my clicks.

Update: This issue doesn't seem to be specific to the Tapped event. The same thing happens with the PointerReleased event (sometimes it doesn't fire).

To work around this issue, I first tried adding a Tapped handler to the slider using AddHandler() with handledEventsToo=true, as @StephenLPeters suggested. This doesn't help though, the events still get dropped on the table.

I used this same trick to listen to PointerReleased events, however, and that does work! This has allowed me to work around the bug for the time being. Whenever I get a pointer released event, I treat it as a tap.

ranjeshj commented 11 months ago

Could you help me understand the issue and why ValueChanged event cannot be used to know when the value changes? Doing this for all input would mean handling it for touch (tapped), mouse (pointer), keyboard etc.. ValueChanged would be a simpler way to handle it. The only case would be when the value changes through the data model, but that might be easier to figure out/exclude since the app is doing that maybe?

eFail commented 11 months ago

@ranjeshj For what it's worth, I think there is a legitimate bug here and I don't think this issue should be closed out permanently as "not planned". It's very simple to repro this:

Expected result: Every adjustment results in all three events being fired. Actual result: The events randomly don't fire. The slider control eats them at random times.

This is basic functionality that developers should be able to rely on. IMO this needs to be fixed. Maybe it doesn't have a high priority, but it's definitely broken.

Why can't ValueChanged be used instead? Because that tells you about value changes only and tells you nothing about manipulation. Here's a concrete scenario: Imagine you have a slider where after the user stops dragging the slider around, you want to save the final value off to a database. You don't want to do this during every single value change while the slider is being dragged around. You only want to do it after the slider has been released and the user has finalized their value selection.

It should be very simple to do this. Listen for PointerReleased or Tapped, and then commit the slider's current value. Instead, the slider control is broken, and developers have to go to extravagant lengths to work around this bug.

zelaskjd commented 11 months ago

What @eFail said.

@ranjeshj Why even provide those handlers if they aren't going to work properly? Ignorant choice to close this issue prematurely. Especially awful to do that immediately after literally asking for more information...

ranjeshj commented 9 months ago

Sorry about that. I can repro this easily. The track seems to be swallowing these events. Clicking on the track is not raising the event, but anywhere outside the track works.

kmahone commented 7 months ago

The behavior here is a little confusing, but it is ultimately a consequence of how input event routing works in xaml. The design has been around since the beginning and is unlikely to change.

UIElement defines a bunch of basic input events such as PointerPressed etc. The different implementations of the controls (which ultimately derive from UIElement) internally handle these events as part of their interaction logic. The events that are internally handled consequently do not get raised publicly. This is not specific to Slider, you can see the same behavior on Button. If you register for the PointerPressed event on Button, you will see that it is not raised. Similarly the implementation of the Slider is interfering with the Tapped gesture as the thumb moves under the pointer in response to the PointerPressed.

The intention is that consumers of the control use the 'higher level' events that are defined on the control such as ValueChanged, Click, etc.

The scenario that is brought up on this thread is interesting - being able to tell if a specific ValueChanged came from the user and also being able to consolidate the events instead of handling each one that is raised as the user scrubs the slider.

I think the best way to handle this scenario is to continue with the approach that has previously been discussed here: use UIElement.AddHandler for PointerPressed and PointerReleased with the handledEventsToo parameter set to true. This will allow you to see the Pressed and Released events even though they are being handled by the Slider. Between a Pressed and a Released event you can temporarily stop updating your ViewModel in response to the ValueChanged events that get raised.

When customizing the behavior of the Slider in this manner, make sure to consider other methods of the user modifying the value including keyboard interaction as well as UIAutomation (such as users of accessibility tools such as Narrator).

The current behavior of Slider is a consequence of how input routing and gesture recognition work. It is a little confusing but it is not something that we are going to be able to change.

eFail commented 7 months ago

@kmahone Thank you for the detailed information. It's unfortunate to hear that this can't be fixed, and I understand the closed resolution. In the interest of WinUI adoption, maybe you at least work with a content writer to have this official recommendation called out on the MSDN page for Slider? I get not needing this documentation for a Button, but coalescing values for a Slider does seem like a somewhat common scenario. It would be nice if the documentation explicitly called out how to handle this scenario, since the workaround needed isn't something trivial to figure out on your own.

kmahone commented 7 months ago

I think this is an interesting scenario that would be useful to support. We have a few similar cases in the platform where this works well such as ScrollViewerViewChangedEventArgs has a bool IsIntermediate property to allow one to determine if a particular instance of the event being raised is an intermediate event or a final event. I have filed an internal work item on our backlog to add support for this. This work will be triaged relative to other work so it will likely not be available in the near future.