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.

Sign up for GitHub

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

Jump to bottom

[slider] cannot move slider using mouse on iPad #1988

Closed
jamessampford opened this issue Jun 17, 2021 · 8 comments · Fixed by #2327
Closed

[slider] cannot move slider using mouse on iPad #1988

jamessampford opened this issue Jun 17, 2021 · 8 comments · Fixed by #2327
Labels
device/mobile Any issues relating to mobile devices type/bug Any issue which is a bug or PR which fixes a bug
Milestone
2.9.0

Comments

@jamessampford
Copy link
Contributor

Bug Report

It appears that when using a mouse (in my case a trackpad) on an iPad, the slider cannot not move when trying to either drag the slider along not when clicking at a position along the slider. At least, this appears to be the case when using iPad OS 14.6. When touching the screen, the slider works as expected

Steps to reproduce

  1. Try adjusting any slider on https://fomantic-ui.com/modules/slider.html

Expected result

Slider slides

Actual result

Slider does not slide

Version

2.8.7
@jamessampford jamessampford added state/awaiting-investigation Anything which needs more investigation state/awaiting-triage Any issues or pull requests which haven't yet been triaged type/bug Any issue which is a bug or PR which fixes a bug labels Jun 17, 2021
@lubber-de lubber-de removed the state/awaiting-triage Any issues or pull requests which haven't yet been triaged label Jun 17, 2021
@jamessampford jamessampford mentioned this issue Jun 17, 2021
Closed
@lubber-de
Copy link
Member

The iPad still gets recognized as a touch device, thus the mouse events are not bound (which the trackpad/mouse would trigger) . Without being able to test this myself, this is most probably the reason.

@lubber-de lubber-de added the device/mobile Any issues relating to mobile devices label Jun 17, 2021
@jamessampford
Copy link
Contributor Author

I imagine there may be a similar issue for other devices, like a Windows laptop that has a touchscreen, if this is the case? Presumably would be the same on Android if pairing a Bluetooth mouse - I’d test this, but have no Bluetooth mouse

@mhthies mhthies mentioned this issue Jun 22, 2021
Closed
@Warzulus
Copy link

I can confirm the same behavior on several Android devices using a OTG adapter and USB mouse. This is also true for dropdowns.
#1989

@mhthies
Copy link
Contributor

mhthies commented Apr 16, 2022

The issue is caused by the distinction between touch and non-touch devices (see slider.js, line 239): On touch devices, only (!) touch events are handled, on non-touch devices (only) mouse events are handled.

I'm currently not sure why the code does this distinction at all. Instead it could only bind to mouse events and rely on the (typically very good) mouse event emulation of the web browsers. I don't see any touch specific functionality, which requires the touch event handlers. So IMHO, the trivial solution would be removing this whole distinction and all the touch event binding code.

@lubber-de
Copy link
Member

lubber-de commented Apr 16, 2022 •
edited
Loading

The touch events are needed, otherwise real mobile devices do not trigger the mousedown/enter/leave events (which are complained by this issue) and the slider wont work at all on (real) mobile devices.
The mouse events are added all the time, that's why i can only guess, that the mobile browsers somehow do not detect an attached mouse (and wont trigger the mouse events)

I dont have an ipad nor a touchscreen notebook to test this myself. But we cannot purely rely on the desktop browser mobile view emulation.

@mhthies
Copy link
Contributor

mhthies commented Apr 16, 2022

The mouse events are added all the time, that's why i can only guess, that the mobile browsers somehow do not detect an attached mouse (and wont trigger the mouse events)

Unfortunately, not all of the events are always added (Sorry, my fault, I pointed to the wrong line): The mousedown event is always added (even on touch devices), but the mousemove and mouseup events are only added on non-touch devices, see line 280 And I would guess, that's the cause of the issue.

The touch events are needed, otherwise real mobile devices do not trigger the mousedown/enter/leave events (which are complained by this issue) and the slider wont work at all on (real) mobile devices.

