ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.93k stars 13.52k forks source link

bug: scrolling should not cause range to move #19004

Closed charlesj1 closed 2 years ago

charlesj1 commented 5 years ago

Ionic version: [x] 4.x

I'm submitting a ... [x] bug report [ ] feature request

Current behavior: If I touch an ion-range while doing a vertical swipe (.e.g. to scroll down the page), the knob of the ion-range changes position

Expected behavior: The knob of the ion-range should only change position if I tap the slider, or drag along the slider. It should be unaffected if I just happen to touch the slider while doing a vertical swipe, since the intent is clearly to scroll down the page, not to interact with the slider. This is in line with how native apps function.

Steps to reproduce: Go to any page with an on it and press the slider and make a vertical swipe gesture. The knob will immediately move to the new position, even though the gesture is clearly not intended to do this.

Ionic info: I'm on a stencil site with: "@ionic/core": "4.7.1" "@stencil/core": "^1.1.9" I observe the same behaviour on the official docs page: https://ionicframework.com/docs/api/range

liamdebeasi commented 5 years ago

Thanks for the issue. Can you clarify how to reproduce this issue? I am not seeing this on my end.

charlesj1 commented 5 years ago

For example, spin up a page with a whole bunch of ion-range elements. Then on a mobile device, try to scroll up and down the page. You'll find that you keep accidentally moving the slider knobs all over the place every time your finger touches one of the sliders. `

`

I would expect the element to be smart enough to know that a scroll is taking place and the user is not trying to move the slider knob. For example, the slider built into Android knows that...if I go to the display page in Android's settings, which contains a brightness slider, I can scroll that page up and down and it never mistakenly moves the brightness slider, even if my finger touches it.

Om3s commented 5 years ago

I'm facing the same issue, @charlesj1 did you find a workaround for this problem?

charlesj1 commented 5 years ago

@Om3s I didn't find a workaround. It's quite problematic for UIs like mine that have a whole list of sliders to be adjusted because it's easy for users to inadvertently move a slider knob without realizing it while vertically scrolling.

Polymer's equivalent (paper-slider) handles it better. Their approach is to disable the intended vertical scroll altogether if the touch starts on a slider. It still moves the slider knob, which is not ideal, but at least it's more obvious to the user what has just happened. The native Android/iOS elements handle it the best though (performing the intended scroll but not moving the slider knob)

I was waiting to see if Ionic would reply, and otherwise I plan to switch to the Polymer element. I don't know enough to implement a fix to ion-range myself. Let me know if you find anything!

Ruuudi commented 4 years ago

We have the same problem. In previous versions of ionic you could fix this behavior with the following code.

.range-slider {
    pointer-events: none;
}

.range-knob-handle {
    pointer-events: auto;
}

With ionic 4 and WebComponents this is no longer possible. Anyone have an idea how to solve the problem anyway?

TakkuzOld commented 4 years ago

Really thank you @Ruuudi. I've left ion-range to use ng5-slider, which is an Angular Component so I can style it with ::ng-deep and your workaround works.

charlesj1 commented 4 years ago

A workaround I've been using is to disable ion-content scroll when an ion-range fires the touchStart event, and re-enable it when the touchEnd event is fired. This means that if they accidentally touch the ion-range while attempting to scroll, it will move the ion-range knob, but the vertical scrolling will stop and so hopefully they will realize what they've done.

I think an even better experience would be to allow the vertical scroll but not move the ion-range knob. But I'm not sure how to achieve that.

nunoarruda commented 4 years ago

@liamdebeasi this is what @charles1 means:

If you scroll/swipe vertically while also touching an ion-range it will trigger an accidental input/change.

If you're using an Ionic version that already provides CSS Shadow Parts then, based on @Ruuudi comment, you can use a workaround like:

Angular template:

<ion-range [ngClass]="{'custom-slider': isCSSshadowPartsSupported}"></ion-range>

CSS:

.custom-slider {
  pointer-events: none;
}

.custom-slider::part(knob),
.custom-slider::part(pin) {
  pointer-events: auto;
}

Class

public isCSSshadowPartsSupported = 'part' in HTMLElement.prototype;

But there's a catch, you lose the ability to "tap-to-change", only dragging the knob and pin will be possible.

I guess, ideally, we would want to avoid changing the ion-range when a vertical swipe is detected. That might be possible to achieve with Ionic's Gestures utility or Hammer.js.

Update: Added animated gif showing the issue. Also added feature detection so it doesn't break browsers that do not support CSS Shadow Parts.

mcastets commented 3 years ago

Unfortunately, we're also experiencing this issue on mobile devices... Any progress?

liamdebeasi commented 2 years ago

Thanks for the issue. I can reproduce this behavior. The range slider should prevent scrolling when swiping on the range slider and should not change when actually scrolling.

liamdebeasi commented 2 years ago

Hi everyone,

Here is a dev build with a proposed fix if anyone is interested in testing. Let me know if you encounter any errors with the fix.

6.1.7-dev.11653426403.141ccc66

Note: This dev build applies to all packages (@ionic/angular, @ionic/react, etc).

Example: npm install @ionic/angular@6.1.7-dev.11653426403.141ccc66

liamdebeasi commented 2 years ago

Thanks for the issue. This has been resolved via https://github.com/ionic-team/ionic-framework/pull/25343, and a fix will be available in an upcoming release of Ionic Framework. Please feel free to continue testing the dev build included in https://github.com/ionic-team/ionic-framework/issues/19004#issuecomment-1136436196.

ionitron-bot[bot] commented 2 years ago

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.