perf(sheet): fixing performance regression on modal sheets when expandToScroll is false (#30267)

Issue number: internal

---------

<!-- 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. -->
Currently, when a sheet is moved while `expandToScroll` is disabled, the
DOM is queried excessively causing performance degradation.


## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

We now cache the targeted element in `onStart` and refer to it in
`onMove` and `onEnd`, preventing over-querying the DOM

## 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/docs/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. -->
This regression was introduced in #30257 and quickly highlighted by a
member of the community
This commit is contained in:
Shane
2025-03-19 11:55:46 -07:00
committed by GitHub
parent 2018a045ab
commit c4b9212640

View File

@ -1,6 +1,6 @@
import { isIonContent, findClosestIonContent } from '@utils/content'; import { findClosestIonContent, isIonContent } from '@utils/content';
import { createGesture } from '@utils/gesture'; 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 { FOCUS_TRAP_DISABLE_CLASS } from '@utils/overlays';
import type { Animation } from '../../../interface'; import type { Animation } from '../../../interface';
@ -83,6 +83,7 @@ export const createSheetGesture = (
let currentBreakpoint = initialBreakpoint; let currentBreakpoint = initialBreakpoint;
let offset = 0; let offset = 0;
let canDismissBlocksGesture = false; let canDismissBlocksGesture = false;
let cachedScrollEl: HTMLElement | null = null;
const canDismissMaxStep = 0.95; const canDismissMaxStep = 0.95;
const maxBreakpoint = breakpoints[breakpoints.length - 1]; const maxBreakpoint = breakpoints[breakpoints.length - 1];
const minBreakpoint = breakpoints[0]; const minBreakpoint = breakpoints[0];
@ -233,6 +234,17 @@ export const createSheetGesture = (
*/ */
canDismissBlocksGesture = baseEl.canDismiss !== undefined && baseEl.canDismiss !== true && minBreakpoint === 0; 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.
*/
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 * If expandToScroll is disabled, we need to swap
* the footer visibility to the original, so if the modal * the footer visibility to the original, so if the modal
@ -267,14 +279,9 @@ export const createSheetGesture = (
* If `expandToScroll` is disabled, and an upwards swipe gesture is done within * If `expandToScroll` is disabled, and an upwards swipe gesture is done within
* the scrollable content, we should not allow the swipe gesture to continue. * the scrollable content, we should not allow the swipe gesture to continue.
*/ */
if (!expandToScroll && detail.deltaY <= 0) { if (!expandToScroll && detail.deltaY <= 0 && cachedScrollEl) {
const contentEl = findClosestIonContent(detail.event.target! as HTMLElement);
const scrollEl =
contentEl && isIonContent(contentEl) ? getElementRoot(contentEl).querySelector('.inner-scroll') : contentEl;
if (scrollEl) {
return; return;
} }
}
/** /**
* If we are pulling down, then it is possible we are pulling on the content. * If we are pulling down, then it is possible we are pulling on the content.
@ -334,13 +341,9 @@ export const createSheetGesture = (
* function to be called if the user is trying to swipe content upwards and the content * function to be called if the user is trying to swipe content upwards and the content
* is not scrolled to the top. * is not scrolled to the top.
*/ */
if (!expandToScroll && detail.deltaY <= 0 && findClosestIonContent(detail.event.target! as HTMLElement)) { if (!expandToScroll && detail.deltaY <= 0 && cachedScrollEl && cachedScrollEl.scrollTop > 0) {
const contentEl = findClosestIonContent(detail.event.target! as HTMLElement)!;
const scrollEl = isIonContent(contentEl) ? getElementRoot(contentEl).querySelector('.inner-scroll') : contentEl;
if (scrollEl!.scrollTop > 0) {
return; return;
} }
}
/** /**
* When the gesture releases, we need to determine * When the gesture releases, we need to determine