This is not quite right. As I mentioned above, (mobile and desktop) web browsers have a built-in mouse emulation that triggers some mouse events along with touch events (see this MDN article) – as long as this is not prevented by event.preventDefault() during the touch event handling. However, I just noticed that his seems to only support "tap" events (click/mousedown→mouseup), no dragging (mousemove) – at least in Google Chrome on Android. So for dragging support, the touch events actually need be handled explicitly.

I dont have an ipad nor a touchscreen notebook to test this myself. But we cannot purely rely on the desktop browser mobile view emulation.

I can reproduce the issue on my Android smartphone with a Bluetooth mouse connected.
I also tested hardcoding module.is.touch = false, which resulted in working mouse interaction but broken touch dragging on my Android smartphone – which is expected, based on my thoughts above.

@lubber-de lubber-de added the tag/help-wanted Issues which need help to fix or implement label Apr 16, 2022
@mhthies
Copy link
Contributor

mhthies commented Apr 16, 2022 •
edited
Loading

I propose to

  • remove the distinction between touch and non-touch devices
  • remove the current touch handling
  • add new touch handling, only listening for touchdown events on the handle (.thumb ) to allow dragging the handle on touch devices without the accidential dragging issue (which I tried to tackle in [slider] Add option to disable touch-/mouse-sliding on the track #1987)

Touch-tapping on the slide track would be handled through the click event emulation, mouse dragging would continue to work (other than in my approach in the PR).

mhthies added a commit to mhthies/Fomantic-UI that referenced this issue Apr 17, 2022
@mhthies
[slider] Remove touch event binding
edf1455 
This fixes fomantic#1988 (mouse interaction on touch devices) but disables
dragging the slider via touch input. Touch-tapping for slider
positioning still works through browsers' mouse event emulation.
Touch-dragging will be re-added in a modified form in a later commit.
@mhthies mhthies mentioned this issue Apr 17, 2022
Merged
@lubber-de lubber-de added state/has-pr An issue which has a related PR open and removed tag/help-wanted Issues which need help to fix or implement state/awaiting-investigation Anything which needs more investigation labels Apr 18, 2022
@lubber-de lubber-de added this to the 2.9.0 milestone Apr 18, 2022
@lubber-de
Copy link
Member

Fixed by #2327

lubber-de pushed a commit that referenced this issue Apr 18, 2022
@mhthies
fix(slider): support mouse & touch all the time
f964821 
As proposed in #1988, I did a rework of the touch event handling of the slider component. The new touch event code
does not prevent mouse input (fixes [slider] cannot move slider using mouse on iPad #1988)
tracks Touch identifier to avoid confusion by other fingers on the touch screen
handles touchcancel events in a sensible way
does no dynamic binding and unbinding to all touch events on the page. In combination with the second point, this enables multitouch sliding of multiple sliders on the same page (I know, nobody asked for this, but it works. ;) ) – Unfortunately, multitouch sliding of the first and second thumb of the same slider is not easy to implement due global state within each slider module.
only binds to touchstart events on the .thumb element to avoid accidential sliding while scrolling the page on a mobile device (obsoletes [slider] Add option to disable touch-/mouse-sliding on the track #1987)
The last point can be considered a disadvantage/regression compared to the old code, if you consider touch-sliding anywhere on the slider as a feature. However, in my experience, it's really annoying to change a slider position (which has immediate real-world effects in my home automation software) when scrolling the page and accidentially hitting a slider. Using the mouse, one can still slide the slider anywhere on its track.
@lubber-de lubber-de added tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build and removed state/has-pr An issue which has a related PR open labels Apr 18, 2022
@lubber-de lubber-de removed the tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build label Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
device/mobile Any issues relating to mobile devices type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Milestone
2.9.0
Development

Successfully merging a pull request may close this issue.

fix(slider): support mouse & touch all the time mhthies/Fomantic-UI
4 participants
@jamessampford @mhthies @lubber-de @Warzulus