Skip to content

Conversation

@ScarletKuro
Copy link
Member

@ScarletKuro ScarletKuro commented Jul 19, 2024

Description

Fixes: #9288

I don’t know why, but somehow these:

@ontouchstart="OnTouchStart"
@ontouchend="OnTouchEnd"
@ontouchcancel="OnTouchCancel"

were affecting the behavior. It wasn’t due to the JS mudElementRef.addDefaultPreventingHandlers or the stopPropagation="true".

Replacing them with onpointerdown, onpointerup, and onpointercancel fixed the issue for some reason.

Pointer events are a superset of mouse, touch, and pen events. This is an improvement because I always thought swipe gestures should work with both touch and mouse devices, but it didn’t before.

How Has This Been Tested?

Visually

Type 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)
  • Documentation (fix or improvement to the website or code docs)

Checklist

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

@danielchalmers
Copy link
Member

Does this mean you can perform swipe actions with a mouse?

@ScarletKuro ScarletKuro added breaking change This change will require consumer code updates (apply this instead of enhancement) enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library bug Unexpected behavior or functionality not working as intended API change Modifies the public API surface labels Jul 19, 2024
@ScarletKuro
Copy link
Member Author

Does this mean you can perform swipe actions with a mouse?

Yes. Was in a process to write a description.

@ScarletKuro
Copy link
Member Author

ScarletKuro commented Jul 19, 2024

Forgot to migrate tests

upd: Done

@danielchalmers
Copy link
Member

Does it work with Color Picker, drag & drop re-ordering, etc? Though I guess those wouldn't have worked right with touch in that case.

I wonder how people that use this control expect it to work and if they want to use it with mice. I can't think of any strong arguments either way - it seems like a fine change.

MAUI/Windows don't do it: dotnet/maui#6152 (comment) but sites like YouTube do.

@ScarletKuro
Copy link
Member Author

ScarletKuro commented Jul 19, 2024

Does it work with Color Picker, drag & drop re-ordering, etc? Though I guess those wouldn't have worked right with touch in that case.

Do you mean do they work if MudSwipeArea is placed on same page? No idea, didn't test that yet, you can test it if you want, I'm going to sleep for now.

I wonder how people that use this control expect it to work and if they want to use it with mice

Personally, I think it's a must. It's used in carousel, and not having the swipe gesture for mice in a carousel is awkward. If they don't want the gesture there is a property that disables it for the carousel.

@ScarletKuro ScarletKuro reopened this Jul 19, 2024
@ScarletKuro
Copy link
Member Author

ScarletKuro commented Jul 19, 2024

MAUI/Windows don't do it

Yea, it's their legacy behavior that they drag everywhere WPF, Winform etc. It's a poor design, always got annoyed on poor support for touch in WPF in the components. Making a swipe gesture for a WrapPanel etc when business say that scollbar should be hidden is like good luck.

@codecov
Copy link

codecov bot commented Jul 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.54%. Comparing base (00eb472) to head (dcde011).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #9445   +/-   ##
=======================================
  Coverage   91.53%   91.54%           
=======================================
  Files         414      414           
  Lines       13014    13014           
  Branches     2454     2454           
=======================================
+ Hits        11913    11914    +1     
  Misses        551      551           
+ Partials      550      549    -1     

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


🚨 Try these New Features:

@ScarletKuro
Copy link
Member Author

Does it work with Color Picker, drag & drop re-ordering, etc?

It works with both mice and touch when these components are present with MudSwipeArea.

However, while testing, I noticed that Drag and Drop also causes the same exact bug with the Menu when it's present on the page. This is because it also uses:

@ontouchstart="TouchStartedAsync"
@ontouchend="TouchEndedAsync"
@ontouchmove="TouchMovedAsync"

But this is a separate issue as it involves a different component.

@ScarletKuro
Copy link
Member Author

The #9458 fixes the main issue.
But we can have this as enchantment. I will add v8 label

@ScarletKuro ScarletKuro added the v8 label Jul 21, 2024
@henon
Copy link
Contributor

henon commented Jul 23, 2024

enchantment

Ego invoco deus MudBlazori ab mihi. Protego mihi ab hostili et malum!

xD

@ScarletKuro
Copy link
Member Author

enchantment

