From e4ab6e491ca836050264741fb0d2af1f319d7397 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Thu, 8 Feb 2024 14:08:59 -0800 Subject: [PATCH 01/10] refactor(range): update value on touchEnd or drag --- core/src/components/range/range.tsx | 74 +++++++++++++++++++++-------- 1 file changed, 55 insertions(+), 19 deletions(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 1df4ed9c3f5..9796a6be1fc 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -60,6 +60,7 @@ export class Range implements ComponentInterface { private initialContentScrollY = true; private originalIonInput?: EventEmitter; private legacyFormController!: LegacyFormController; + private isScrolling = false; // This flag ensures we log the deprecation warning at most once. private hasLoggedDeprecationWarning = false; @@ -419,41 +420,64 @@ export class Range implements ComponentInterface { } private onStart(detail: GestureDetail) { - const { contentEl } = this; - if (contentEl) { - this.initialContentScrollY = disableContentScrollY(contentEl); - } + console.log('onStart', detail); - const rect = (this.rect = this.rangeSlider!.getBoundingClientRect() as any); + this.ionKnobMoveStart.emit({ value: this.ensureValueInBounds(this.value) }); + } + + private onMove(detail: GestureDetail) { + const { contentEl } = this; + const deltaY = detail.deltaY; 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; + // dont proceed if the user is scrolling on the page + const isScrolling = Math.abs(deltaY) > Math.abs(detail.deltaX); + if (isScrolling) { + console.log('isScrolling', isScrolling); + this.isScrolling = true; + this.pressedKnob = undefined; + return; } - this.pressedKnob = !this.dualKnobs || Math.abs(this.ratioA - ratio) < Math.abs(this.ratioB - ratio) ? 'A' : 'B'; + if (this.pressedKnob === undefined) { + this.setPressedKnob(currentX); + } - this.setFocus(this.pressedKnob); + // the page should not scroll when dragging the range slider + if (contentEl && this.initialContentScrollY === undefined) { + this.initialContentScrollY = disableContentScrollY(contentEl); + } - // update the active knob's position + console.log('onMove', detail); this.update(currentX); - - this.ionKnobMoveStart.emit({ value: this.ensureValueInBounds(this.value) }); - } - - private onMove(detail: GestureDetail) { - this.update(detail.currentX); } private onEnd(detail: GestureDetail) { + console.log('onEnd', detail); const { contentEl, initialContentScrollY } = this; + const currentX = detail.currentX; + if (contentEl) { resetContentScrollY(contentEl, initialContentScrollY); } - this.update(detail.currentX); + // the user is done scrolling on the page + if (this.isScrolling) { + this.isScrolling = false; + return; + } + + // The pressed knob can be undefined if the user doesn't drag the knob + if (this.pressedKnob === undefined) { + this.setPressedKnob(currentX); + } + + this.setFocus(this.pressedKnob); + + // update the active knob's position + this.update(currentX); + + // this.update(detail.currentX); this.pressedKnob = undefined; this.emitValueChange(); @@ -485,6 +509,18 @@ export class Range implements ComponentInterface { this.updateValue(); } + private setPressedKnob(currentX: number) { + // this.rect = this.rangeSlider!.getBoundingClientRect(); + 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'; + } + private get valA() { return ratioToValue(this.ratioA, this.min, this.max, this.step); } From 453a6948fe370695f71b2b3ca99f998784d2ba31 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Fri, 9 Feb 2024 11:05:33 -0800 Subject: [PATCH 02/10] refactor(range): cleanup --- core/src/components/range/range.tsx | 116 +++++++++++++++++++++------- 1 file changed, 88 insertions(+), 28 deletions(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 9796a6be1fc..ea0af6bfdc1 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -301,7 +301,7 @@ export class Range implements ComponentInterface { gestureName: 'range', gesturePriority: 100, threshold: 0, - onStart: (ev) => this.onStart(ev), + onStart: () => this.onStart(), onMove: (ev) => this.onMove(ev), onEnd: (ev) => this.onEnd(ev), }); @@ -419,65 +419,124 @@ export class Range implements ComponentInterface { this.ionChange.emit({ value: this.value }); } - private onStart(detail: GestureDetail) { - console.log('onStart', detail); - + /** + * The value should be updated on touch end. + * 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 knob. + */ + private onStart() { this.ionKnobMoveStart.emit({ value: this.ensureValueInBounds(this.value) }); } + /** + * The value should be updated on touch end: + * - When the user lifts their finger from the screen after + * dragging the knob. + * + * While the user is dragging the knob, 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 is not being dragged. + * + * @param detail The details of the gesture event. + */ private onMove(detail: GestureDetail) { - const { contentEl } = this; + const { contentEl, pressedKnob } = this; const deltaY = detail.deltaY; const currentX = detail.currentX; - // dont proceed if the user is scrolling on the page - const isScrolling = Math.abs(deltaY) > Math.abs(detail.deltaX); - if (isScrolling) { - console.log('isScrolling', isScrolling); + /** + * The user is scrolling on the view. + * When scrolling, the range should not be dragged. + */ + const isScrolling = deltaY != 0; + if (isScrolling && pressedKnob === undefined) { this.isScrolling = true; - this.pressedKnob = undefined; return; } - if (this.pressedKnob === undefined) { - this.setPressedKnob(currentX); - } - - // the page should not scroll when dragging the range slider + /** + * It's been determined that the user is not scrolling on the view. + * Since the user is dragging the knob, the view should not scroll. + * + * This only needs to be done once. + */ if (contentEl && this.initialContentScrollY === undefined) { this.initialContentScrollY = disableContentScrollY(contentEl); } - console.log('onMove', detail); + /** + * 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); + } + this.update(currentX); } + /** + * The value should be updated on touch end: + * - When the user lifts their finger from the screen after + * tapping the bar or dragging the knob. + * + * + * + * @param detail The details of the gesture event. + */ private onEnd(detail: GestureDetail) { - console.log('onEnd', detail); const { contentEl, initialContentScrollY } = this; const currentX = detail.currentX; - if (contentEl) { - resetContentScrollY(contentEl, initialContentScrollY); - } - - // the user is done scrolling on the page - if (this.isScrolling) { + /** + * The user is no longer scrolling on the view. + * + * The user can now scroll on the view or drag the range or knob + * in the next gesture event. + */ + if (this.isScrolling && this.pressedKnob === undefined) { this.isScrolling = false; return; } - // The pressed knob can be undefined if the user doesn't drag the knob + /** + * The `pressedKnob` can be undefined if the user never + * dragged the knob. They just clicked on the range. + * + * 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); } - this.setFocus(this.pressedKnob); + /** + * The user is no longer dragging the range or knob. + * + * The user can now scroll on the view in the next gesture event. + */ + if (contentEl && initialContentScrollY !== undefined) { + resetContentScrollY(contentEl, initialContentScrollY); + } // update the active knob's position this.update(currentX); - - // this.update(detail.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(); @@ -510,7 +569,6 @@ export class Range implements ComponentInterface { } private setPressedKnob(currentX: number) { - // this.rect = this.rangeSlider!.getBoundingClientRect(); const rect = (this.rect = this.rangeSlider!.getBoundingClientRect() as any); // figure out which knob they started closer to @@ -519,6 +577,8 @@ export class Range implements ComponentInterface { 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() { From a4524c2eda22ac39241681e15ef697e2717b91e0 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Fri, 9 Feb 2024 13:54:52 -0800 Subject: [PATCH 03/10] refactor(range): use threshold for smoother usage --- core/src/components/range/range.tsx | 32 +++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index ea0af6bfdc1..9a1aecb976d 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -60,7 +60,7 @@ export class Range implements ComponentInterface { private initialContentScrollY = true; private originalIonInput?: EventEmitter; private legacyFormController!: LegacyFormController; - private isScrolling = false; + private isScrollingView = false; // This flag ensures we log the deprecation warning at most once. private hasLoggedDeprecationWarning = false; @@ -447,14 +447,24 @@ export class Range implements ComponentInterface { const { contentEl, pressedKnob } = this; const deltaY = detail.deltaY; const currentX = detail.currentX; + /** + * Provide a threshold since the drag will not be + * smooth. The finger might move a few pixels up or down + * while dragging the knob. + */ + const threshold = 3; + const isScrollingView = Math.abs(deltaY) > threshold; /** - * The user is scrolling on the view. - * When scrolling, the range should not be dragged. + * The user is scrolling on the view while not dragging the knob. + * During this scenario, the view should scroll and + * the knob should not move. + * + * The user might be dragging the knob and then start + * scrolling on the view. This is why the `pressedKnob` is checked. */ - const isScrolling = deltaY != 0; - if (isScrolling && pressedKnob === undefined) { - this.isScrolling = true; + if (isScrollingView && pressedKnob === undefined) { + this.isScrollingView = true; return; } @@ -502,17 +512,17 @@ export class Range implements ComponentInterface { /** * The user is no longer scrolling on the view. * - * The user can now scroll on the view or drag the range or knob + * The user can now scroll on the view again or drag the knob * in the next gesture event. */ - if (this.isScrolling && this.pressedKnob === undefined) { - this.isScrolling = false; + if (this.isScrollingView) { + this.isScrollingView = false; return; } /** * The `pressedKnob` can be undefined if the user never - * dragged the knob. They just clicked on the range. + * 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. @@ -523,7 +533,7 @@ export class Range implements ComponentInterface { } /** - * The user is no longer dragging the range or knob. + * The user is no longer dragging the knob (if they were dragging it). * * The user can now scroll on the view in the next gesture event. */ From c0e4fa9784b834722e6540cda535fb4962c898ff Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Wed, 14 Feb 2024 10:02:20 -0800 Subject: [PATCH 04/10] refactor(range): use PointerEvent API --- core/src/components/range/range.tsx | 133 +++++++++------------------- 1 file changed, 41 insertions(+), 92 deletions(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 9a1aecb976d..f6edea399ad 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -10,7 +10,7 @@ import { isRTL } from '@utils/rtl'; import { createColorClasses, hostContext } from '@utils/theme'; import { getIonMode } from '../../global/ionic-global'; -import type { Color, Gesture, GestureDetail, StyleEventDetail } from '../../interface'; +import type { Color, Gesture, StyleEventDetail } from '../../interface'; import { roundToMaxDecimalPlaces } from '../../utils/floating-point'; import type { @@ -49,7 +49,6 @@ import type { }) export class Range implements ComponentInterface { private rangeId = `ion-r-${rangeIds++}`; - private didLoad = false; private noUpdate = false; private rect!: ClientRect; private hasFocus = false; @@ -61,6 +60,7 @@ export class Range implements ComponentInterface { private originalIonInput?: EventEmitter; private legacyFormController!: LegacyFormController; private isScrollingView = false; + private isTouching = false; // This flag ensures we log the deprecation warning at most once. private hasLoggedDeprecationWarning = false; @@ -293,22 +293,6 @@ export class Range implements ComponentInterface { */ @Event() ionKnobMoveEnd!: EventEmitter; - private setupGesture = async () => { - const rangeSlider = this.rangeSlider; - if (rangeSlider) { - this.gesture = (await import('../../utils/gesture')).createGesture({ - el: rangeSlider, - gestureName: 'range', - gesturePriority: 100, - threshold: 0, - onStart: () => this.onStart(), - onMove: (ev) => this.onMove(ev), - onEnd: (ev) => this.onEnd(ev), - }); - this.gesture.enable(!this.disabled); - } - }; - componentWillLoad() { /** * If user has custom ID set then we should @@ -323,9 +307,7 @@ export class Range implements ComponentInterface { componentDidLoad() { this.originalIonInput = this.ionInput; - this.setupGesture(); this.updateRatio(); - this.didLoad = true; } connectedCallback() { @@ -338,16 +320,6 @@ export class Range implements ComponentInterface { this.disabledChanged(); this.activeBarStartChanged(); - /** - * If we have not yet rendered - * ion-range, then rangeSlider is not defined. - * But if we are moving ion-range via appendChild, - * then rangeSlider will be defined. - */ - if (this.didLoad) { - this.setupGesture(); - } - this.contentEl = findClosestIonContent(this.el); } @@ -426,9 +398,10 @@ export class Range implements ComponentInterface { * For example: When the user lifts their finger from the * screen after tapping the bar or dragging the knob. */ - private onStart() { + private handlePointerDown = () => { + this.isTouching = true; this.ionKnobMoveStart.emit({ value: this.ensureValueInBounds(this.value) }); - } + }; /** * The value should be updated on touch end: @@ -441,41 +414,19 @@ export class Range implements ComponentInterface { * * The user can scroll on the view if the knob is not being dragged. * - * @param detail The details of the gesture event. + * @param ev The pointer event. */ - private onMove(detail: GestureDetail) { - const { contentEl, pressedKnob } = this; - const deltaY = detail.deltaY; - const currentX = detail.currentX; - /** - * Provide a threshold since the drag will not be - * smooth. The finger might move a few pixels up or down - * while dragging the knob. - */ - const threshold = 3; - const isScrollingView = Math.abs(deltaY) > threshold; + private handlePointerMove = (ev: PointerEvent) => { + console.log('handlePointerMove', this.isTouching); + const { pressedKnob } = this; + const currentX = ev.clientX; - /** - * The user is scrolling on the view while not dragging the knob. - * During this scenario, the view should scroll and - * the knob should not move. - * - * The user might be dragging the knob and then start - * scrolling on the view. This is why the `pressedKnob` is checked. - */ - if (isScrollingView && pressedKnob === undefined) { - this.isScrollingView = true; - return; - } + // if (ev.pressure === 0) { + // return; + // } - /** - * It's been determined that the user is not scrolling on the view. - * Since the user is dragging the knob, the view should not scroll. - * - * This only needs to be done once. - */ - if (contentEl && this.initialContentScrollY === undefined) { - this.initialContentScrollY = disableContentScrollY(contentEl); + if (!this.isTouching) { + return; } /** @@ -494,31 +445,19 @@ export class Range implements ComponentInterface { } this.update(currentX); - } + }; /** * The value should be updated on touch end: * - When the user lifts their finger from the screen after * tapping the bar or dragging the knob. * - * - * - * @param detail The details of the gesture event. + * @param ev The pointer event. */ - private onEnd(detail: GestureDetail) { - const { contentEl, initialContentScrollY } = this; - const currentX = detail.currentX; + private handlePointerUp = (ev: PointerEvent) => { + const currentX = ev.clientX; - /** - * The user is no longer scrolling on the view. - * - * The user can now scroll on the view again or drag the knob - * in the next gesture event. - */ - if (this.isScrollingView) { - this.isScrollingView = false; - return; - } + this.isTouching = false; /** * The `pressedKnob` can be undefined if the user never @@ -532,15 +471,6 @@ export class Range implements ComponentInterface { this.setPressedKnob(currentX); } - /** - * The user is no longer dragging the 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); - } - // update the active knob's position this.update(currentX); /** @@ -551,7 +481,7 @@ export class Range implements ComponentInterface { this.emitValueChange(); this.ionKnobMoveEnd.emit({ value: this.ensureValueInBounds(this.value) }); - } + }; private update(currentX: number) { // figure out where the pointer is currently at @@ -818,6 +748,9 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop inheritedAttributes, rangeId, pinFormatter, + handlePointerUp, + handlePointerDown, + handlePointerMove, } = this; /** @@ -905,7 +838,18 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop } return ( -
(this.rangeSlider = rangeEl)}> +
(this.rangeSlider = rangeEl)} + onPointerUp={handlePointerUp} + onPointerDown={handlePointerDown} + onPointerMove={handlePointerMove} + onPointerCancel={(ev: PointerEvent) => { + console.log('onPointerCancel', ev); + this.isTouching = false; + this.pressedKnob = undefined; + }} + > {ticks.map((tick) => (
); @@ -985,6 +931,7 @@ interface RangeKnob { labelText?: string | null; labelledBy?: string; handleKeyboard: (name: KnobName, isIncrease: boolean) => void; + handlePointerMove: (ev: PointerEvent) => void; } const renderKnob = ( @@ -1002,6 +949,7 @@ const renderKnob = ( labelText, labelledBy, pinFormatter, + handlePointerMove, }: RangeKnob ) => { const start = rtl ? 'right' : 'left'; @@ -1028,6 +976,7 @@ const renderKnob = ( ev.stopPropagation(); } }} + // onPointerMove={handlePointerMove} class={{ 'range-knob-handle': true, 'range-knob-a': knob === 'A', From 35c60c463882d054fff0c78392ebd7456ede55ed Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Wed, 14 Feb 2024 16:26:53 -0800 Subject: [PATCH 05/10] refactor(range): use a threshold through the setupGesture --- core/src/components/range/range.tsx | 132 ++++++++++++++++++---------- 1 file changed, 85 insertions(+), 47 deletions(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index f6edea399ad..7127fbe1fb1 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -10,7 +10,7 @@ import { isRTL } from '@utils/rtl'; import { createColorClasses, hostContext } from '@utils/theme'; import { getIonMode } from '../../global/ionic-global'; -import type { Color, Gesture, StyleEventDetail } from '../../interface'; +import type { Color, Gesture, GestureDetail, StyleEventDetail } from '../../interface'; import { roundToMaxDecimalPlaces } from '../../utils/floating-point'; import type { @@ -49,6 +49,7 @@ import type { }) export class Range implements ComponentInterface { private rangeId = `ion-r-${rangeIds++}`; + private didLoad = false; private noUpdate = false; private rect!: ClientRect; private hasFocus = false; @@ -59,8 +60,6 @@ export class Range implements ComponentInterface { private initialContentScrollY = true; private originalIonInput?: EventEmitter; private legacyFormController!: LegacyFormController; - private isScrollingView = false; - private isTouching = false; // This flag ensures we log the deprecation warning at most once. private hasLoggedDeprecationWarning = false; @@ -293,6 +292,28 @@ export class Range implements ComponentInterface { */ @Event() ionKnobMoveEnd!: EventEmitter; + private setupGesture = async () => { + const rangeSlider = this.rangeSlider; + if (rangeSlider) { + this.gesture = (await import('../../utils/gesture')).createGesture({ + el: rangeSlider, + gestureName: 'range', + gesturePriority: 100, + /** + * 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), + }); + this.gesture.enable(!this.disabled); + } + }; + componentWillLoad() { /** * If user has custom ID set then we should @@ -307,7 +328,9 @@ export class Range implements ComponentInterface { componentDidLoad() { this.originalIonInput = this.ionInput; + this.setupGesture(); this.updateRatio(); + this.didLoad = true; } connectedCallback() { @@ -320,6 +343,16 @@ export class Range implements ComponentInterface { this.disabledChanged(); this.activeBarStartChanged(); + /** + * If we have not yet rendered + * ion-range, then rangeSlider is not defined. + * But if we are moving ion-range via appendChild, + * then rangeSlider will be defined. + */ + if (this.didLoad) { + this.setupGesture(); + } + this.contentEl = findClosestIonContent(this.el); } @@ -392,41 +425,41 @@ export class Range implements ComponentInterface { } /** - * The value should be updated on touch end. + * 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 knob. + * screen after tapping the bar or dragging the bar or knob. */ - private handlePointerDown = () => { - this.isTouching = true; + private onStart() { this.ionKnobMoveStart.emit({ value: this.ensureValueInBounds(this.value) }); - }; + } /** - * The value should be updated on touch end: - * - When the user lifts their finger from the screen after - * dragging the knob. + * The value should be updated while dragging the + * bar or knob. * - * While the user is dragging the knob, the view + * 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 is not being dragged. + * The user can scroll on the view if the knob or + * bar is not being dragged. * - * @param ev The pointer event. + * @param detail The details of the gesture event. */ - private handlePointerMove = (ev: PointerEvent) => { - console.log('handlePointerMove', this.isTouching); - const { pressedKnob } = this; - const currentX = ev.clientX; - - // if (ev.pressure === 0) { - // return; - // } + private onMove(detail: GestureDetail) { + const { contentEl, pressedKnob } = this; + const currentX = detail.currentX; - if (!this.isTouching) { - return; + /** + * 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); } /** @@ -445,19 +478,18 @@ export class Range implements ComponentInterface { } this.update(currentX); - }; + } /** * The value should be updated on touch end: * - When the user lifts their finger from the screen after - * tapping the bar or dragging the knob. + * tapping the bar. * - * @param ev The pointer event. + * @param detail The details of the gesture or mouse event. */ - private handlePointerUp = (ev: PointerEvent) => { - const currentX = ev.clientX; - - this.isTouching = false; + private onEnd(detail: GestureDetail | MouseEvent) { + const { contentEl, initialContentScrollY } = this; + const currentX = (detail as GestureDetail).currentX || (detail as MouseEvent).clientX; /** * The `pressedKnob` can be undefined if the user never @@ -471,6 +503,16 @@ export class Range implements ComponentInterface { 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); + } + // update the active knob's position this.update(currentX); /** @@ -481,7 +523,7 @@ export class Range implements ComponentInterface { this.emitValueChange(); this.ionKnobMoveEnd.emit({ value: this.ensureValueInBounds(this.value) }); - }; + } private update(currentX: number) { // figure out where the pointer is currently at @@ -748,9 +790,6 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop inheritedAttributes, rangeId, pinFormatter, - handlePointerUp, - handlePointerDown, - handlePointerMove, } = this; /** @@ -841,14 +880,17 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop
(this.rangeSlider = rangeEl)} - onPointerUp={handlePointerUp} - onPointerDown={handlePointerDown} - onPointerMove={handlePointerMove} - onPointerCancel={(ev: PointerEvent) => { - console.log('onPointerCancel', ev); - this.isTouching = false; - this.pressedKnob = undefined; - }} + /** + * 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. + * + * However, this causes the value to not update + * if the user taps on the range. This is why + * we need to listen for the "click" event. + */ + onClick={(ev: MouseEvent) => this.onEnd(ev)} > {ticks.map((tick) => (
); @@ -931,7 +971,6 @@ interface RangeKnob { labelText?: string | null; labelledBy?: string; handleKeyboard: (name: KnobName, isIncrease: boolean) => void; - handlePointerMove: (ev: PointerEvent) => void; } const renderKnob = ( @@ -949,7 +988,6 @@ const renderKnob = ( labelText, labelledBy, pinFormatter, - handlePointerMove, }: RangeKnob ) => { const start = rtl ? 'right' : 'left'; From 6ee2134e1d9dfa0475b63a5d52b08b87fcacee94 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Thu, 15 Feb 2024 10:29:30 -0800 Subject: [PATCH 06/10] test(range): add check for end event on tap --- .../components/range/test/range-events.e2e.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/core/src/components/range/test/range-events.e2e.ts b/core/src/components/range/test/range-events.e2e.ts index bc019628e12..610270f4fbb 100644 --- a/core/src/components/range/test/range-events.e2e.ts +++ b/core/src/components/range/test/range-events.e2e.ts @@ -67,6 +67,39 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => 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'); From 3f4e38270582b8417d16e54a661bc93adebedc96 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Fri, 16 Feb 2024 15:07:11 -0800 Subject: [PATCH 07/10] refactor(range): remove commented code --- core/src/components/range/range.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 7127fbe1fb1..de384f8d4bb 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -1014,7 +1014,6 @@ const renderKnob = ( ev.stopPropagation(); } }} - // onPointerMove={handlePointerMove} class={{ 'range-knob-handle': true, 'range-knob-a': knob === 'A', From adc79ef7dc188e62ceccfeba9785d69b8d8597d6 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Tue, 5 Mar 2024 11:02:12 -0800 Subject: [PATCH 08/10] feat(range): emit start during onClick --- core/src/components/range/range.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index de384f8d4bb..2314b232063 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -886,11 +886,15 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop * the threshold. This is to prevent the range * from moving when the user is scrolling. * - * However, this causes the value to not update + * 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 "click" event. */ - onClick={(ev: MouseEvent) => this.onEnd(ev)} + onClick={(ev: MouseEvent) => { + this.onStart(); + this.onEnd(ev); + }} > {ticks.map((tick) => (
Date: Wed, 6 Mar 2024 12:47:13 -0800 Subject: [PATCH 09/10] refactor(range): remove duplicate events --- core/src/components/range/range.tsx | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 2314b232063..7b620addb01 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -889,11 +889,25 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop * 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 "click" event. + * we need to listen for the "pointerUp" event. */ - onClick={(ev: MouseEvent) => { - this.onStart(); - this.onEnd(ev); + onPointerUp={(ev: MouseEvent) => { + /** + * 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) => ( From ce4fc9992c87abbd4f38263e6331fe823c8230e1 Mon Sep 17 00:00:00 2001 From: Maria Hutt Date: Wed, 6 Mar 2024 13:19:24 -0800 Subject: [PATCH 10/10] refactor(range): add correct event --- core/src/components/range/range.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 7b620addb01..d8904a2f548 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -891,7 +891,7 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop * if the user taps on the range. This is why * we need to listen for the "pointerUp" event. */ - onPointerUp={(ev: MouseEvent) => { + onPointerUp={(ev: PointerEvent) => { /** * If the user drags the knob on the web * version (does not occur on mobile),