fix(react-router): use location keys for forward detection and prevent overlap in nested outlets

This commit is contained in:
ShaneK
2026-03-10 11:07:51 -07:00
parent 28f94d8761
commit b5ff48d248
3 changed files with 282 additions and 16 deletions

View File

@@ -83,12 +83,20 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
const viewStack = useRef(new ReactRouterViewStack());
const incomingRouteParams = useRef<Partial<RouteInfo> | null>(null);
/**
* Tracks URLs (pathname + search) that the user navigated away from via
* browser back. When a POP event's destination matches the top of this
* stack, it's a browser forward navigation. Cleared on PUSH (new
* navigation invalidates forward history, just like in the browser).
* Tracks location keys that the user navigated away from via browser back.
* When a POP event's destination key matches the top of this stack, it's a
* browser forward navigation. Uses React Router's unique location.key
* instead of URLs to correctly handle duplicate URLs in history (e.g.,
* navigating to /details, then /settings, then /details via routerLink,
* then pressing back).
* Cleared on PUSH (new navigation invalidates forward history).
*/
const forwardStack = useRef<string[]>([]);
/**
* Tracks the current location key so we can push it onto the forward stack
* when navigating back. Updated after each history change.
*/
const currentLocationKeyRef = useRef<string>(location.key);
const [routeInfo, setRouteInfo] = useState<RouteInfo>({
id: generateId('routeInfo'),
@@ -203,7 +211,7 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
const currentRoute = locationHistory.current.current();
const isForwardNavigation =
forwardStack.current.length > 0 &&
forwardStack.current[forwardStack.current.length - 1] === location.pathname + location.search;
forwardStack.current[forwardStack.current.length - 1] === location.key;
if (isForwardNavigation) {
forwardStack.current.pop();
@@ -213,8 +221,8 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
tab: tabToUse,
};
} else if (currentRoute && currentRoute.pushedByRoute) {
// Back navigation — record current URL for potential forward
forwardStack.current.push(currentRoute.pathname + (currentRoute.search || ''));
// Back navigation. Record current location key for potential forward
forwardStack.current.push(currentLocationKeyRef.current);
const prevInfo = locationHistory.current.findLastLocation(currentRoute);
incomingRouteParams.current = { ...prevInfo, routeAction: 'pop', routeDirection: 'back' };
} else {
@@ -346,6 +354,9 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
setRouteInfo(routeInfo);
}
// Update the current location key after processing the history change.
// This ensures the forward stack records the correct key when navigating back.
currentLocationKeyRef.current = location.key;
incomingRouteParams.current = null;
};
@@ -481,8 +492,8 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
const condition1 = routeInfo.lastPathname === routeInfo.pushedByRoute;
const condition2 = prevInfo.pathname === routeInfo.pushedByRoute && routeInfo.tab === '' && prevInfo.tab === '';
if (condition1 || condition2) {
// Record current URL so browser forward is detectable
forwardStack.current.push(routeInfo.pathname + (routeInfo.search || ''));
// Record the current location key so browser forward is detectable
forwardStack.current.push(currentLocationKeyRef.current);
navigate(-1);
} else {
/**

View File

@@ -33,7 +33,7 @@ interface StackManagerProps {
}
const isViewVisible = (el: HTMLElement) =>
!el.classList.contains('ion-page-invisible') && !el.classList.contains('ion-page-hidden');
!el.classList.contains('ion-page-invisible') && !el.classList.contains('ion-page-hidden') && el.style.display !== 'none';
const hideIonPageElement = (element: HTMLElement | undefined): void => {
if (element) {
@@ -44,6 +44,7 @@ const hideIonPageElement = (element: HTMLElement | undefined): void => {
const showIonPageElement = (element: HTMLElement | undefined): void => {
if (element) {
element.style.removeProperty('display');
element.classList.remove('ion-page-hidden');
element.removeAttribute('aria-hidden');
}
@@ -322,8 +323,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
const enteringEl = enteringViewItem.ionPageElement;
if (enteringEl) {
enteringEl.classList.remove('ion-page-hidden', 'ion-page-invisible');
enteringEl.removeAttribute('aria-hidden');
showIonPageElement(enteringEl);
enteringEl.classList.remove('ion-page-invisible');
}
this.forceUpdate();
@@ -417,7 +418,9 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
this.lastTransition = currentTransition;
this.transitionPage(routeInfo, enteringViewItem, leavingViewItem);
const shouldSkipAnimation = this.applySkipAnimationIfNeeded(enteringViewItem, leavingViewItem);
this.transitionPage(routeInfo, enteringViewItem, leavingViewItem, undefined, false, shouldSkipAnimation);
if (shouldUnmountLeavingViewItem && leavingViewItem && enteringViewItem !== leavingViewItem) {
leavingViewItem.mount = false;
@@ -547,6 +550,36 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
}
}
/**
* Determines whether to skip the transition animation and, if so, immediately
* hides the leaving view with inline `display:none`.
*
* Skips transitions in outlets nested inside a parent IonPage. These outlets
* render pages inside a parent page's content area. The MD animation shows
* both entering and leaving pages simultaneously, causing text overlap and
* nested scrollbars (each page has its own IonContent). Top-level outlets
* are unaffected and animate normally.
*
* Uses inline display:none rather than ion-page-hidden class because core's
* beforeTransition() removes ion-page-hidden via setPageHidden().
* Inline display:none survives that removal, keeping the page hidden
* until React unmounts it after ionViewDidLeave fires.
*/
private applySkipAnimationIfNeeded(
enteringViewItem: ViewItem,
leavingViewItem: ViewItem | undefined
): boolean {
const isNestedOutlet = !!this.routerOutletElement?.closest('.ion-page');
const shouldSkip = isNestedOutlet && !!leavingViewItem && enteringViewItem !== leavingViewItem;
if (shouldSkip && leavingViewItem?.ionPageElement) {
leavingViewItem.ionPageElement.style.setProperty('display', 'none');
leavingViewItem.ionPageElement.setAttribute('aria-hidden', 'true');
}
return shouldSkip;
}
/**
* Handles entering view with no ion-page element yet (waiting for render).
*/
@@ -609,7 +642,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
const latestLeavingView = this.context.findLeavingViewItemByRouteInfo(routeInfo, this.id) ?? leavingViewItem;
if (latestEnteringView?.ionPageElement) {
this.transitionPage(routeInfo, latestEnteringView, latestLeavingView ?? undefined);
const shouldSkipAnimation = this.applySkipAnimationIfNeeded(latestEnteringView, latestLeavingView ?? undefined);
this.transitionPage(routeInfo, latestEnteringView, latestLeavingView ?? undefined, undefined, false, shouldSkipAnimation);
if (shouldUnmountLeavingViewItem && latestLeavingView && latestEnteringView !== latestLeavingView) {
latestLeavingView.mount = false;
@@ -863,6 +897,17 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
return;
}
/**
* Don't let a nested element (e.g., ion-router-outlet with ionPage prop)
* override an existing IonPage registration when the existing element is
* an ancestor of the new one. This ensures ionPageElement always points
* to the outermost IonPage, which is needed to properly hide the entire
* page during back navigation (not just the inner outlet).
*/
if (oldPageElement && oldPageElement !== page && oldPageElement.isConnected && oldPageElement.contains(page)) {
return;
}
foundView.ionPageElement = page;
foundView.ionRoute = true;
@@ -1008,13 +1053,18 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
* @param progressAnimation Indicates if the transition is part of a
* gesture controlled animation (e.g., swipe to go back).
* Defaults to `false`.
* @param skipAnimation When true, forces `duration: 0` so the page
* swap is instant (no visible animation). Used for ionPage outlets
* and back navigations that unmount the leaving view to prevent
* overlapping content during the transition. Defaults to `false`.
*/
async transitionPage(
routeInfo: RouteInfo,
enteringViewItem: ViewItem,
leavingViewItem?: ViewItem,
direction?: 'forward' | 'back',
progressAnimation = false
progressAnimation = false,
skipAnimation = false
) {
const runCommit = async (enteringEl: HTMLElement, leavingEl?: HTMLElement) => {
const skipTransition = this.skipTransition;
@@ -1055,7 +1105,7 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
}
await routerOutlet.commit(enteringEl, leavingEl, {
duration: skipTransition || directionToUse === undefined ? 0 : undefined,
duration: skipTransition || skipAnimation || directionToUse === undefined ? 0 : undefined,
direction: directionToUse,
showGoBack: !!routeInfo.pushedByRoute,
progressAnimation,

View File

@@ -49,6 +49,76 @@ describe('Nested Params', () => {
cy.get('[data-testid="user-settings-param"]').should('contain', 'Settings view user: 123');
});
it('/nested-params > Navigate to user then back > No visual overlap during transition', () => {
cy.visit(`http://localhost:${port}/nested-params`);
cy.ionPageVisible('nested-params-landing');
// Navigate to user 99
cy.get('#go-to-user-99').click();
cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 99');
// Install an overlap detector that runs every animation frame.
// It checks whether the landing page and user page IonPage elements are
// simultaneously visible — which is the root cause of the overlap bug.
// We check the IonPage elements directly (not descendants) because CSS
// display is not inherited — a child's computed display can be 'block'
// even when a parent has display:none.
cy.window().then((win) => {
win.__overlapDetected = false;
const check = () => {
const landing = win.document.querySelector('[data-pageid="nested-params-landing"]');
const userPage = win.document.querySelector('[data-pageid^="nested-params-user-"]');
if (landing && userPage) {
const landingStyle = win.getComputedStyle(landing);
const userStyle = win.getComputedStyle(userPage);
const landingVisible = landingStyle.display !== 'none' && landingStyle.visibility !== 'hidden' && landingStyle.opacity !== '0';
const userVisible = userStyle.display !== 'none' && userStyle.visibility !== 'hidden' && userStyle.opacity !== '0';
if (landingVisible && userVisible) {
win.__overlapDetected = true;
}
}
if (!win.__overlapCheckDone) {
requestAnimationFrame(check);
}
};
requestAnimationFrame(check);
});
// Go back to landing
cy.go('back');
cy.ionPageVisible('nested-params-landing');
cy.get('[data-testid="user-layout-param"]').should('not.exist');
// Stop the observer and verify no overlap was detected
cy.window().then((win) => {
win.__overlapCheckDone = true;
expect(win.__overlapDetected).to.be.false;
});
});
it('/nested-params > Navigate to user, back, forward, back > Pages should not persist', () => {
cy.visit(`http://localhost:${port}/nested-params`);
cy.ionPageVisible('nested-params-landing');
// Navigate to user 99
cy.get('#go-to-user-99').click();
cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 99');
// Go back
cy.go('back');
cy.ionPageVisible('nested-params-landing');
// Go forward — wait for async POP event processing and transition to settle
cy.go('forward');
cy.wait(500);
cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 99');
// Go back again
cy.go('back');
cy.ionPageVisible('nested-params-landing');
cy.get('[data-testid="user-layout-param"]').should('not.exist');
});
it('/nested-params > Different users should have different params', () => {
cy.visit(`http://localhost:${port}/nested-params`);
cy.ionPageVisible('nested-params-landing');
@@ -67,4 +137,139 @@ describe('Nested Params', () => {
cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 99');
cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 99');
});
it('/nested-params > Full back navigation from sibling routes to root > Root page should show', () => {
// Start at root
cy.visit(`http://localhost:${port}/`);
cy.ionPageVisible('home');
// Navigate to nested-params
cy.contains('ion-item', 'Nested Params').click();
cy.ionPageVisible('nested-params-landing');
// Navigate to user 99 details
cy.get('#go-to-user-99').click();
cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 99');
cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 99');
// Go to settings
cy.get('#go-to-settings').click();
cy.get('[data-testid="user-settings-param"]').should('contain', 'Settings view user: 99');
// Hit "Back to Details" (this is a forward push via routerLink)
cy.contains('ion-button', 'Back to Details').click();
cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 99');
// Browser back repeatedly to root
// Back 1: details -> settings
cy.go('back');
cy.get('[data-testid="user-settings-param"]').should('contain', 'Settings view user: 99');
// Back 2: settings -> details
cy.go('back');
cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 99');
// Back 3: details -> nested-params landing
cy.go('back');
cy.ionPageVisible('nested-params-landing');
// Back 4: nested-params -> root
cy.go('back');
cy.ionPageVisible('home');
cy.get('[data-testid="user-layout-param"]').should('not.exist');
});
it('/nested-params > Sibling route transitions (details <-> settings) > No visual overlap', () => {
// Start directly on the details page
cy.visit(`http://localhost:${port}/nested-params/user/99/details`);
cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 99');
// Install overlap detector that checks whether details and settings IonPage
// elements are simultaneously visible inside the nested outlet.
cy.window().then((win) => {
win.__siblingOverlapDetected = false;
const check = () => {
const details = win.document.querySelector('[data-pageid="nested-params-details"]');
const settings = win.document.querySelector('[data-pageid="nested-params-settings"]');
if (details && settings) {
const detailsStyle = win.getComputedStyle(details);
const settingsStyle = win.getComputedStyle(settings);
const detailsVisible = detailsStyle.display !== 'none' && detailsStyle.visibility !== 'hidden' && detailsStyle.opacity !== '0';
const settingsVisible = settingsStyle.display !== 'none' && settingsStyle.visibility !== 'hidden' && settingsStyle.opacity !== '0';
if (detailsVisible && settingsVisible) {
win.__siblingOverlapDetected = true;
}
}
if (!win.__siblingOverlapCheckDone) {
requestAnimationFrame(check);
}
};
requestAnimationFrame(check);
});
// Navigate details -> settings
cy.get('#go-to-settings').click();
cy.get('[data-testid="user-settings-param"]').should('contain', 'Settings view user: 99');
// Navigate settings -> details
cy.contains('ion-button', 'Back to Details').click();
cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 99');
// Stop the observer and verify no overlap was detected in either direction
cy.window().then((win) => {
win.__siblingOverlapCheckDone = true;
expect(win.__siblingOverlapDetected).to.be.false;
});
});
it('/nested-params > Sibling route transitions > No nested scrollbars during transition', () => {
// Start directly on the details page
cy.visit(`http://localhost:${port}/nested-params/user/99/details`);
cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 99');
// Install a detector that checks for multiple visible ion-content elements
// inside the nested outlet. Multiple visible ion-content elements cause
// nested scrollbars during transitions.
cy.window().then((win) => {
win.__nestedScrollbarsDetected = false;
const check = () => {
const outlet = win.document.querySelector('#nested-params-user-outlet');
if (outlet) {
const contents = outlet.querySelectorAll('ion-content');
let visibleCount = 0;
contents.forEach((content) => {
// Check the parent IonPage element's visibility
const page = content.closest('.ion-page');
if (page) {
const style = win.getComputedStyle(page);
if (style.display !== 'none' && style.visibility !== 'hidden' && style.opacity !== '0') {
visibleCount++;
}
}
});
if (visibleCount > 1) {
win.__nestedScrollbarsDetected = true;
}
}
if (!win.__nestedScrollbarsCheckDone) {
requestAnimationFrame(check);
}
};
requestAnimationFrame(check);
});
// Navigate details -> settings
cy.get('#go-to-settings').click();
cy.get('[data-testid="user-settings-param"]').should('contain', 'Settings view user: 99');
// Navigate settings -> details
cy.contains('ion-button', 'Back to Details').click();
cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 99');
// Stop the observer and verify no nested scrollbars were detected
cy.window().then((win) => {
win.__nestedScrollbarsCheckDone = true;
expect(win.__nestedScrollbarsDetected).to.be.false;
});
});
});