Maui icon indicating copy to clipboard operation
Maui copied to clipboard

Fix Touch Behavior Cancelled too fast if user just tap it once

Open albilaga opened this issue 1 year ago • 5 comments

Description of Change

When user tap the button or element that have TouchBehavior, the TouchBehavior animation didn't respect DefaultAnimationDuration to finish the animation. So basically after user tap it will directly cancel and it will throw stacktrace

System.Threading.Tasks.TaskCanceledException: A task was canceled.
   at CommunityToolkit.Maui.Behaviors.GestureManager.SetScale(TouchBehavior sender, TouchState touchState, HoverState hoverState, TimeSpan duration, Easing easing, CancellationToken token) in /Users/albilaga/Documents/Personal/CommunityToolkit.Maui/src/CommunityToolkit.Maui/Behaviors/PlatformBehaviors/Touch/GestureManager.shared.cs:line 475
   at CommunityToolkit.Maui.Behaviors.GestureManager.RunAnimationTask(TouchBehavior sender, TouchState touchState, HoverState hoverState, CancellationToken token, Nullable`1 durationMultiplier) in /Users/albilaga/Documents/Personal/CommunityToolkit.Maui/src/CommunityToolkit.Maui/Behaviors/PlatformBehaviors/Touch/GestureManager.shared.cs:line 799
   at CommunityToolkit.Maui.Behaviors.GestureManager.ChangeStateAsync(TouchBehavior sender, Boolean animated, CancellationToken token) in /Users/albilaga/Documents/Personal/CommunityToolkit.Maui/src/CommunityToolkit.Maui/Behaviors/PlatformBehaviors/Touch/GestureManager.shared.cs:line 118
   at CommunityToolkit.Maui.Behaviors.TouchBehavior.ForceUpdateState(CancellationToken token, Boolean animated) in /Users/albilaga/Documents/Personal/CommunityToolkit.Maui/src/CommunityToolkit.Maui/Behaviors/PlatformBehaviors/Touch/TouchBehavior.methods.shared.cs:line 65
System.Threading.Tasks.TaskCanceledException: A task was canceled.

This PR attempt to fix that by waiting with Task.Delay from DefaultAnimationDuration instead of just directly abort it

Linked Issues

  • Fixes #2063

PR Checklist

  • [ ] Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • [ ] Has tests (if omitted, state reason in description)
  • [x] Has samples (if omitted, state reason in description)
  • [x] Rebased on top of main at time of PR
  • [x] Changes adhere to coding standard
  • [ ] Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

albilaga avatar Aug 08 '24 15:08 albilaga

Does your change handles double-tap? If so, how is the behavior?

Based on the referenced issue this fix doesn't solve that problem. I asked the author to open this anyway to show what they had and we work through it. I've added a review comment to show what I think might be the fix. I'll try and test it out later

bijington avatar Aug 09 '24 06:08 bijington

Does your change handles double-tap? If so, how is the behavior?

Based on the referenced issue this fix doesn't solve that problem. I asked the author to open this anyway to show what they had and we work through it. I've added a review comment to show what I think might be the fix. I'll try and test it out later

Yeah, I asked that, because (for me) would be odd to have the animation going on after I release the button, or if I double tap it, would that replicates two animations, if yes, that could cause some confusion on the user...

pictos avatar Aug 10 '24 02:08 pictos

@pictos here is the result when double tap. As you thought it is like handled by two animations. here is the attached result

https://github.com/user-attachments/assets/42c1bd8c-232f-49b3-be17-cda65a495529

so you suggest to like giving throttle to ignore the next tap? or how it should be

Based on the referenced issue this fix doesn't solve that problem. I asked the author to open this anyway to show what they had and we work through it. I've added a review comment to show what I think might be the fix. I'll try and test it out later

@bijington I don't see your review or I miss something?

Btw I change it to draft because I don't think it is ready yet

albilaga avatar Aug 12 '24 03:08 albilaga

@albilaga I believe the control should ignore any other touch while animation is performing. Since this is a single tap instead of a double-tap gesture. For me, at least, feels wrong having that behavior

pictos avatar Aug 12 '24 05:08 pictos

@pictos testing with native android with button pressed state it will just do pressed state again without ignoring the second tap? CleanShot 2024-08-24 at 23 04 40

albilaga avatar Aug 24 '24 16:08 albilaga

Hi @albilaga! Could you update this PR to resolve the merge conflicts?

We've finally merged the massive .NET 9 PR today and it introduced a few merge conflicts across our open PRs.

TheCodeTraveler avatar Dec 18 '24 01:12 TheCodeTraveler

Hi @brminnick will do. this week hopefully can do resolve merge conflict. still got some fever 🤒🤒🤒

albilaga avatar Dec 18 '24 03:12 albilaga

Hi @brminnick , PR already reabased

albilaga avatar Dec 21 '24 09:12 albilaga

Thanks @albilaga! We reviewed this in our January Standup and we decided that the best way forward is to make this behavior configurable.

@jfversluis Will be opening a New Feature Proposal to include the details discussed in the standup.

TheCodeTraveler avatar Jan 09 '25 20:01 TheCodeTraveler

Opened new proposal here: https://github.com/CommunityToolkit/Maui/issues/2433

jfversluis avatar Jan 10 '25 08:01 jfversluis