Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sheet): change default visibility to original footer when expandToScroll is false #30310

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+5 −26
Open

fix(sheet): change default visibility to original footer when expandToScroll is false #30310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
79575c7
fix(modal):change visibility to original footer after animation finis…
kumibrr Mar 26, 2025
917b261
fix(sheet): changed default footer visibility to original
kumibrr Mar 26, 2025
25ed124
refactor(sheet): removed unnecesary footer visibility swap.
kumibrr Mar 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions core/src/components/modal/animations/ios.enter.ts
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to update this comment to reflect the changes. Maybe something like:

- * flickering. The original footer is hidden until the modal
- * is dismissed. This maintains the animation of the footer
+ * flickering. The cloned footer is hidden until the modal
+ * is moving. This maintains the animation of the footer

Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,10 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts: ModalAnimationOptio
*/
const ionFooterAlreadyAppended = baseEl.shadowRoot!.querySelector('ion-footer');
if (ionFooter && !ionFooterAlreadyAppended) {
const footerHeight = ionFooter.clientHeight;
const clonedFooter = ionFooter.cloneNode(true) as HTMLIonFooterElement;

clonedFooter.style.setProperty('display', 'none');
clonedFooter.setAttribute('aria-hidden', 'true');
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`);
}
});

Expand Down
10 changes: 2 additions & 8 deletions core/src/components/modal/animations/md.enter.ts
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to update this comment to reflect the changes. Maybe something like:

- * flickering. The original footer is hidden until the modal
- * is dismissed. This maintains the animation of the footer
+ * flickering. The cloned footer is hidden until the modal
+ * is moving. This maintains the animation of the footer

Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,10 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts: ModalAnimationOption
*/
const ionFooterAlreadyAppended = baseEl.shadowRoot!.querySelector('ion-footer');
if (ionFooter && !ionFooterAlreadyAppended) {
const footerHeight = ionFooter.clientHeight;
const clonedFooter = ionFooter.cloneNode(true) as HTMLIonFooterElement;

clonedFooter.style.setProperty('display', 'none');
clonedFooter.setAttribute('aria-hidden', 'true');
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`);
}
});

Expand Down
11 changes: 1 addition & 10 deletions core/src/components/modal/gestures/sheet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,16 +245,6 @@ export const createSheetGesture = (
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
* is dismissed, the footer dismisses with the modal
* and doesn't stay on the screen after the modal is gone.
*/
if (!expandToScroll) {
swapFooterVisibility('original');
}

/**
* If we are pulling down, then it is possible we are pulling on the content.
* We do not want scrolling to happen at the same time as the gesture.
Expand Down Expand Up @@ -462,6 +452,7 @@ export const createSheetGesture = (
.onFinish(
() => {
if (shouldRemainOpen) {
swapFooterVisibility('original');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make sure to keep the comment and the check.

Suggested change
swapFooterVisibility('original');
/**
* 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.
*/
if (!expandToScroll) {
swapFooterVisibility('original');
}

/**
* Once the snapping animation completes,
* we need to reset the animation to go
Expand Down