diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 1df4ed9c3f5..d8904a2f548 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -299,8 +299,14 @@ export class Range implements ComponentInterface { el: rangeSlider, gestureName: 'range', gesturePriority: 100, - threshold: 0, - onStart: (ev) => this.onStart(ev), + /** + * Provide a threshold since the drag movement + * might be a user scrolling the view. + * If this is true, then the range + * should not move. + */ + threshold: 10, + onStart: () => this.onStart(), onMove: (ev) => this.onMove(ev), onEnd: (ev) => this.onEnd(ev), }); @@ -418,42 +424,101 @@ export class Range implements ComponentInterface { this.ionChange.emit({ value: this.value }); } - private onStart(detail: GestureDetail) { - const { contentEl } = this; - if (contentEl) { - this.initialContentScrollY = disableContentScrollY(contentEl); - } + /** + * The value should be updated on touch end or + * when the component is being dragged. + * This follows the native behavior of mobile devices. + * + * For example: When the user lifts their finger from the + * screen after tapping the bar or dragging the bar or knob. + */ + private onStart() { + this.ionKnobMoveStart.emit({ value: this.ensureValueInBounds(this.value) }); + } - const rect = (this.rect = this.rangeSlider!.getBoundingClientRect() as any); + /** + * The value should be updated while dragging the + * bar or knob. + * + * While the user is dragging, the view + * should not scroll. This is to prevent the user from + * feeling disoriented while dragging. + * + * The user can scroll on the view if the knob or + * bar is not being dragged. + * + * @param detail The details of the gesture event. + */ + private onMove(detail: GestureDetail) { + const { contentEl, pressedKnob } = this; const currentX = detail.currentX; - // figure out which knob they started closer to - let ratio = clamp(0, (currentX - rect.left) / rect.width, 1); - if (isRTL(this.el)) { - ratio = 1 - ratio; + /** + * Since the user is dragging on the bar or knob, the view should not scroll. + * + * This only needs to be done once. + */ + if (contentEl && this.initialContentScrollY === undefined) { + this.initialContentScrollY = disableContentScrollY(contentEl); } - this.pressedKnob = !this.dualKnobs || Math.abs(this.ratioA - ratio) < Math.abs(this.ratioB - ratio) ? 'A' : 'B'; - - this.setFocus(this.pressedKnob); + /** + * The `pressedKnob` can be undefined if the user just + * started dragging the knob. + * + * This is necessary to determine which knob the user is dragging, + * especially when it's a dual knob. + * Plus, it determines when to apply certain styles. + * + * This only needs to be done once since the knob won't change + * while the user is dragging. + */ + if (pressedKnob === undefined) { + this.setPressedKnob(currentX); + } - // update the active knob's position this.update(currentX); - - this.ionKnobMoveStart.emit({ value: this.ensureValueInBounds(this.value) }); } - private onMove(detail: GestureDetail) { - this.update(detail.currentX); - } - - private onEnd(detail: GestureDetail) { + /** + * The value should be updated on touch end: + * - When the user lifts their finger from the screen after + * tapping the bar. + * + * @param detail The details of the gesture or mouse event. + */ + private onEnd(detail: GestureDetail | MouseEvent) { const { contentEl, initialContentScrollY } = this; - if (contentEl) { + const currentX = (detail as GestureDetail).currentX || (detail as MouseEvent).clientX; + + /** + * The `pressedKnob` can be undefined if the user never + * dragged the knob. They just tapped on the bar. + * + * This is necessary to determine which knob the user is changing, + * especially when it's a dual knob. + * Plus, it determines when to apply certain styles. + */ + if (this.pressedKnob === undefined) { + this.setPressedKnob(currentX); + } + + /** + * The user is no longer dragging the bar or + * knob (if they were dragging it). + * + * The user can now scroll on the view in the next gesture event. + */ + if (contentEl && initialContentScrollY !== undefined) { resetContentScrollY(contentEl, initialContentScrollY); } - this.update(detail.currentX); + // update the active knob's position + this.update(currentX); + /** + * Reset the pressed knob to undefined since the user + * may start dragging a different knob in the next gesture event. + */ this.pressedKnob = undefined; this.emitValueChange(); @@ -485,6 +550,19 @@ export class Range implements ComponentInterface { this.updateValue(); } + private setPressedKnob(currentX: number) { + const rect = (this.rect = this.rangeSlider!.getBoundingClientRect() as any); + + // figure out which knob they started closer to + let ratio = clamp(0, (currentX - rect.left) / rect.width, 1); + if (isRTL(this.el)) { + ratio = 1 - ratio; + } + this.pressedKnob = !this.dualKnobs || Math.abs(this.ratioA - ratio) < Math.abs(this.ratioB - ratio) ? 'A' : 'B'; + + this.setFocus(this.pressedKnob); + } + private get valA() { return ratioToValue(this.ratioA, this.min, this.max, this.step); } @@ -799,7 +877,39 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop } return ( -
(this.rangeSlider = rangeEl)}> +
(this.rangeSlider = rangeEl)} + /** + * Since the gesture has a threshold, the value + * won't change until the user has dragged past + * the threshold. This is to prevent the range + * from moving when the user is scrolling. + * + * This results in the value not being updated + * and the event emitters not being triggered + * if the user taps on the range. This is why + * we need to listen for the "pointerUp" event. + */ + onPointerUp={(ev: PointerEvent) => { + /** + * If the user drags the knob on the web + * version (does not occur on mobile), + * the "pointerUp" event will be triggered + * along with the gesture's events. + * This leads to duplicate events. + * + * By checking if the pressedKnob is undefined, + * we can determine if the "pointerUp" event was + * triggered by a tap or a drag. If it was + * dragged, the pressedKnob will be defined. + */ + if (this.pressedKnob === undefined) { + this.onStart(); + this.onEnd(ev); + } + }} + > {ticks.map((tick) => (
expect(rangeEnd).toHaveReceivedEventDetail({ value: 21 }); }); + test('should emit end event on tap', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/28487', + }); + + await page.setContent(``, config); + + const range = page.locator('ion-range'); + const rangeEndSpy = await page.spyOnEvent('ionKnobMoveEnd'); + const rangeBoundingBox = await range.boundingBox(); + /** + * Coordinates for the click event. + * These need to be near the end of the range + * (or anything that isn't the current value). + * + * The number 50 is arbitrary, but it should be + * less than the width of the range. + */ + const x = rangeBoundingBox!.width - 50; + // The y coordinate is the middle of the range. + const y = rangeBoundingBox!.height / 2; + + // Click near the end of the range. + await range.click({ + position: { x, y }, + }); + + await rangeEndSpy.next(); + + expect(rangeEndSpy.length).toBe(1); + }); + // TODO FW-2873 test.skip('should not scroll when the knob is swiped', async ({ page, skip }) => { skip.browser('webkit', 'mouse.wheel is not available in WebKit');