fix(range): disable scroll when range is being dragged

core/src/components/range/range.tsx

@@ -325,7 +325,8 @@ export class Range implements ComponentInterface {
       this.setupGesture();
     }
 
-    this.contentEl = findClosestIonContent(this.el);
+    const ionContent = findClosestIonContent(this.el);
+    this.contentEl = ionContent?.querySelector('.ion-content-scroll-host') ?? ionContent;
   }
 
   disconnectedCallback() {
@@ -418,7 +419,7 @@ export class Range implements ComponentInterface {
      *
      * This only needs to be done once.
      */
-    if (contentEl && this.initialContentScrollY === undefined) {
+    if (contentEl && this.pressedKnob === undefined) {
       this.initialContentScrollY = disableContentScrollY(contentEl);
     }
 
@@ -469,7 +470,7 @@ export class Range implements ComponentInterface {
      *
      * The user can now scroll on the view in the next gesture event.
      */
-    if (contentEl && initialContentScrollY !== undefined) {
+    if (contentEl && this.pressedKnob !== undefined) {
       resetContentScrollY(contentEl, initialContentScrollY);
     }
 

core/src/components/range/test/range-events.e2e.ts

@@ -7,10 +7,7 @@ import { configs, dragElementBy, test } from '@utils/test/playwright';
 configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
   test.describe(title('range: events:'), () => {
     test.describe('range: knob events', () => {
-      /**
-       * The mouse events are flaky on CI
-       */
-      test.fixme('should emit start/end events', async ({ page }) => {
+      test('should emit start/end events', async ({ page }) => {
         /**
          * Requires padding to prevent the knob from being clipped.
          * If it's clipped, then the value might be one off.
@@ -31,23 +28,34 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
 
         const rangeEl = page.locator('ion-range');
 
-        await dragElementBy(rangeEl, page, 300, 0);
-        await page.waitForChanges();
-
         /**
-         * dragElementBy defaults to starting the drag from the middle of the el,
-         * so the start value should jump to 50 despite the range defaulting to 20.
+         * Verify both events fire if range is dragged.
          */
-        expect(rangeStart).toHaveReceivedEventDetail({ value: 50 });
+        await dragElementBy(rangeEl, page, 180);
+
+        await rangeStart.next();
+        await rangeEnd.next();
+
+        // Once the knob is dragged, the start event should fire with
+        // the initial value.
+        expect(rangeStart).toHaveReceivedEventDetail({ value: 20 });
+        // Once the knob is released, the end event should fire with
+        // the final value.
         expect(rangeEnd).toHaveReceivedEventDetail({ value: 100 });
 
         /**
-         * Verify both events fire if range is clicked without dragging.
+         * Verify both events fire if range is tapped without dragging.
          */
         await dragElementBy(rangeEl, page, 0, 0);
-        await page.waitForChanges();
 
-        expect(rangeStart).toHaveReceivedEventDetail({ value: 50 });
+        await rangeStart.next();
+        await rangeEnd.next();
+
+        // Once the tap is released, the start event should fire with
+        // the initial value.
+        expect(rangeStart).toHaveReceivedEventDetail({ value: 100 });
+        // Once the tap is released, the end event should fire with
+        // the final value.
         expect(rangeEnd).toHaveReceivedEventDetail({ value: 50 });
       });
 
@@ -99,31 +107,6 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
 
         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');
-
-        await page.goto(`/src/components/range/test/basic`, config);
-
-        const knobEl = page.locator('ion-range#stacked-range .range-knob-handle');
-        const scrollEl = page.locator('ion-content .inner-scroll');
-
-        expect(await scrollEl.evaluate((el: HTMLElement) => el.scrollTop)).toEqual(0);
-
-        await dragElementBy(knobEl, page, 30, 0, undefined, undefined, false);
-
-        /**
-         * Do not use scrollToBottom() or other scrolling methods
-         * on ion-content as those will update the scroll position.
-         * Setting scrollTop still works even with overflow-y: hidden.
-         * However, simulating a user gesture should not scroll the content.
-         */
-        await page.mouse.wheel(0, 100);
-        await page.waitForChanges();
-
-        expect(await scrollEl.evaluate((el: HTMLElement) => el.scrollTop)).toEqual(0);
-      });
     });
 
     test.describe('ionChange', () => {
@@ -151,14 +134,26 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
         expect(ionChangeSpy).toHaveReceivedEventTimes(0);
       });
 
-      // TODO FW-2873
-      test.skip('should emit when the knob is released', async ({ page }) => {
-        await page.setContent(`<ion-range aria-label="range"></ion-range>`, config);
+      test('should emit when the knob is released', async ({ page }) => {
+        /**
+         * Requires padding to prevent the knob from being clipped.
+         * If it's clipped, then the value might be one off.
+         * For example, if the knob is clipped on the right, then the value
+         * will be 99 instead of 100.
+         */
+        await page.setContent(
+          `
+          <div style="padding: 0 20px">
+            <ion-range aria-label="Range"></ion-range>
+          </div>
+        `,
+          config
+        );
 
-        const rangeHandle = page.locator('ion-range .range-knob-handle');
+        const rangeEl = page.locator('ion-range');
         const ionChangeSpy = await page.spyOnEvent('ionChange');
 
-        await dragElementBy(rangeHandle, page, 100, 0);
+        await dragElementBy(rangeEl, page, 100);
 
         await ionChangeSpy.next();
 
@@ -196,16 +191,26 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
     });
 
     test.describe('ionInput', () => {
-      // TODO(FW-2873) Enable this test when touch events/gestures are better supported in Playwright
-      test.skip('should emit when the knob is dragged', async ({ page }) => {
-        await page.setContent(`<ion-range aria-label="range"></ion-range>`, config);
+      test('should emit when the knob is dragged', async ({ page }) => {
+        /**
+         * Requires padding to prevent the knob from being clipped.
+         * If it's clipped, then the value might be one off.
+         * For example, if the knob is clipped on the right, then the value
+         * will be 99 instead of 100.
+         */
+        await page.setContent(
+          `
+          <div style="padding: 0 20px">
+            <ion-range aria-label="range"></ion-range>
+          </div>
+        `,
+          config
+        );
 
-        const rangeHandle = page.locator('ion-range .range-knob-handle');
+        const rangeEl = page.locator('ion-range');
         const ionInputSpy = await page.spyOnEvent('ionInput');
 
-        await rangeHandle.hover();
-
-        await dragElementBy(rangeHandle, page, 100, 0, undefined, undefined, false);
+        await dragElementBy(rangeEl, page, 100);
 
         await ionInputSpy.next();
 

core/src/components/range/test/scroll-target/range.e2e.ts

@@ -1,29 +1,26 @@
 import { expect } from '@playwright/test';
-import { configs, test } from '@utils/test/playwright';
+import { configs, test, dragElementBy } from '@utils/test/playwright';
 
-// TODO FW-2873
 /**
  * This behavior does not vary across modes/directions.
  */
 configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
-  test.describe.skip(title('range: scroll-target'), () => {
+  test.describe(title('range: scroll-target'), () => {
     test('should not scroll when the knob is swiped in custom scroll target', async ({ page, skip }) => {
+      /**
+       * The Playwright team has stated that they will not implement this feature:
+       * https://github.com/microsoft/playwright/issues/28755
+       */
       skip.browser('webkit', 'mouse.wheel is not available in WebKit');
 
       await page.goto(`/src/components/range/test/scroll-target`, config);
 
-      const knobEl = page.locator('ion-range .range-knob-handle');
+      const rangeEl = page.locator('ion-range');
       const scrollEl = page.locator('.ion-content-scroll-host');
 
       expect(await scrollEl.evaluate((el: HTMLElement) => el.scrollTop)).toEqual(0);
 
-      const box = (await knobEl.boundingBox())!;
-      const centerX = box.x + box.width / 2;
-      const centerY = box.y + box.height / 2;
-
-      await page.mouse.move(centerX, centerY);
-      await page.mouse.down();
-      await page.mouse.move(centerX + 30, centerY);
+      await dragElementBy(rangeEl, page, 100, 0, undefined, undefined, false);
 
       /**
        * Do not use scrollToBottom() or other scrolling methods

core/src/components/range/test/scroll/index.html

@@ -0,0 +1,73 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+  <head>
+    <meta charset="UTF-8" />
+    <title>Range - Scroll</title>
+    <meta
+      name="viewport"
+      content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no"
+    />
+    <link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
+    <link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
+    <script src="../../../../../scripts/testing/scripts.js"></script>
+    <script nomodule src="../../../../../dist/ionic/ionic.js"></script>
+    <script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
+  </head>
+  <body>
+    <ion-app>
+      <ion-header>
+        <ion-toolbar>
+          <ion-title>Range - Scroll</ion-title>
+        </ion-toolbar>
+      </ion-header>
+
+      <ion-content class="ion-padding">
+        <p>
+          Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur faucibus nulla a nunc tincidunt semper. Nam
+          nibh lorem, pharetra ac ex ac, tempus fringilla est. Aenean tincidunt ipsum pellentesque, consequat libero id,
+          feugiat leo. In vestibulum faucibus velit, non tincidunt erat tincidunt in. Donec a diam sed nisl convallis
+          maximus. Aenean cursus sagittis lorem vitae tristique. Pellentesque pellentesque, quam eget lobortis finibus,
+          lectus lorem maximus purus, quis sagittis tortor sem sed tellus.
+        </p>
+
+        <ion-range value="40">
+          <div slot="label">Range Label</div>
+          <ion-label slot="start">Start</ion-label>
+          <ion-label slot="end">End</ion-label>
+        </ion-range>
+
+        <p>
+          Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur faucibus nulla a nunc tincidunt semper. Nam
+          nibh lorem, pharetra ac ex ac, tempus fringilla est. Aenean tincidunt ipsum pellentesque, consequat libero id,
+          feugiat leo. In vestibulum faucibus velit, non tincidunt erat tincidunt in. Donec a diam sed nisl convallis
+          maximus. Aenean cursus sagittis lorem vitae tristique. Pellentesque pellentesque, quam eget lobortis finibus,
+          lectus lorem maximus purus, quis sagittis tortor sem sed tellus.
+        </p>
+
+        <p>
+          Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur faucibus nulla a nunc tincidunt semper. Nam
+          nibh lorem, pharetra ac ex ac, tempus fringilla est. Aenean tincidunt ipsum pellentesque, consequat libero id,
+          feugiat leo. In vestibulum faucibus velit, non tincidunt erat tincidunt in. Donec a diam sed nisl convallis
+          maximus. Aenean cursus sagittis lorem vitae tristique. Pellentesque pellentesque, quam eget lobortis finibus,
+          lectus lorem maximus purus, quis sagittis tortor sem sed tellus.
+        </p>
+
+        <p>
+          Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur faucibus nulla a nunc tincidunt semper. Nam
+          nibh lorem, pharetra ac ex ac, tempus fringilla est. Aenean tincidunt ipsum pellentesque, consequat libero id,
+          feugiat leo. In vestibulum faucibus velit, non tincidunt erat tincidunt in. Donec a diam sed nisl convallis
+          maximus. Aenean cursus sagittis lorem vitae tristique. Pellentesque pellentesque, quam eget lobortis finibus,
+          lectus lorem maximus purus, quis sagittis tortor sem sed tellus.
+        </p>
+
+        <p>
+          Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur faucibus nulla a nunc tincidunt semper. Nam
+          nibh lorem, pharetra ac ex ac, tempus fringilla est. Aenean tincidunt ipsum pellentesque, consequat libero id,
+          feugiat leo. In vestibulum faucibus velit, non tincidunt erat tincidunt in. Donec a diam sed nisl convallis
+          maximus. Aenean cursus sagittis lorem vitae tristique. Pellentesque pellentesque, quam eget lobortis finibus,
+          lectus lorem maximus purus, quis sagittis tortor sem sed tellus.!
+        </p>
+      </ion-content>
+    </ion-app>
+  </body>
+</html>

core/src/components/range/test/scroll/range.e2e.ts

@@ -0,0 +1,46 @@
+import { expect } from '@playwright/test';
+import { configs, dragElementBy, test } from '@utils/test/playwright';
+
+/**
+ * This behavior does not vary across modes/directions.
+ */
+configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
+  test.describe(title('range: scroll'), () => {
+    test('should not scroll when the knob is being dragged', async ({ page, skip }) => {
+      /**
+       * The Playwright team has stated that they will not implement this feature:
+       * https://github.com/microsoft/playwright/issues/28755
+       */
+      skip.browser('webkit', 'mouse.wheel is not available in WebKit');
+
+      /**
+       * Requires padding to prevent the knob from being clipped.
+       * If it's clipped, then the value might be one off.
+       * For example, if the knob is clipped on the right, then the value
+       * will be 99 instead of 100.
+       *
+       * The ion-content is also required to be taller than the viewport
+       * to allow for scrolling.
+       */
+      await page.goto(`/src/components/range/test/scroll`, config);
+
+      const rangeEl = page.locator('ion-range');
+      const scrollEl = page.locator('ion-content .inner-scroll');
+
+      expect(await scrollEl.evaluate((el: HTMLElement) => el.scrollTop)).toEqual(0);
+
+      await dragElementBy(rangeEl, page, 100, 0, undefined, undefined, false);
+
+      /**
+       * Do not use scrollToBottom() or other scrolling methods
+       * on ion-content as those will update the scroll position.
+       * Setting scrollTop still works even with overflow-y: hidden.
+       * However, simulating a user gesture should not scroll the content.
+       */
+      await page.mouse.wheel(0, 100);
+      await page.waitForChanges();
+
+      expect(await scrollEl.evaluate((el: HTMLElement) => el.scrollTop)).toEqual(0);
+    });
+  });
+});