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

bug: ion-range should not update value or lock scrolling until touchEnd or drag within the bar #28487

Closed
3 tasks done
cmaas opened this issue Nov 7, 2023 · 5 comments · Fixed by #29005
Closed
3 tasks done
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@cmaas
Copy link

cmaas commented Nov 7, 2023

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

If you click/tap on an ion-range component somewhere on the bar (not on the knob), the knob will immediately jump to the place where you clicked or touched down. See video. The circle indicates the beginning of a click (onTouchStart):

CleanShot 2023-11-07 at 21 05 54

This is "too sensitive", because it causes accidentally changing the ion-range value if the user just wants to scroll in some cases. In my app, there are several ion-range elements further down on the screen that get accidentally changed if the user merely wants to scroll.

On native iOS 17 and Android 14 UI elements, the behavior is different: if you touch down on the knob, you can start dragging it. But if you touch down on the bar somewhere else, the knob will move there only "onTouchEnd" when you lift your finger. This reduces the sensitivity of this element and prevents accidents. Furthermore, the value of the element will only be changed if you lift your finger while it's still hovering over it. If you touch down on the bar and move your finger somewhere else while keeping it touched down, the parent view starts to scroll vertically (most of the time at least).

The behavior is pretty consistent between native iOS and Android elements and feels quite off in Ionic.

Expected Behavior

ion-range should mimic native behavior and trigger value adjustments only "onTouchEnd" and only, if the finger is still over the component. If the user touches down on the bar, but moves the finger to another place on the screen and lifts the finger there, ion-range value should not be changed.

Steps to Reproduce

  1. Sample code that gives you some vertical scrolling around ion-range.
  2. Consider the audio settings on a real iOS or Android device to get a feeling for how a native range input should behave.

Code Reproduction URL

Here

Ionic Info

Latest via CDN

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Nov 7, 2023
@amandaejohnston amandaejohnston self-assigned this Nov 9, 2023
@amandaejohnston
Copy link
Contributor

amandaejohnston commented Nov 9, 2023

Thank you for the report! Looking into this, I agree that the experience could be improved. I've tested with the Volume settings on my Android device (video below) and come to the same conclusions as you. To summarize:

  1. If the bar is tapped outside the knob, then released without dragging, the value only updates on touch end. (Ionic currently updates immediately on touch start.)
  2. If the bar is tapped, but not dragged, the view can still be scrolled. (Ionic currently locks scrolling immediately on touch start.) At this point, if the touch is lifted while no longer on the range, the value is not changed.
  3. If the bar is tapped, then dragged, the view cannot be scrolled until the knob is released. (This matches current Ionic behavior.)

This isn't shown in the video, but I noticed that item 2 also applies when tapping directly on the knob.

2023-11-09.09-32-12.mp4

@amandaejohnston amandaejohnston changed the title bug: ion-range is too sensitive and should not trigger "onTouchStart" when clicking outside the knob bug: ion-range should not update value or lock scrolling until touchEnd or drag within the bar Nov 9, 2023
@amandaejohnston amandaejohnston added package: core @ionic/core package type: bug a confirmed bug report and removed triage labels Nov 9, 2023
@amandaejohnston amandaejohnston removed their assignment Nov 9, 2023
@fudom
Copy link

fudom commented Jan 19, 2024

This behavior makes the ion-range component impossible to use in a scrolling context. You can only use it on a static page or dialog where you are sure you will never be able to scroll. Otherwise, you'll provide a really bad UI/UX that will result in angry users. This component does not appear to be suitable for mobile devices with touch, as the rest of the framework suggests. No offense. Native HTML range input has similar behavior and changes values when scrolling. At least in an Ionic Cordova app on Android. But the native range slider (not HTML) works better for mobile device. I'm not sure how to implement it exactly. We could reverse engineer it by observing the behavior. For example as we see in the video above. It would be great if we could fix that bug.

Ionic 6: Demo
Ionic 7: Demo

Tested in mobile device view of Chrome DevTools.

demo

Read more: #28844

@domske
Copy link
Contributor

domske commented Feb 9, 2024

I created a range slider that is optimized for touch. Matching the behavior shown in the video of @amandaejohnston (post). Should we change the range component in this way? It's maybe too special (not MD). With my solution, you can't even move the knob without long-pressing it, which I personally prefer. Anyway, it's nice to see a PR on this issue. I haven't checked that yet. Currently no time. However, my change would be more of a redesign than a small bug fix. We could offer the option to only allow interactions via long press. Whatever solution we choose, it should be optimized for mobile devices (touch), like Ionic is. Scrolling should be safe, even if the user touches the element while scrolling.

Here is a Demo if you want to see it in action. Tested on Android and Chrome DevTools mobile view (touch).

demo

github-merge-queue bot pushed a commit that referenced this issue Mar 6, 2024
Issue number: resolves #28487

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

There are two behaviors that need to be addressed.

1. The range value is updated when the gesture `onStart` event is fired.
This can lead to the values being accidentally updated when the user is
scrolling on the view.
The user might tap on the range to scroll on the view, but the range
value is updated instead.

2. The component prevents the view from scrolling while the user has
touched any part of the range.
The user might want to scroll and they happen to touch the range. This
can lead to the user feeling disoriented because they can't scroll on
the view anymore.

These behaviors do not follow the native behavior of mobile devices.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- The range value is updated on touch end or when the knob is being
dragged.
- The view can be scrolled while the user is not dragging the knob.
- A new variable `isScrollingView` is used to determine if the user is
scrolling on the view regardless of whether the user is dragging the
knob or not. This determines what logic to apply.
- The `pressedKnob` variable is no longer being set in the `onStart`
event. It is now being set in the `onMove` and `onEnd` events. (the
reason behind this can be found within the newly added comments)
- The `initialContentScrollY` variable is no longer being set in the
`onStart` event. It is now being set in the `onMove` event. (the reason
behind this can be found within the newly added comments)

I did not change the behavior of the range when the user is dragging the
knob. The view should not scroll while the user is dragging the knob.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

The new behavior aligns with the native mobile devices.
@thetaPC
Copy link
Contributor

thetaPC commented Mar 6, 2024

Thanks for the issue! This has been resolved via #29005 and will be available in an upcoming release of Ionic.

Copy link

ionitron-bot bot commented Apr 5, 2024

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Apr 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants