From 0b92dffa92c05705ff83518c10608e3dc3651d51 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 31 May 2022 11:35:07 -0400 Subject: [PATCH] fix(range): dragging knob no longer scrolls page (#25343) resolves #19004 --- core/src/components/range/range.tsx | 15 ++++ .../components/range/test/basic/index.html | 2 +- .../components/range/test/basic/range.e2e.ts | 31 +++++++ .../range/test/scroll-target/index.html | 85 +++++++++++++++++++ .../range/test/scroll-target/range.e2e.ts | 35 ++++++++ core/src/utils/content/index.ts | 34 ++++++++ 6 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 core/src/components/range/test/scroll-target/index.html create mode 100644 core/src/components/range/test/scroll-target/range.e2e.ts diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 86dd925d65..5bf61d19c8 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -13,6 +13,7 @@ import type { RangeValue, StyleEventDetail, } from '../../interface'; +import { findClosestIonContent, disableContentScrollY, resetContentScrollY } from '../../utils/content'; import type { Attributes } from '../../utils/helpers'; import { inheritAriaAttributes, clamp, debounceEvent, getAriaLabel, renderHiddenInput } from '../../utils/helpers'; import { isRTL } from '../../utils/rtl'; @@ -50,6 +51,8 @@ export class Range implements ComponentInterface { private rangeSlider?: HTMLElement; private gesture?: Gesture; private inheritedAttributes: Attributes = {}; + private contentEl: HTMLElement | null = null; + private initialContentScrollY = true; @Element() el!: HTMLIonRangeElement; @@ -259,6 +262,8 @@ export class Range implements ComponentInterface { if (this.didLoad) { this.setupGesture(); } + + this.contentEl = findClosestIonContent(this.el); } disconnectedCallback() { @@ -313,6 +318,11 @@ export class Range implements ComponentInterface { } private onStart(detail: GestureDetail) { + const { contentEl } = this; + if (contentEl) { + this.initialContentScrollY = disableContentScrollY(contentEl); + } + const rect = (this.rect = this.rangeSlider!.getBoundingClientRect() as any); const currentX = detail.currentX; @@ -337,6 +347,11 @@ export class Range implements ComponentInterface { } private onEnd(detail: GestureDetail) { + const { contentEl, initialContentScrollY } = this; + if (contentEl) { + resetContentScrollY(contentEl, initialContentScrollY); + } + this.update(detail.currentX); this.pressedKnob = undefined; diff --git a/core/src/components/range/test/basic/index.html b/core/src/components/range/test/basic/index.html index 8402505c37..642f4e974c 100644 --- a/core/src/components/range/test/basic/index.html +++ b/core/src/components/range/test/basic/index.html @@ -93,7 +93,7 @@ Stacked Label - + Start End diff --git a/core/src/components/range/test/basic/range.e2e.ts b/core/src/components/range/test/basic/range.e2e.ts index 6915c5de26..a5d3317f51 100644 --- a/core/src/components/range/test/basic/range.e2e.ts +++ b/core/src/components/range/test/basic/range.e2e.ts @@ -53,4 +53,35 @@ test.describe('range: basic', () => { expect(rangeStart).toHaveReceivedEventDetail({ value: 20 }); expect(rangeEnd).toHaveReceivedEventDetail({ value: 21 }); }); + + test('should not scroll when the knob is swiped', async ({ page, browserName }, testInfo) => { + test.skip(browserName === 'webkit', 'mouse.wheel is not available in WebKit'); + test.skip(testInfo.project.metadata.rtl === true, 'This feature does not have RTL-specific behaviors'); + + await page.goto(`/src/components/range/test/basic`); + + 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); + + 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); + + /** + * 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); + }); }); diff --git a/core/src/components/range/test/scroll-target/index.html b/core/src/components/range/test/scroll-target/index.html new file mode 100644 index 0000000000..f3881c1340 --- /dev/null +++ b/core/src/components/range/test/scroll-target/index.html @@ -0,0 +1,85 @@ + + + + + Range - Scroll Target + + + + + + + + + + + + + Range - Scroll Target + + + + +
+

+ 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. +

+ + + Stacked Label + + Start + End + + + +

+ 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. +

+ +

+ 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. +

+ +

+ 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. +

+ +

+ 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. +

+
+ + + + diff --git a/core/src/components/range/test/scroll-target/range.e2e.ts b/core/src/components/range/test/scroll-target/range.e2e.ts new file mode 100644 index 0000000000..26dd5049a4 --- /dev/null +++ b/core/src/components/range/test/scroll-target/range.e2e.ts @@ -0,0 +1,35 @@ +import { expect } from '@playwright/test'; +import { test } from '@utils/test/playwright'; + +test.describe('range: scroll-target', () => { + test('should not scroll when the knob is swiped in custom scroll target', async ({ page, browserName }, testInfo) => { + test.skip(browserName === 'webkit', 'mouse.wheel is not available in WebKit'); + test.skip(testInfo.project.metadata.rtl === true, 'This feature does not have RTL-specific behaviors'); + + await page.goto(`/src/components/range/test/scroll-target`); + + const knobEl = page.locator('ion-range .range-knob-handle'); + 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); + + /** + * 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); + }); +}); diff --git a/core/src/utils/content/index.ts b/core/src/utils/content/index.ts index 0d46e0fbfd..dd7011fcd0 100644 --- a/core/src/utils/content/index.ts +++ b/core/src/utils/content/index.ts @@ -101,3 +101,37 @@ export const scrollByPoint = (el: HTMLElement, x: number, y: number, durationMs: export const printIonContentErrorMsg = (el: HTMLElement) => { return printRequiredElementError(el, ION_CONTENT_ELEMENT_SELECTOR); }; + +/** + * Several components in Ionic need to prevent scrolling + * during a gesture (card modal, range, item sliding, etc). + * Use this utility to account for ion-content and custom content hosts. + */ +export const disableContentScrollY = (contentEl: HTMLElement): boolean => { + if (isIonContent(contentEl)) { + const ionContent = contentEl as HTMLIonContentElement; + const initialScrollY = ionContent.scrollY; + ionContent.scrollY = false; + + /** + * This should be passed into resetContentScrollY + * so that we can revert ion-content's scrollY to the + * correct state. For example, if scrollY = false + * initially, we do not want to enable scrolling + * when we call resetContentScrollY. + */ + return initialScrollY; + } else { + contentEl.style.setProperty('overflow', 'hidden'); + + return true; + } +}; + +export const resetContentScrollY = (contentEl: HTMLElement, initialScrollY: boolean) => { + if (isIonContent(contentEl)) { + (contentEl as HTMLIonContentElement).scrollY = initialScrollY; + } else { + contentEl.style.removeProperty('overflow'); + } +};