mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2025-11-09 16:16:41 +08:00
fix(sheet): move sheet footers instead of cloning while dragging (#30433)
Issue number: resolves several (listed at the bottom) + 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 expand to scroll is disabled in a sheet modal, we duplicate the footer on drag and show a cloned version in the shadow DOM instead of the original. This causes many issues (described in the Other Information section), especially because often times the cloned version of the footer would stick around instead of the original version, which made any event listeners in the footer (and styling) broken by default instead of only while dragging. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> We are now following [method 2 of the design doc](https://github.com/ionic-team/ionic-framework-design-documents/blob/main/projects/ionic-framework/components/modal/0003-sheet-modal-scroll.md#approach-2-move-the-footer), with minor deviation. Now we try to eliminate the shaky behavior in the footer by setting the footer to be absolutely positioned on the page and adding padding to the bottom of the modal while dragging is happening to offset the missing footer content. We are additionally performing some extra logic to swap back when the modal is dragged below the height of the footer so that it collapses correctly visually. Note this is a minor variation in method two from the design doc because I moved the footer to the body instead of to the parent modal. This is because the parent modal will prevent anything not in its ion-page from displaying, but it seems to work fine. As a side-effect of this, I had to add an extra class and support for that class in a few areas so we could identify footers that belonged to modals while they were moving and apply applicable classes. Additionally with this change I was able to remove all of the extra code in the leave/enter animations because we no longer need to worry about managing a clone of an element there. This allows me to contain all code relating to behavior of the footer to `sheet.ts`. ## 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 This refactor in how the footer combats shakiness should resolve many issues caused by the previous implementation, including the following: - this change no longer has the footer being swapped out, effectively breaking event listeners, which fixes #30315 - this change no longer has the footer being duplicated at all, which fixes #30341 - this change no longer has the footer (sometimes) being moved to the shadow DOM, which fixes #30312 **Current dev build**: `8.5.8-dev.11748530383.18b4e301` --------- Co-authored-by: Israel de la Barrera <israeldlbarreral@pm.me> Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com> Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
This commit is contained in:
Shane
@ -41,50 +41,7 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts: ModalAnimationOptio
|
||||
.addElement(baseEl)
|
||||
.easing('cubic-bezier(0.32,0.72,0,1)')
|
||||
.duration(500)
|
||||
.addAnimation([wrapperAnimation])
|
||||
.beforeAddWrite(() => {
|
||||
if (expandToScroll) {
|
||||
// Scroll can only be done when the modal is fully expanded.
|
||||
return;
|
||||
}
|
||||
|
||||
/**
|
||||
* There are some browsers that causes flickering when
|
||||
* dragging the content when scroll is enabled at every
|
||||
* breakpoint. This is due to the wrapper element being
|
||||
* transformed off the screen and having a snap animation.
|
||||
*
|
||||
* A workaround is to clone the footer element and append
|
||||
* it outside of the wrapper element. This way, the footer
|
||||
* is still visible and the drag can be done without
|
||||
* flickering. The original footer is hidden until the modal
|
||||
* is dismissed. This maintains the animation of the footer
|
||||
* when the modal is dismissed.
|
||||
*
|
||||
* The workaround needs to be done before the animation starts
|
||||
* so there are no flickering issues.
|
||||
*/
|
||||
const ionFooter = baseEl.querySelector('ion-footer');
|
||||
/**
|
||||
* This check is needed to prevent more than one footer
|
||||
* from being appended to the shadow root.
|
||||
* Otherwise, iOS and MD enter animations would append
|
||||
* the footer twice.
|
||||
*/
|
||||
const ionFooterAlreadyAppended = baseEl.shadowRoot!.querySelector('ion-footer');
|
||||
if (ionFooter && !ionFooterAlreadyAppended) {
|
||||
const footerHeight = ionFooter.clientHeight;
|
||||
const clonedFooter = ionFooter.cloneNode(true) as HTMLIonFooterElement;
|
||||
|
||||
baseEl.shadowRoot!.appendChild(clonedFooter);
|
||||
ionFooter.style.setProperty('display', 'none');
|
||||
ionFooter.setAttribute('aria-hidden', 'true');
|
||||
|
||||
// Padding is added to prevent some content from being hidden.
|
||||
const page = baseEl.querySelector('.ion-page') as HTMLElement;
|
||||
page.style.setProperty('padding-bottom', `${footerHeight}px`);
|
||||
}
|
||||
});
|
||||
.addAnimation([wrapperAnimation]);
|
||||
|
||||
if (contentAnimation) {
|
||||
baseAnimation.addAnimation(contentAnimation);
|
||||
|
||||
@ -19,7 +19,7 @@ const createLeaveAnimation = () => {
|
||||
* iOS Modal Leave Animation
|
||||
*/
|
||||
export const iosLeaveAnimation = (baseEl: HTMLElement, opts: ModalAnimationOptions, duration = 500): Animation => {
|
||||
const { presentingEl, currentBreakpoint, expandToScroll } = opts;
|
||||
const { presentingEl, currentBreakpoint } = opts;
|
||||
const root = getElementRoot(baseEl);
|
||||
const { wrapperAnimation, backdropAnimation } =
|
||||
currentBreakpoint !== undefined ? createSheetLeaveAnimation(opts) : createLeaveAnimation();
|
||||
@ -32,33 +32,7 @@ export const iosLeaveAnimation = (baseEl: HTMLElement, opts: ModalAnimationOptio
|
||||
.addElement(baseEl)
|
||||
.easing('cubic-bezier(0.32,0.72,0,1)')
|
||||
.duration(duration)
|
||||
.addAnimation(wrapperAnimation)
|
||||
.beforeAddWrite(() => {
|
||||
if (expandToScroll) {
|
||||
// Scroll can only be done when the modal is fully expanded.
|
||||
return;
|
||||
}
|
||||
|
||||
/**
|
||||
* If expandToScroll is disabled, we need to swap
|
||||
* the visibility to the original, so the footer
|
||||
* dismisses with the modal and doesn't stay
|
||||
* until the modal is removed from the DOM.
|
||||
*/
|
||||
const ionFooter = baseEl.querySelector('ion-footer');
|
||||
if (ionFooter) {
|
||||
const clonedFooter = baseEl.shadowRoot!.querySelector('ion-footer')!;
|
||||
|
||||
ionFooter.style.removeProperty('display');
|
||||
ionFooter.removeAttribute('aria-hidden');
|
||||
|
||||
clonedFooter.style.setProperty('display', 'none');
|
||||
clonedFooter.setAttribute('aria-hidden', 'true');
|
||||
|
||||
const page = baseEl.querySelector('.ion-page') as HTMLElement;
|
||||
page.style.removeProperty('padding-bottom');
|
||||
}
|
||||
});
|
||||
.addAnimation(wrapperAnimation);
|
||||
|
||||
if (presentingEl) {
|
||||
const isMobile = window.innerWidth < 768;
|
||||
|
||||
@ -37,56 +37,13 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts: ModalAnimationOption
|
||||
|
||||
// The content animation is only added if scrolling is enabled for
|
||||
// all the breakpoints.
|
||||
expandToScroll && contentAnimation?.addElement(baseEl.querySelector('.ion-page')!);
|
||||
!expandToScroll && contentAnimation?.addElement(baseEl.querySelector('.ion-page')!);
|
||||
|
||||
const baseAnimation = createAnimation()
|
||||
.addElement(baseEl)
|
||||
.easing('cubic-bezier(0.36,0.66,0.04,1)')
|
||||
.duration(280)
|
||||
.addAnimation([backdropAnimation, wrapperAnimation])
|
||||
.beforeAddWrite(() => {
|
||||
if (expandToScroll) {
|
||||
// Scroll can only be done when the modal is fully expanded.
|
||||
return;
|
||||
}
|
||||
|
||||
/**
|
||||
* There are some browsers that causes flickering when
|
||||
* dragging the content when scroll is enabled at every
|
||||
* breakpoint. This is due to the wrapper element being
|
||||
* transformed off the screen and having a snap animation.
|
||||
*
|
||||
* A workaround is to clone the footer element and append
|
||||
* it outside of the wrapper element. This way, the footer
|
||||
* is still visible and the drag can be done without
|
||||
* flickering. The original footer is hidden until the modal
|
||||
* is dismissed. This maintains the animation of the footer
|
||||
* when the modal is dismissed.
|
||||
*
|
||||
* The workaround needs to be done before the animation starts
|
||||
* so there are no flickering issues.
|
||||
*/
|
||||
const ionFooter = baseEl.querySelector('ion-footer');
|
||||
/**
|
||||
* This check is needed to prevent more than one footer
|
||||
* from being appended to the shadow root.
|
||||
* Otherwise, iOS and MD enter animations would append
|
||||
* the footer twice.
|
||||
*/
|
||||
const ionFooterAlreadyAppended = baseEl.shadowRoot!.querySelector('ion-footer');
|
||||
if (ionFooter && !ionFooterAlreadyAppended) {
|
||||
const footerHeight = ionFooter.clientHeight;
|
||||
const clonedFooter = ionFooter.cloneNode(true) as HTMLIonFooterElement;
|
||||
|
||||
baseEl.shadowRoot!.appendChild(clonedFooter);
|
||||
ionFooter.style.setProperty('display', 'none');
|
||||
ionFooter.setAttribute('aria-hidden', 'true');
|
||||
|
||||
// Padding is added to prevent some content from being hidden.
|
||||
const page = baseEl.querySelector('.ion-page') as HTMLElement;
|
||||
page.style.setProperty('padding-bottom', `${footerHeight}px`);
|
||||
}
|
||||
});
|
||||
.addAnimation([backdropAnimation, wrapperAnimation]);
|
||||
|
||||
if (contentAnimation) {
|
||||
baseAnimation.addAnimation(contentAnimation);
|
||||
|
||||
@ -21,7 +21,7 @@ const createLeaveAnimation = () => {
|
||||
* Md Modal Leave Animation
|
||||
*/
|
||||
export const mdLeaveAnimation = (baseEl: HTMLElement, opts: ModalAnimationOptions): Animation => {
|
||||
const { currentBreakpoint, expandToScroll } = opts;
|
||||
const { currentBreakpoint } = opts;
|
||||
const root = getElementRoot(baseEl);
|
||||
const { wrapperAnimation, backdropAnimation } =
|
||||
currentBreakpoint !== undefined ? createSheetLeaveAnimation(opts) : createLeaveAnimation();
|
||||
@ -32,33 +32,7 @@ export const mdLeaveAnimation = (baseEl: HTMLElement, opts: ModalAnimationOption
|
||||
const baseAnimation = createAnimation()
|
||||
.easing('cubic-bezier(0.47,0,0.745,0.715)')
|
||||
.duration(200)
|
||||
.addAnimation([backdropAnimation, wrapperAnimation])
|
||||
.beforeAddWrite(() => {
|
||||
if (expandToScroll) {
|
||||
// Scroll can only be done when the modal is fully expanded.
|
||||
return;
|
||||
}
|
||||
|
||||
/**
|
||||
* If expandToScroll is disabled, we need to swap
|
||||
* the visibility to the original, so the footer
|
||||
* dismisses with the modal and doesn't stay
|
||||
* until the modal is removed from the DOM.
|
||||
*/
|
||||
const ionFooter = baseEl.querySelector('ion-footer');
|
||||
if (ionFooter) {
|
||||
const clonedFooter = baseEl.shadowRoot!.querySelector('ion-footer')!;
|
||||
|
||||
ionFooter.style.removeProperty('display');
|
||||
ionFooter.removeAttribute('aria-hidden');
|
||||
|
||||
clonedFooter.style.setProperty('display', 'none');
|
||||
clonedFooter.setAttribute('aria-hidden', 'true');
|
||||
|
||||
const page = baseEl.querySelector('.ion-page') as HTMLElement;
|
||||
page.style.removeProperty('padding-bottom');
|
||||
}
|
||||
});
|
||||
.addAnimation([backdropAnimation, wrapperAnimation]);
|
||||
|
||||
return baseAnimation;
|
||||
};
|
||||
|
||||
@ -84,6 +84,9 @@ export const createSheetGesture = (
|
||||
let offset = 0;
|
||||
let canDismissBlocksGesture = false;
|
||||
let cachedScrollEl: HTMLElement | null = null;
|
||||
let cachedFooterEl: HTMLIonFooterElement | null = null;
|
||||
let cachedFooterYPosition: number | null = null;
|
||||
let currentFooterState: 'moving' | 'stationary' | null = null;
|
||||
const canDismissMaxStep = 0.95;
|
||||
const maxBreakpoint = breakpoints[breakpoints.length - 1];
|
||||
const minBreakpoint = breakpoints[0];
|
||||
@ -118,33 +121,74 @@ export const createSheetGesture = (
|
||||
};
|
||||
|
||||
/**
|
||||
* Toggles the visible modal footer when `expandToScroll` is disabled.
|
||||
* @param footer The footer to show.
|
||||
* Toggles the footer to an absolute position while moving to prevent
|
||||
* it from shaking while the sheet is being dragged.
|
||||
* @param newPosition Whether the footer is in a moving or stationary position.
|
||||
*/
|
||||
const swapFooterVisibility = (footer: 'original' | 'cloned') => {
|
||||
const originalFooter = baseEl.querySelector('ion-footer') as HTMLIonFooterElement | null;
|
||||
|
||||
if (!originalFooter) {
|
||||
return;
|
||||
const swapFooterPosition = (newPosition: 'moving' | 'stationary') => {
|
||||
if (!cachedFooterEl) {
|
||||
cachedFooterEl = baseEl.querySelector('ion-footer') as HTMLIonFooterElement | null;
|
||||
if (!cachedFooterEl) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const clonedFooter = wrapperEl.nextElementSibling as HTMLIonFooterElement;
|
||||
const footerToHide = footer === 'original' ? clonedFooter : originalFooter;
|
||||
const footerToShow = footer === 'original' ? originalFooter : clonedFooter;
|
||||
const page = baseEl.querySelector('.ion-page') as HTMLElement | null;
|
||||
|
||||
footerToShow.style.removeProperty('display');
|
||||
footerToShow.removeAttribute('aria-hidden');
|
||||
currentFooterState = newPosition;
|
||||
if (newPosition === 'stationary') {
|
||||
// Reset positioning styles to allow normal document flow
|
||||
cachedFooterEl.classList.remove('modal-footer-moving');
|
||||
cachedFooterEl.style.removeProperty('position');
|
||||
cachedFooterEl.style.removeProperty('width');
|
||||
cachedFooterEl.style.removeProperty('height');
|
||||
cachedFooterEl.style.removeProperty('top');
|
||||
cachedFooterEl.style.removeProperty('left');
|
||||
page?.style.removeProperty('padding-bottom');
|
||||
|
||||
const page = baseEl.querySelector('.ion-page') as HTMLElement;
|
||||
if (footer === 'original') {
|
||||
page.style.removeProperty('padding-bottom');
|
||||
// Move to page
|
||||
page?.appendChild(cachedFooterEl);
|
||||
} else {
|
||||
const pagePadding = footerToShow.clientHeight;
|
||||
page.style.setProperty('padding-bottom', `${pagePadding}px`);
|
||||
}
|
||||
// Get both the footer and document body positions
|
||||
const cachedFooterElRect = cachedFooterEl.getBoundingClientRect();
|
||||
const bodyRect = document.body.getBoundingClientRect();
|
||||
|
||||
footerToHide.style.setProperty('display', 'none');
|
||||
footerToHide.setAttribute('aria-hidden', 'true');
|
||||
// Add padding to the parent element to prevent content from being hidden
|
||||
// when the footer is positioned absolutely. This has to be done before we
|
||||
// make the footer absolutely positioned or we may accidentally cause the
|
||||
// sheet to scroll.
|
||||
const footerHeight = cachedFooterEl.clientHeight;
|
||||
page?.style.setProperty('padding-bottom', `${footerHeight}px`);
|
||||
|
||||
// Apply positioning styles to keep footer at bottom
|
||||
cachedFooterEl.classList.add('modal-footer-moving');
|
||||
|
||||
// Calculate absolute position relative to body
|
||||
// We need to subtract the body's offsetTop to get true position within document.body
|
||||
const absoluteTop = cachedFooterElRect.top - bodyRect.top;
|
||||
const absoluteLeft = cachedFooterElRect.left - bodyRect.left;
|
||||
|
||||
// Capture the footer's current dimensions and hard code them during the drag
|
||||
cachedFooterEl.style.setProperty('position', 'absolute');
|
||||
cachedFooterEl.style.setProperty('width', `${cachedFooterEl.clientWidth}px`);
|
||||
cachedFooterEl.style.setProperty('height', `${cachedFooterEl.clientHeight}px`);
|
||||
cachedFooterEl.style.setProperty('top', `${absoluteTop}px`);
|
||||
cachedFooterEl.style.setProperty('left', `${absoluteLeft}px`);
|
||||
|
||||
// Also cache the footer Y position, which we use to determine if the
|
||||
// sheet has been moved below the footer. When that happens, we need to swap
|
||||
// the position back so it will collapse correctly.
|
||||
cachedFooterYPosition = absoluteTop;
|
||||
// If there's a toolbar, we need to combine the toolbar height with the footer position
|
||||
// because the toolbar moves with the drag handle, so when it starts overlapping the footer,
|
||||
// we need to account for that.
|
||||
const toolbar = baseEl.querySelector('ion-toolbar') as HTMLIonToolbarElement | null;
|
||||
if (toolbar) {
|
||||
cachedFooterYPosition -= toolbar.clientHeight;
|
||||
}
|
||||
|
||||
document.body.appendChild(cachedFooterEl);
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
@ -247,12 +291,11 @@ export const createSheetGesture = (
|
||||
|
||||
/**
|
||||
* If expandToScroll is disabled, we need to swap
|
||||
* the footer visibility to the original, so if the modal
|
||||
* is dismissed, the footer dismisses with the modal
|
||||
* and doesn't stay on the screen after the modal is gone.
|
||||
* the footer position to moving so that it doesn't shake
|
||||
* while the sheet is being dragged.
|
||||
*/
|
||||
if (!expandToScroll) {
|
||||
swapFooterVisibility('original');
|
||||
swapFooterPosition('moving');
|
||||
}
|
||||
|
||||
/**
|
||||
@ -275,6 +318,21 @@ export const createSheetGesture = (
|
||||
};
|
||||
|
||||
const onMove = (detail: GestureDetail) => {
|
||||
/**
|
||||
* If `expandToScroll` is disabled, we need to see if we're currently below
|
||||
* the footer element and the footer is in a stationary position. If so,
|
||||
* we need to make the stationary the original position so that the footer
|
||||
* collapses with the sheet.
|
||||
*/
|
||||
if (!expandToScroll && cachedFooterYPosition !== null && currentFooterState !== null) {
|
||||
// Check if we need to swap the footer position
|
||||
if (detail.currentY >= cachedFooterYPosition && currentFooterState === 'moving') {
|
||||
swapFooterPosition('stationary');
|
||||
} else if (detail.currentY < cachedFooterYPosition && currentFooterState === 'stationary') {
|
||||
swapFooterPosition('moving');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* If `expandToScroll` is disabled, and an upwards swipe gesture is done within
|
||||
* the scrollable content, we should not allow the swipe gesture to continue.
|
||||
@ -431,15 +489,6 @@ export const createSheetGesture = (
|
||||
*/
|
||||
gesture.enable(false);
|
||||
|
||||
/**
|
||||
* If expandToScroll is disabled, we need to swap
|
||||
* the footer visibility to the cloned one so the footer
|
||||
* doesn't flicker when the sheet's height is animated.
|
||||
*/
|
||||
if (!expandToScroll && shouldRemainOpen) {
|
||||
swapFooterVisibility('cloned');
|
||||
}
|
||||
|
||||
if (shouldPreventDismiss) {
|
||||
handleCanDismiss(baseEl, animation);
|
||||
} else if (!shouldRemainOpen) {
|
||||
@ -457,11 +506,31 @@ export const createSheetGesture = (
|
||||
contentEl.scrollY = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* If expandToScroll is disabled and we're animating
|
||||
* to close the sheet, we need to swap
|
||||
* the footer position to stationary so that it
|
||||
* will collapse correctly. We cannot just always swap
|
||||
* here or it'll be jittery while animating movement.
|
||||
*/
|
||||
if (!expandToScroll && snapToBreakpoint === 0) {
|
||||
swapFooterPosition('stationary');
|
||||
}
|
||||
|
||||
return new Promise<void>((resolve) => {
|
||||
animation
|
||||
.onFinish(
|
||||
() => {
|
||||
if (shouldRemainOpen) {
|
||||
/**
|
||||
* If expandToScroll is disabled, we need to swap
|
||||
* the footer position to stationary so that it
|
||||
* will act as it would by default.
|
||||
*/
|
||||
if (!expandToScroll) {
|
||||
swapFooterPosition('stationary');
|
||||
}
|
||||
|
||||
/**
|
||||
* Once the snapping animation completes,
|
||||
* we need to reset the animation to go
|
||||
|
||||
@ -87,16 +87,3 @@
|
||||
:host(.modal-sheet) .modal-wrapper {
|
||||
@include border-radius(var(--border-radius), var(--border-radius), 0, 0);
|
||||
}
|
||||
|
||||
// iOS Sheet Modal - Scroll at all breakpoints
|
||||
// --------------------------------------------------
|
||||
|
||||
/**
|
||||
* Sheet modals require an additional padding as mentioned in the
|
||||
* `core.scss` file. However, there's a workaround that requires
|
||||
* a cloned footer to be added to the modal. This is only necessary
|
||||
* because the core styles are not being applied to the cloned footer.
|
||||
*/
|
||||
:host(.modal-sheet.modal-no-expand-scroll) ion-footer ion-toolbar:first-of-type {
|
||||
padding-top: $modal-sheet-padding-top;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user