[slider] Add option to disable touch-/mouse-sliding on the track #1987

mhthies · 2021-06-17T18:52:15Z

Description

Currently, the slider is quite sensible to mouse and touch events: Any mouse drag and touch event that starts anywhere on the slider will move the knob (not only when hitting the knob). This is especially annoying when trying to scroll a page with many sliders on a mobile device with touch screen. You'll find yourself moving a slider accidentially quite often.

Thus, this PR introduces a setting slideEverywhere to sliders to change the click and touch behaviour. By default, it is true, which corresponds to the old behaviour. With slideEverywhere=true, the slider can only be moved by either dragging the knob or by clicking / tapping (not dragging) anywhere on the slider.

Testcase

https://jsfiddle.net/81cs7dbg/1/

Screenshot (if possible)

Does not change anything visually.

lubber-de

Here is a testcase using your latest code
Try this on real mobile devices (i did on my Android Xperia 5ii)
https://fomantic-ui.com/jsfiddle/#!lubber/thr6dvLp/1/

The current code does not seem to work on real mobile devices at all. No matter if i touch or swipe on the track: It is always doing the same for me (as before: changing the slider immediatly)

It is also not completely working as expected when using on desktop browsers:
https://jsfiddle.net/lubber/thr6dvLp/1/
The mouse event does indeed not change the slider on mousedown and drag... but when i release the mouse button after the drag, the slider thumb still gets repositioned to where i released the mouse button. I think your intention was to not change the thumb at all then.

Desktop

Modile (using Browserstack on a Samsung Galaxy S21)

src/definitions/modules/slider.js

mhthies · 2021-06-19T11:29:56Z

The current code does not seem to work on real mobile devices at all. No matter if i touch or swipe on the track: It is always doing the same for me (as before: changing the slider immediatly)

Oh, yes, you are right. This is due to the logics screwup in line 286 that you already commented on. I actually used the new setting the wrong way round for touch events on the $module. This is easy to fix.

However, the click events are still not triggered on touch devices. I'll try to find a solution for this.

It is also not completely working as expected when using on desktop browsers:
https://jsfiddle.net/lubber/thr6dvLp/1/
The mouse event does indeed not change the slider on mousedown and drag... but when i release the mouse button after the drag, the slider thumb still gets repositioned to where i released the mouse button. I think your intention was to not change the thumb at all then.

