refactor(range): update value on touchEnd or drag

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,21 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop
     }
 
     return (
-      <div class="range-slider" ref={(rangeEl) => (this.rangeSlider = rangeEl)}>
+      <div
+        class="range-slider"
+        ref={(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.
+         *
+         * 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) => (
           <div
             style={tickStyle(tick)}
@@ -922,6 +1014,7 @@ const renderKnob = (
           ev.stopPropagation();
         }
       }}
+      // onPointerMove={handlePointerMove}
       class={{
         'range-knob-handle': true,
         'range-knob-a': knob === 'A',

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(`<ion-range aria-label="Range" value="20"></ion-range>`, 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');