refactor(range): update value on touchEnd or drag (ionic-team#29005)

Issue number: resolves ionic-team#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.

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 (
-      <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.
+         *
+         * 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) => (
           <div
             style={tickStyle(tick)}

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');