From b315cf6ad8bda71b7b45a646f68e646457f8e157 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Tue, 18 Mar 2025 10:57:23 -0700 Subject: [PATCH 1/3] fix(sheet): fixing performance regression on modal sheets when expandToScroll is false --- core/src/components/modal/gestures/sheet.ts | 30 ++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/core/src/components/modal/gestures/sheet.ts b/core/src/components/modal/gestures/sheet.ts index 2375cfd89bd..133279d57a8 100644 --- a/core/src/components/modal/gestures/sheet.ts +++ b/core/src/components/modal/gestures/sheet.ts @@ -1,6 +1,6 @@ -import { isIonContent, findClosestIonContent } from '@utils/content'; +import { findClosestIonContent, isIonContent } from '@utils/content'; import { createGesture } from '@utils/gesture'; -import { clamp, raf, getElementRoot } from '@utils/helpers'; +import { clamp, getElementRoot, raf } from '@utils/helpers'; import { FOCUS_TRAP_DISABLE_CLASS } from '@utils/overlays'; import type { Animation } from '../../../interface'; @@ -83,6 +83,7 @@ export const createSheetGesture = ( let currentBreakpoint = initialBreakpoint; let offset = 0; let canDismissBlocksGesture = false; + let cachedScrollEl: HTMLElement | null = null; const canDismissMaxStep = 0.95; const maxBreakpoint = breakpoints[breakpoints.length - 1]; const minBreakpoint = breakpoints[0]; @@ -233,6 +234,14 @@ export const createSheetGesture = ( */ canDismissBlocksGesture = baseEl.canDismiss !== undefined && baseEl.canDismiss !== true && minBreakpoint === 0; + /** + * Cache the scroll element reference when the gesture starts, + * this allows us to avoid querying the DOM for the target in onMove, + * which would impact performance significantly. + */ + const targetEl = findClosestIonContent(detail.event.target! as HTMLElement); + cachedScrollEl = targetEl && isIonContent(targetEl) ? getElementRoot(targetEl).querySelector('.inner-scroll') : targetEl; + /** * If expandToScroll is disabled, we need to swap * the footer visibility to the original, so if the modal @@ -267,13 +276,8 @@ export const createSheetGesture = ( * If `expandToScroll` is disabled, and an upwards swipe gesture is done within * the scrollable content, we should not allow the swipe gesture to continue. */ - if (!expandToScroll && detail.deltaY <= 0) { - const contentEl = findClosestIonContent(detail.event.target! as HTMLElement); - const scrollEl = - contentEl && isIonContent(contentEl) ? getElementRoot(contentEl).querySelector('.inner-scroll') : contentEl; - if (scrollEl) { - return; - } + if (!expandToScroll && detail.deltaY <= 0 && cachedScrollEl) { + return; } /** @@ -334,12 +338,8 @@ export const createSheetGesture = ( * function to be called if the user is trying to swipe content upwards and the content * is not scrolled to the top. */ - if (!expandToScroll && detail.deltaY <= 0 && findClosestIonContent(detail.event.target! as HTMLElement)) { - const contentEl = findClosestIonContent(detail.event.target! as HTMLElement)!; - const scrollEl = isIonContent(contentEl) ? getElementRoot(contentEl).querySelector('.inner-scroll') : contentEl; - if (scrollEl!.scrollTop > 0) { - return; - } + if (!expandToScroll && detail.deltaY <= 0 && cachedScrollEl && cachedScrollEl.scrollTop > 0) { + return; } /** From 28d4b2a4fd5eb28041458e51543dcd69073de7c3 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Tue, 18 Mar 2025 11:15:03 -0700 Subject: [PATCH 2/3] fix(lint): fixing lint --- core/src/components/modal/gestures/sheet.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/components/modal/gestures/sheet.ts b/core/src/components/modal/gestures/sheet.ts index 133279d57a8..c1881731349 100644 --- a/core/src/components/modal/gestures/sheet.ts +++ b/core/src/components/modal/gestures/sheet.ts @@ -240,7 +240,8 @@ export const createSheetGesture = ( * which would impact performance significantly. */ const targetEl = findClosestIonContent(detail.event.target! as HTMLElement); - cachedScrollEl = targetEl && isIonContent(targetEl) ? getElementRoot(targetEl).querySelector('.inner-scroll') : targetEl; + cachedScrollEl = + targetEl && isIonContent(targetEl) ? getElementRoot(targetEl).querySelector('.inner-scroll') : targetEl; /** * If expandToScroll is disabled, we need to swap From 1cf590430b99b4fd3402bef9039eacfcbb854978 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Tue, 18 Mar 2025 11:25:19 -0700 Subject: [PATCH 3/3] fix(sheet): improving performance more by only running DOM query logic when expandToScroll is diabled --- core/src/components/modal/gestures/sheet.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/src/components/modal/gestures/sheet.ts b/core/src/components/modal/gestures/sheet.ts index c1881731349..68df1a2ecaf 100644 --- a/core/src/components/modal/gestures/sheet.ts +++ b/core/src/components/modal/gestures/sheet.ts @@ -239,9 +239,11 @@ export const createSheetGesture = ( * this allows us to avoid querying the DOM for the target in onMove, * which would impact performance significantly. */ - const targetEl = findClosestIonContent(detail.event.target! as HTMLElement); - cachedScrollEl = - targetEl && isIonContent(targetEl) ? getElementRoot(targetEl).querySelector('.inner-scroll') : targetEl; + if (!expandToScroll) { + const targetEl = findClosestIonContent(detail.event.target! as HTMLElement); + cachedScrollEl = + targetEl && isIonContent(targetEl) ? getElementRoot(targetEl).querySelector('.inner-scroll') : targetEl; + } /** * If expandToScroll is disabled, we need to swap