Ego invoco deus MudBlazori ab mihi. Protego mihi ab hostili et malum!

xD

Ups xD enhancement*

@ScarletKuro
Copy link
Member Author

ScarletKuro commented Oct 31, 2024

Do we want this change? to make Swipe work with mouse not just touch or nah?

@danielchalmers
Copy link
Member

Do we want this change? to make Swipe work with mouse not just touch or nah?

I think it makes some sense to have it for both. YouTube lets you open the sidebar with a cursor or touch for example

@ScarletKuro
Copy link
Member Author

ScarletKuro commented Nov 8, 2024

I think it makes some sense to have it for both. YouTube lets you open the sidebar with a cursor or touch for example

Yeah, I also see a lot of carousel ads on electronic websites, and I always swipe them with a mouse to see if there are any good discount / gift offer.

@sonarqubecloud
Copy link

Copy link
Member

@danielchalmers danielchalmers left a comment

Choose a reason for hiding this comment

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

Could make the text in the examples user-select: none;

video6.mp4

@ScarletKuro
Copy link
Member Author

Added to v8.0.0 Migration Guide #9953

@ScarletKuro ScarletKuro merged commit fe4b8fd into MudBlazor:dev Nov 22, 2024
@ScarletKuro ScarletKuro deleted the swipe_fix branch November 22, 2024 14:50
@veler
Copy link

veler commented Dec 13, 2024

Hi @ScarletKuro,

I'm fighting with MudSwipeArea today. I don't seem to get it to work on an emulator. I tested on Edge's Dev Tools with the iPhone simulation and a .NET MAUI's Android emulator. None work. Interestingly, the MudSwipeArea in MudBlazor documentation website works in the emulator. Perhaps it's running on an older version of MudBlazor.

I tried making a div tag and handle the ontouchstart and other events manually in Blazor. It works fine in the emulators.

I haven't tested a physical device. Would it make a difference?

Thanks

@Anu6is
Copy link
Contributor

Anu6is commented Dec 13, 2024

@veler are you using the version 8 preview release? These changes aren't yet in the officially released version - v7

@veler
Copy link

veler commented Dec 13, 2024

@veler are you using the version 8 preview release? These changes aren't yet in the officially released version - v7

Hi! Yes, I'm using 8.0 preview 5

@ScarletKuro
Copy link
Member Author

As it was said, this change are not released yet.

@veler
Copy link

veler commented Dec 13, 2024

As it was said, this change are not released yet.

Are you sure? I do see this change in the changelog of the release 8.0 preview 5, and that's what I'm using.

https://github.com/MudBlazor/MudBlazor/releases/tag/v8.0.0-preview.5

@ScarletKuro
Copy link
Member Author

ScarletKuro commented Dec 13, 2024

As it was said, this change are not released yet.

Are you sure? I do see this change in the changelog of the release 8.0 preview 5, and that's what I'm using.

https://github.com/MudBlazor/MudBlazor/releases/tag/v8.0.0-preview.5

My bad, you are right. Does it work for you on dev.mudblazor.com ?

upd: hm seems not to work on real device, I will check later.

@ScarletKuro
Copy link
Member Author

upd: hm seems not to work on real device, I will check later.

Weird that it does work in firefox touch emulator tho.

@veler
Copy link

veler commented Dec 15, 2024

upd: hm seems not to work on real device, I will check later.

Weird that it does work in firefox touch emulator tho.

that's interesting 🤔

LLauter pushed a commit to cannellamedia/MudBlazor that referenced this pull request Dec 16, 2024
@ScarletKuro
Copy link
Member Author

ScarletKuro commented Dec 17, 2024

Fixed in #10461. As a temporary solution, you can add touch-action: none; on your own.

@veler
Copy link

veler commented Dec 18, 2024

Fixed in #10461. As a temporary solution, you can add touch-action: none; on your own.

You rock! Thank you so much for this quick turnaround!

@changingmission
Copy link

Mobile Scroll is not working even after setting PreventDefault="false" : https://try.mudblazor.com/snippet/GYGpOPQkEjygXgAJ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API change Modifies the public API surface breaking change This change will require consumer code updates (apply this instead of enhancement) bug Unexpected behavior or functionality not working as intended enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MudSwipeArea prevents interaction with MudMenu items

6 participants