diff --git a/core/src/components/modal/gestures/sheet.ts b/core/src/components/modal/gestures/sheet.ts index cacf61c5b43..a4c155a731e 100644 --- a/core/src/components/modal/gestures/sheet.ts +++ b/core/src/components/modal/gestures/sheet.ts @@ -7,7 +7,7 @@ import type { Animation, ModalDragEventDetail } from '../../../interface'; import type { GestureDetail } from '../../../utils/gesture'; import { getBackdropValueForSheet } from '../utils'; -import { calculateSpringStep, handleCanDismiss } from './utils'; +import { calculateSpringStep, canSwipeOnContent, handleCanDismiss } from './utils'; export interface MoveSheetToBreakpointOptions { /** @@ -261,9 +261,6 @@ export const createSheetGesture = ( const canStart = (detail: GestureDetail) => { /** - * If we are swiping on the content, swiping should only be possible if the content - * is scrolled all the way to the top so that we do not interfere with scrolling. - * * We cannot assume that the `ion-content` target will remain consistent between swipes. * For example, when using ion-nav within a modal it is possible to swipe, push a view, * and then swipe again. The target content will not be the same between swipes. @@ -272,27 +269,11 @@ export const createSheetGesture = ( currentBreakpoint = getCurrentBreakpoint(); /** - * If `expandToScroll` is disabled, we should not allow the swipe gesture - * to start if the content is not scrolled to the top. + * Upwards swipes on the content cannot move the sheet anyway, so this only + * blocks swiping the sheet down from the content. */ - if (!expandToScroll && contentEl) { - const scrollEl = isIonContent(contentEl) ? getElementRoot(contentEl).querySelector('.inner-scroll') : contentEl; - return scrollEl!.scrollTop === 0; - } - - if (currentBreakpoint === 1 && contentEl) { - /** - * The modal should never swipe to close on the content with a refresher. - * Note 1: We cannot solve this by making this gesture have a higher priority than - * the refresher gesture as the iOS native refresh gesture uses a scroll listener in - * addition to a gesture. - * - * Note 2: Do not use getScrollElement here because we need this to be a synchronous - * operation, and getScrollElement is asynchronous. - */ - const scrollEl = isIonContent(contentEl) ? getElementRoot(contentEl).querySelector('.inner-scroll') : contentEl; - const hasRefresherInContent = !!contentEl.querySelector('ion-refresher'); - return !hasRefresherInContent && scrollEl!.scrollTop === 0; + if (contentEl && (!expandToScroll || currentBreakpoint === 1)) { + return canSwipeOnContent(contentEl); } return true; diff --git a/core/src/components/modal/gestures/swipe-to-close.ts b/core/src/components/modal/gestures/swipe-to-close.ts index d04220c88e7..f5b753d8b01 100644 --- a/core/src/components/modal/gestures/swipe-to-close.ts +++ b/core/src/components/modal/gestures/swipe-to-close.ts @@ -1,7 +1,7 @@ import { getTimeGivenProgression } from '@utils/animation/cubic-bezier'; import { isIonContent, findClosestIonContent, disableContentScrollY, resetContentScrollY } from '@utils/content'; import { createGesture } from '@utils/gesture'; -import { clamp, getElementRoot } from '@utils/helpers'; +import { clamp } from '@utils/helpers'; import { OVERLAY_GESTURE_PRIORITY } from '@utils/overlays'; import type { Animation, ModalDragEventDetail } from '../../../interface'; @@ -9,7 +9,7 @@ import type { GestureDetail } from '../../../utils/gesture'; import type { Style as StatusBarStyle } from '../../../utils/native/status-bar'; import { setCardStatusBarDark, setCardStatusBarDefault } from '../utils'; -import { calculateSpringStep, handleCanDismiss } from './utils'; +import { calculateSpringStep, canSwipeOnContent, handleCanDismiss } from './utils'; // Defaults for the card swipe animation export const SwipeToCloseDefaults = { @@ -35,7 +35,6 @@ export const createSwipeToCloseGesture = ( let isOpen = false; let canDismissBlocksGesture = false; let contentEl: HTMLElement | null = null; - let scrollEl: HTMLElement | null = null; const canDismissMaxStep = 0.2; let initialScrollY = true; let lastStep = 0; @@ -60,12 +59,6 @@ export const createSwipeToCloseGesture = ( } /** - * If we are swiping on the content, - * swiping should only be possible if - * the content is scrolled all the way - * to the top so that we do not interfere - * with scrolling. - * * We cannot assume that the `ion-content` * target will remain consistent between * swipes. For example, when using @@ -76,29 +69,7 @@ export const createSwipeToCloseGesture = ( */ contentEl = findClosestIonContent(target); if (contentEl) { - /** - * The card should never swipe to close - * on the content with a refresher. - * Note: We cannot solve this by making the - * swipeToClose gesture have a higher priority - * than the refresher gesture as the iOS native - * refresh gesture uses a scroll listener in - * addition to a gesture. - * - * Note: Do not use getScrollElement here - * because we need this to be a synchronous - * operation, and getScrollElement is - * asynchronous. - */ - if (isIonContent(contentEl)) { - const root = getElementRoot(contentEl); - scrollEl = root.querySelector('.inner-scroll'); - } else { - scrollEl = contentEl; - } - - const hasRefresherInContent = !!contentEl.querySelector('ion-refresher'); - return !hasRefresherInContent && scrollEl!.scrollTop === 0; + return canSwipeOnContent(contentEl); } /** diff --git a/core/src/components/modal/gestures/utils.ts b/core/src/components/modal/gestures/utils.ts index 694227d630d..270341719a8 100644 --- a/core/src/components/modal/gestures/utils.ts +++ b/core/src/components/modal/gestures/utils.ts @@ -1,7 +1,26 @@ +import { findRefresherInContent, isIonContent } from '@utils/content'; +import { getElementRoot } from '@utils/helpers'; import { GESTURE } from '@utils/overlays'; import type { Animation } from '../../../interface'; +/** + * Swiping is only possible when the content is scrolled to the top, so that we + * do not interfere with scrolling, and never on content with a refresher. + * + * Note: We cannot solve the refresher case with gesture priority as the iOS + * native refresh gesture uses a scroll listener in addition to a gesture. + * + * Note: Do not use `getScrollElement` here because we need this to be a + * synchronous operation, and `getScrollElement` is asynchronous. + */ +export const canSwipeOnContent = (contentEl: HTMLElement) => { + const scrollEl = isIonContent(contentEl) ? getElementRoot(contentEl).querySelector('.inner-scroll') : contentEl; + const hasRefresherInContent = !!findRefresherInContent(contentEl); + + return !hasRefresherInContent && scrollEl!.scrollTop === 0; +}; + export const handleCanDismiss = async (el: HTMLIonModalElement, animation: Animation) => { /** * If canDismiss is not a function diff --git a/core/src/components/modal/test/refresher-scroll-target/index.html b/core/src/components/modal/test/refresher-scroll-target/index.html new file mode 100644 index 00000000000..12206de68df --- /dev/null +++ b/core/src/components/modal/test/refresher-scroll-target/index.html @@ -0,0 +1,136 @@ + + + + + Modal - Refresher with Custom Scroll Target + + + + + + + + + + + + + + + +
+ + + Refresher with Custom Scroll Target + + + + + + + + + + +
+ + + + + diff --git a/core/src/components/modal/test/refresher-scroll-target/modal.e2e.ts b/core/src/components/modal/test/refresher-scroll-target/modal.e2e.ts new file mode 100644 index 00000000000..c327da2436d --- /dev/null +++ b/core/src/components/modal/test/refresher-scroll-target/modal.e2e.ts @@ -0,0 +1,142 @@ +import { expect } from '@playwright/test'; +import type { E2EPage } from '@utils/test/playwright'; +import { configs, dragElementByYAxis, test } from '@utils/test/playwright'; + +const dragDownOnScrollHost = async (page: E2EPage, dragByY = 250) => { + // The refresher creates its gesture asynchronously, so wait for it to hydrate. + await page.locator('ion-modal ion-refresher.hydrated').waitFor({ state: 'attached' }); + + const scrollHost = page.locator('ion-modal .ion-content-scroll-host'); + const box = (await scrollHost.boundingBox())!; + + // Start near the top of the host so the drag stays within smaller viewports. + await dragElementByYAxis(scrollHost, page, dragByY, box.y + 5); +}; + +/** + * This behavior does not vary across directions. + */ +configs({ directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('sheet modal: refresher with custom scroll target'), () => { + test.beforeEach(async ({ page }) => { + await page.goto('/src/components/modal/test/refresher-scroll-target', config); + }); + + test('should refresh instead of dismissing when pulling down on the custom scroll target', async ({ + page, + }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31332', + }); + + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss'); + const ionRefresh = await page.spyOnEvent('ionRefresh'); + + await page.click('#sheet'); + await ionModalDidPresent.next(); + + await dragDownOnScrollHost(page); + + await ionRefresh.next(); + expect(ionModalDidDismiss).toHaveReceivedEventTimes(0); + }); + + test('should refresh instead of dismissing when expandToScroll is disabled', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31332', + }); + + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss'); + const ionRefresh = await page.spyOnEvent('ionRefresh'); + + await page.click('#sheet-no-expand'); + await ionModalDidPresent.next(); + + await dragDownOnScrollHost(page); + + await ionRefresh.next(); + expect(ionModalDidDismiss).toHaveReceivedEventTimes(0); + }); + + test('should move the sheet rather than refresh when dragging the content at a partial breakpoint', async ({ + page, + }) => { + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + const ionBreakpointDidChange = await page.spyOnEvent('ionBreakpointDidChange'); + const ionRefresh = await page.spyOnEvent('ionRefresh'); + + await page.click('#sheet-breakpoints'); + await ionModalDidPresent.next(); + + await dragDownOnScrollHost(page); + + await ionBreakpointDidChange.next(); + expect(ionRefresh).toHaveReceivedEventTimes(0); + }); + + test('should refresh rather than move the sheet at a partial breakpoint when expandToScroll is disabled', async ({ + page, + }) => { + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + const ionBreakpointDidChange = await page.spyOnEvent('ionBreakpointDidChange'); + const ionRefresh = await page.spyOnEvent('ionRefresh'); + + await page.click('#sheet-breakpoints-no-expand'); + await ionModalDidPresent.next(); + + await dragDownOnScrollHost(page); + + await ionRefresh.next(); + expect(ionBreakpointDidChange).toHaveReceivedEventTimes(0); + }); + + test('should still dismiss when dragging the handle', async ({ page }) => { + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss'); + + await page.click('#sheet-no-expand'); + await ionModalDidPresent.next(); + + await dragElementByYAxis(page.locator('ion-modal .modal-handle'), page, 500); + + await ionModalDidDismiss.next(); + }); + }); +}); + +/** + * Card modals are only available in iOS mode. + * This behavior does not vary across directions. + */ +configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('card modal: refresher with custom scroll target'), () => { + test.beforeEach(async ({ page }) => { + await page.goto('/src/components/modal/test/refresher-scroll-target', config); + }); + + test('should refresh instead of dismissing when pulling down on the custom scroll target', async ({ + page, + }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31332', + }); + + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + const ionModalDidDismiss = await page.spyOnEvent('ionModalDidDismiss'); + const ionRefresh = await page.spyOnEvent('ionRefresh'); + + await page.click('#card'); + await ionModalDidPresent.next(); + + await dragDownOnScrollHost(page); + + await ionRefresh.next(); + expect(ionModalDidDismiss).toHaveReceivedEventTimes(0); + }); + }); +}); diff --git a/core/src/components/refresher/refresher.tsx b/core/src/components/refresher/refresher.tsx index be838aa9c4a..ecd27fd0965 100644 --- a/core/src/components/refresher/refresher.tsx +++ b/core/src/components/refresher/refresher.tsx @@ -2,6 +2,7 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; import { Component, Element, Event, Host, Method, Prop, State, Watch, h, readTask, writeTask } from '@stencil/core'; import { getTimeGivenProgression } from '@utils/animation/cubic-bezier'; import { + findRefresherScrollHost, getScrollElement, ION_CONTENT_CLASS_SELECTOR, ION_CONTENT_ELEMENT_SELECTOR, @@ -526,7 +527,7 @@ export class Refresher implements ComponentInterface { * or the background content element. */ componentOnReady(contentEl, async () => { - const customScrollTarget = contentEl.querySelector(ION_CONTENT_CLASS_SELECTOR); + const customScrollTarget = findRefresherScrollHost(contentEl); /** * Query the custom scroll target (if available), first. In refresher implementations, * the ion-refresher element will always be a direct child of ion-content (slot="fixed"). By diff --git a/core/src/utils/content/content.utils.spec.ts b/core/src/utils/content/content.utils.spec.ts index a9be0a6139f..95b674b7f02 100644 --- a/core/src/utils/content/content.utils.spec.ts +++ b/core/src/utils/content/content.utils.spec.ts @@ -4,6 +4,7 @@ import { printIonContentErrorMsg, findClosestIonContent, findIonContent, + findRefresherInContent, getScrollElement, } from './index'; @@ -48,6 +49,68 @@ describe('Content Utils', () => { }); }); + describe('findRefresherInContent', () => { + const createContent = ({ hasRefresher = true, scrollHostCount = 0 } = {}) => { + const content = document.createElement('ion-content'); + + if (hasRefresher) { + const refresher = document.createElement('ion-refresher'); + refresher.setAttribute('slot', 'fixed'); + content.appendChild(refresher); + } + + for (let i = 0; i < scrollHostCount; i++) { + const scrollHost = document.createElement('div'); + scrollHost.classList.add('ion-content-scroll-host'); + content.appendChild(scrollHost); + } + + return content; + }; + + it('should find the refresher within ion-content', () => { + const content = createContent(); + + expect(findRefresherInContent(content)).toBe(content.querySelector('ion-refresher')); + }); + + it('should return null when ion-content has no refresher', () => { + const content = createContent({ hasRefresher: false }); + + expect(findRefresherInContent(content)).toBe(null); + }); + + it('should find the refresher from a custom scroll host that is a sibling of it', () => { + const content = createContent({ scrollHostCount: 1 }); + const scrollHost = content.querySelector('.ion-content-scroll-host')!; + + expect(findRefresherInContent(scrollHost)).toBe(content.querySelector('ion-refresher')); + }); + + it('should find the refresher from an element nested within the custom scroll host', () => { + const content = createContent({ scrollHostCount: 1 }); + const nested = document.createElement('div'); + content.querySelector('.ion-content-scroll-host')!.appendChild(nested); + + expect(findRefresherInContent(nested)).toBe(content.querySelector('ion-refresher')); + }); + + it('should return null for a custom scroll host the refresher does not scroll with', () => { + const content = createContent({ scrollHostCount: 2 }); + const secondScrollHost = content.querySelectorAll('.ion-content-scroll-host')[1]; + + expect(findRefresherInContent(secondScrollHost)).toBe(null); + }); + + it('should return null for a custom scroll host that is not within an ion-content', () => { + const scrollHost = document.createElement('div'); + scrollHost.classList.add('ion-content-scroll-host'); + scrollHost.appendChild(document.createElement('ion-refresher')); + + expect(findRefresherInContent(scrollHost)).toBe(null); + }); + }); + describe('scrollToTop', () => { describe('scroll duration is 0', () => { it('should call scrollToTop when the tag name is ion-content', () => { diff --git a/core/src/utils/content/index.ts b/core/src/utils/content/index.ts index 44a6f7bff13..3c4f0967592 100644 --- a/core/src/utils/content/index.ts +++ b/core/src/utils/content/index.ts @@ -58,6 +58,41 @@ export const findClosestIonContent = (el: Element) => { return el.closest(ION_CONTENT_SELECTOR); }; +/** + * Queries the custom scroll host an `ion-refresher` scrolls with in the given + * `ion-content`. A refresher only pairs with the first host. + */ +export const findRefresherScrollHost = (ionContent: Element) => { + return ionContent.querySelector(ION_CONTENT_CLASS_SELECTOR); +}; + +/** + * Queries the `ion-refresher` that scrolls with the given content element, + * which may be an `ion-content` or a custom scroll host. A refresher is a + * `slot="fixed"` child of `ion-content`, so it is a sibling of a scroll host + * rather than a descendant of it. + */ +export const findRefresherInContent = (contentEl: Element) => { + // An `ion-content` owns any refresher inside it, scroll host or not. + if (isIonContent(contentEl)) { + return contentEl.querySelector('ion-refresher'); + } + + // A refresher needs an `ion-content` ancestor to initialize. + const ionContent = contentEl.closest(ION_CONTENT_ELEMENT_SELECTOR); + if (ionContent === null) { + return null; + } + + // Any scroll host other than the refresher's own has no refresher. + const refresherScrollHost = findRefresherScrollHost(ionContent); + if (refresherScrollHost === null || !refresherScrollHost.contains(contentEl)) { + return null; + } + + return ionContent.querySelector('ion-refresher'); +}; + /** * Scrolls to the top of the element. If an `ion-content` is found, it will scroll * using the public API `scrollToTop` with a duration.