-
Notifications
You must be signed in to change notification settings - Fork 13.4k
fix(header): prevent flickering during iOS page transitions #30705
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,15 @@ | |
--opacity-scale: inherit; | ||
} | ||
|
||
/** | ||
* Override styles applied during the page transition to prevent | ||
* header flickering. | ||
*/ | ||
.header-collapse-fade.header-transitioning ion-toolbar { | ||
--background: transparent; | ||
--border-style: none; | ||
} | ||
|
||
// iOS Header - Collapse Condense | ||
// -------------------------------------------------- | ||
.header-collapse-condense { | ||
|
@@ -65,8 +74,6 @@ | |
* since it needs to blend in with the header above it. | ||
*/ | ||
.header-collapse-condense ion-toolbar { | ||
--background: var(--ion-background-color, #fff); | ||
|
||
z-index: 0; | ||
} | ||
|
||
|
@@ -93,6 +100,28 @@ | |
transition: all 0.2s ease-in-out; | ||
} | ||
|
||
/** | ||
* Large title toolbar should just use the content background | ||
* since it needs to blend in with the header above it. | ||
*/ | ||
.header-collapse-condense ion-toolbar, | ||
/** | ||
* Override styles applied during the page transition to prevent | ||
* header flickering. | ||
*/ | ||
.header-collapse-condense-inactive.header-transitioning:not(.header-collapse-condense) ion-toolbar { | ||
--background: var(--ion-background-color, #fff); | ||
} | ||
Comment on lines
+107
to
+114
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I grouped these together so they will always be the same background color since they are related to the condensed header. |
||
|
||
/** | ||
* Override styles applied during the page transition to prevent | ||
* header flickering. | ||
*/ | ||
.header-collapse-condense-inactive.header-transitioning:not(.header-collapse-condense) ion-toolbar { | ||
--border-style: none; | ||
--opacity-scale: 1; | ||
} | ||
|
||
.header-collapse-condense-inactive:not(.header-collapse-condense) ion-toolbar.in-toolbar ion-title, | ||
.header-collapse-condense-inactive:not(.header-collapse-condense) ion-toolbar.in-toolbar ion-buttons.buttons-collapse { | ||
opacity: 0; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,34 +18,51 @@ const focusController = createFocusController(); | |
|
||
// TODO(FW-2832): types | ||
|
||
/** | ||
* Executes the main page transition. | ||
* It also manages the lifecycle of header visibility (if any) | ||
* to prevent visual flickering in iOS. The flickering only | ||
* occurs for a condensed header that is placed above the content. | ||
* | ||
* @param opts Options for the transition. | ||
* @returns A promise that resolves when the transition is complete. | ||
*/ | ||
export const transition = (opts: TransitionOptions): Promise<TransitionResult> => { | ||
return new Promise((resolve, reject) => { | ||
writeTask(() => { | ||
beforeTransition(opts); | ||
runTransition(opts).then( | ||
(result) => { | ||
if (result.animation) { | ||
result.animation.destroy(); | ||
const transitioningInactiveHeader = getIosIonHeader(opts); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm grabbing the header here so we don't have to do query it twice within |
||
beforeTransition(opts, transitioningInactiveHeader); | ||
runTransition(opts) | ||
.then( | ||
(result) => { | ||
if (result.animation) { | ||
result.animation.destroy(); | ||
} | ||
afterTransition(opts); | ||
resolve(result); | ||
}, | ||
(error) => { | ||
afterTransition(opts); | ||
reject(error); | ||
} | ||
afterTransition(opts); | ||
resolve(result); | ||
}, | ||
(error) => { | ||
afterTransition(opts); | ||
reject(error); | ||
} | ||
); | ||
) | ||
.finally(() => { | ||
// Ensure that the header is restored to its original state. | ||
setHeaderTransitionClass(transitioningInactiveHeader, false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the class here since it should be removed regardless of the success of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed class at the end so the header can go back to its default, preventing any potential regressions. |
||
}); | ||
}); | ||
}); | ||
}; | ||
|
||
const beforeTransition = (opts: TransitionOptions) => { | ||
const beforeTransition = (opts: TransitionOptions, transitioningInactiveHeader: HTMLElement | null) => { | ||
const enteringEl = opts.enteringEl; | ||
const leavingEl = opts.leavingEl; | ||
|
||
focusController.saveViewFocus(leavingEl); | ||
|
||
setZIndex(enteringEl, leavingEl, opts.direction); | ||
// Prevent flickering of the header by adding a class. | ||
setHeaderTransitionClass(transitioningInactiveHeader, true); | ||
|
||
if (opts.showGoBack) { | ||
enteringEl.classList.add('can-go-back'); | ||
|
@@ -278,6 +295,40 @@ const setZIndex = ( | |
} | ||
}; | ||
|
||
/** | ||
* Add a class to ensure that the header (if any) | ||
* does not flicker during the transition. By adding the | ||
* transitioning class, we ensure that the header has | ||
* the necessary styles to prevent the following flickers: | ||
* 1. When entering a page with a condensed header, the | ||
* header should never be visible. However, | ||
* it briefly renders the background color while | ||
* the transition is occurring. | ||
* 2. When leaving a page with a condensed header, the | ||
* header has an opacity of 0 and the pages | ||
* have a z-index which causes the entering page to | ||
* briefly show it's content underneath the leaving page. | ||
* 3. When entering a page or leaving a page with a fade | ||
* header, the header should not have a background color. | ||
* However, it briefly shows the background color while | ||
* the transition is occurring. | ||
* | ||
* @param header The header element to modify. | ||
* @param isTransitioning Whether the transition is occurring. | ||
*/ | ||
const setHeaderTransitionClass = (header: HTMLElement | null, isTransitioning: boolean) => { | ||
if (!header) { | ||
return; | ||
} | ||
|
||
const transitionClass = 'header-transitioning'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm open to a different name. I'm not too happy with it but not sure what else to call it to be specific. |
||
if (isTransitioning) { | ||
header.classList.add(transitionClass); | ||
} else { | ||
header.classList.remove(transitionClass); | ||
} | ||
}; | ||
|
||
export const getIonPageElement = (element: HTMLElement) => { | ||
if (element.classList.contains('ion-page')) { | ||
return element; | ||
|
@@ -291,6 +342,32 @@ export const getIonPageElement = (element: HTMLElement) => { | |
return element; | ||
}; | ||
|
||
/** | ||
* Retrieves the ion-header element from a page based on the | ||
* direction of the transition. | ||
* | ||
* @param opts Options for the transition. | ||
* @returns The ion-header element or null if not found or not in 'ios' mode. | ||
*/ | ||
const getIosIonHeader = (opts: TransitionOptions): HTMLElement | null => { | ||
const enteringEl = opts.enteringEl; | ||
const leavingEl = opts.leavingEl; | ||
const direction = opts.direction; | ||
const mode = opts.mode; | ||
|
||
if (mode !== 'ios') { | ||
return null; | ||
} | ||
Comment on lines
+358
to
+360
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't see the benefit of querying out the header if the mode is |
||
|
||
const element = direction === 'back' ? leavingEl : enteringEl; | ||
|
||
if (!element) { | ||
return null; | ||
} | ||
|
||
return element.querySelector('ion-header'); | ||
}; | ||
|
||
export interface TransitionOptions extends NavOptions { | ||
progressCallback?: (ani: Animation | undefined) => void; | ||
baseEl: any; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the large title that is tied to the inactive header.