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

MudMenuItem: Don't "click" when scrolling menu on touch devices (#7262) #7809

Conversation

ilovepilav
Copy link
Contributor

@ilovepilav ilovepilav commented Nov 25, 2023

Description

This PR solves the issue which touchscreen users cannot scroll MudMenu without invoking OnTouchHandler
Resolves #7262

Added a private boolean "_isTouchMoved" to the MudMenuItem which only true when ontouchmove for stopping OnTouchHandler invoke.

Touch events always start with ontouchstart so we're making "_isTouchMoved" false back when ontouchstart.

MudMenuItem's OnTouchHandler already binded to ontouchend event so it always works when the user just touches.

ontouchstart + ontouchmove + ontouchend(OnTouchHandler called) => ❌

ontouchstart + ontouchend(OnTouchHandler called) => ✅

How Has This Been Tested?

  • Added 2 unit tests.
  • All tests passed.
  • Tested the view(MenuItemActionTest) on mobile and browser's developer tool responsive view
  • Locally tested scrolling on mobile, didn't add another testing view for it, since they have same functionality with MenuItemActionTest view

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
Before.MP4
After_Fix.MP4
Menu_Scroll_Test_After_Fix.MP4

Checklist:

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

@github-actions github-actions bot added bug Something does not work as intended/expected PR: needs review labels Nov 25, 2023
@ilovepilav ilovepilav changed the title MudMenuItem: Blocked invoking OnToucHandler after ontouchmove (#7262) MudMenuItem: Blocked invoking OnTouchHandler after ontouchmove (#7262) Nov 25, 2023
Copy link

codecov bot commented Nov 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c1aada9) 87.18% compared to head (f584706) 87.19%.
Report is 2 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #7809   +/-   ##
=======================================
  Coverage   87.18%   87.19%           
=======================================
  Files         389      389           
  Lines       11419    11421    +2     
  Branches     2299     2299           
=======================================
+ Hits         9956     9958    +2     
  Misses        966      966           
  Partials      497      497           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@henon henon changed the title MudMenuItem: Blocked invoking OnTouchHandler after ontouchmove (#7262) MudMenuItem: Allow scrolling menu on touch devices (#7262) Nov 25, 2023
@henon henon changed the title MudMenuItem: Allow scrolling menu on touch devices (#7262) MudMenuItem: Don't "click" when scrolling menu on touch devices (#7262) Nov 25, 2023
@henon henon merged commit df6d943 into MudBlazor:dev Nov 25, 2023
4 checks passed
@henon
Copy link
Collaborator

henon commented Nov 25, 2023

Thanks @ilovepilav for fixing this bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as intended/expected PR: needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MudMenu not suited for touchscreen devices
2 participants