This is due to the definition of the ´click` event. From MDN:

An element receives a click event when a pointing device button (such as a mouse's primary mouse button) is both pressed and released while the pointer is located inside the element.

The only way to prevent this, would be implementing a custom "click detection" which uses move events to cancel a click when moving more than a fixed pixel delta from the original click position. I could do this, but I don't think it's worth the additional required code.

lubber-de

Here is a jsfiddle with your latest code.
Unfortunately it is completely broken on desktop for when slideEverywhere is true (which is default and would break every existing code)
https://jsfiddle.net/lubber/thr6dvLp/2/
The first example does not work at all anymore on desktop and throws a console error when you click/hover on the slider

On mobile i can still drag from inside the track in the second example.
https://fomantic-ui.com/jsfiddle/#!lubber/thr6dvLp/2/

mhthies · 2021-06-20T11:42:46Z

Ah, sh*t. I'm sorry for wasting your time with testing my broken patches.
The exceptions on non-touch devices should be fixed now.

Unfortunately, I cannot reproduce the dragging issue in the second example. I tested the latest version locally and the previous version with your JS fiddle link on Chrome mobile 91.0.4472.101 on Android, Firefox mobile 89.1.1 on Android and Safari mobile on iOS. On all three browsers, the behaviour is as intended.

lubber-de

New jsfiddle using your latest commit:
https://jsfiddle.net/lubber/thr6dvLp/3/
https://fomantic-ui.com/jsfiddle/#!lubber/thr6dvLp/3/

Please see my question about the different event name

Also, the setting still does not work on desktop as mentioned. When releasing the mousebutton the thumb still gets repositioned even if dragged from within the track. @fomantic/maintainers Your opinion?

src/definitions/modules/slider.js

mhthies · 2021-06-22T15:11:35Z

Also, the setting still does not work on desktop as mentioned. When releasing the mousebutton the thumb still gets repositioned even if dragged from within the track.

As I explained above:

This is due to the definition of the click event. […]

The only way to prevent this, would be implementing a custom "click detection" which uses move events to cancel a click when moving more than a fixed pixel delta from the original click position. I could do this, but I don't think it's worth the additional required code.

lubber-de · 2021-06-22T15:18:02Z

Yes, that's why i wanted the other maintainers to have an opinion if/how we deal with that. I otherwise smell people complaining the feature does not work on desktop...just saying 😉
I still think there is a possibility to fix/fetch this without implementing a complicated pixel detection mechanism. For example remember the DOM node on mousedown (event.target) (was is "thumb" or something else?) and then use that infomation to deny the thumb move on mouse up again.

mhthies · 2021-06-22T15:24:23Z

Sorry, I misinterpreted that as a criticism on my fixes.

Another compromise could be to restrict the new setting on touch events. I.e. with slideEverywhere == false, mouse interaction would stay the same as before but touch events would not allow dragging on the track. However, the trivial approach to do this would interfere with the solutions for #1988 that I have currently in mind.

lubber-de · 2021-06-22T15:38:34Z

Sorry, I misinterpreted that as a criticism on my fixes.

Wasn't my intention, sorry, if you felt so. 🤗

Another compromise could be to restrict the new setting on touch events. I.e. with slideEverywhere == false, mouse interaction would stay the same as before but touch events would not allow dragging on the track. However, the trivial approach to do this would interfere with the solutions for #1988 that I have currently in mind.

Oh, you are thinking about providing a solution for those two issues (#1988 and #1989 ) ? 🥰 Great! 👍🏼 Then i suggest to think about an overall solution which also possibly fixes/covers those two issues as well before merging this PR (to avoid having the need to revert something later)

prudho · 2021-06-23T06:02:56Z

Mmmmmh... Can't we determine if pointer get out of the component zone, and then cancel the movement ? This seem to be a delicate issue tbh.

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 · 2022-04-18T22:07:44Z

Since #2327 is merged, i think we can close this?

lubber-de · 2022-04-19T08:32:53Z

I made a simple replacement PR #2330 based on the current code. Please check

feat(slider): Allow disabling touch-/mouse-sliding on the track

b55306c

auto-assign bot requested review from ColinFrick, exoego, y0hami, ko2in, lubber-de and prudho June 17, 2021 18:52

fix(slider): Fix reference to slideEverywhere setting

fe02b89

lubber-de requested changes Jun 17, 2021

View reviewed changes

src/definitions/modules/slider.js Outdated Show resolved Hide resolved

lubber-de added lang/javascript Anything involving JavaScript state/awaiting-reviews Pull requests which are waiting for reviews type/feat Any feature requests or improvements labels Jun 17, 2021

lubber-de added this to the 2.8.x milestone Jun 17, 2021

fix(slider): Fix touch event handling with \!slideEverywhere

b48c4f6

lubber-de requested changes Jun 20, 2021

View reviewed changes

fix(slider): Fix broken eventPos function for mouse events

d2df3c1

lubber-de requested changes Jun 22, 2021

View reviewed changes

src/definitions/modules/slider.js Show resolved Hide resolved

lubber-de added the state/on-hold Issues and pull requests which are on hold for any reason label Jul 9, 2021

lubber-de modified the milestones: 2.8.x, 2.9.x Sep 6, 2021

lubber-de removed the state/awaiting-reviews Pull requests which are waiting for reviews label Feb 1, 2022

lubber-de removed this from the 2.9.x milestone Feb 1, 2022

mhthies mentioned this pull request Apr 16, 2022

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

Closed

mhthies mentioned this pull request Apr 17, 2022

fix(slider): support mouse & touch all the time #2327

Merged

mhthies closed this Apr 19, 2022

lubber-de mentioned this pull request Apr 19, 2022

feat(slider): customizable event selectors #2330

Draft

lubber-de removed the state/on-hold Issues and pull requests which are on hold for any reason label Apr 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[slider] Add option to disable touch-/mouse-sliding on the track #1987

[slider] Add option to disable touch-/mouse-sliding on the track #1987

mhthies commented Jun 17, 2021

lubber-de left a comment •

edited

Loading

mhthies commented Jun 19, 2021 •

edited

Loading

lubber-de left a comment •

edited

Loading

mhthies commented Jun 20, 2021 •

edited

Loading

lubber-de left a comment

mhthies commented Jun 22, 2021

lubber-de commented Jun 22, 2021 •

edited

Loading

mhthies commented Jun 22, 2021

lubber-de commented Jun 22, 2021

prudho commented Jun 23, 2021

lubber-de commented Apr 18, 2022

lubber-de commented Apr 19, 2022

[slider] Add option to disable touch-/mouse-sliding on the track #1987

[slider] Add option to disable touch-/mouse-sliding on the track #1987

Conversation

mhthies commented Jun 17, 2021

Description

Testcase

Screenshot (if possible)

lubber-de left a comment • edited Loading

Choose a reason for hiding this comment

Desktop

Modile (using Browserstack on a Samsung Galaxy S21)

mhthies commented Jun 19, 2021 • edited Loading

lubber-de left a comment • edited Loading

Choose a reason for hiding this comment

mhthies commented Jun 20, 2021 • edited Loading

lubber-de left a comment

Choose a reason for hiding this comment

mhthies commented Jun 22, 2021

lubber-de commented Jun 22, 2021 • edited Loading

mhthies commented Jun 22, 2021

lubber-de commented Jun 22, 2021

prudho commented Jun 23, 2021

lubber-de commented Apr 18, 2022

lubber-de commented Apr 19, 2022

lubber-de left a comment •

edited

Loading

mhthies commented Jun 19, 2021 •

edited

Loading

lubber-de left a comment •

edited

Loading

mhthies commented Jun 20, 2021 •

edited

Loading

lubber-de commented Jun 22, 2021 •

edited

Loading