Skip to content

Commit 511a6c7

Browse files
committed
fix(overlays): clean up aria-hidden when overlay is removed without dismis and clean up empty path reuse issue
1 parent e49eb22 commit 511a6c7

13 files changed

Lines changed: 450 additions & 1 deletion

File tree

core/src/components/action-sheet/action-sheet.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
present,
1616
safeCall,
1717
setOverlayId,
18+
cleanupRootFocusTrapAccessibility,
1819
} from '@utils/overlays';
1920
import { getClassMap } from '@utils/theme';
2021

@@ -460,6 +461,15 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
460461
this.gesture = undefined;
461462
}
462463
this.triggerController.removeClickListener();
464+
465+
if (this.presented) {
466+
const el = this.el;
467+
setTimeout(() => {
468+
if (!el.isConnected) {
469+
cleanupRootFocusTrapAccessibility();
470+
}
471+
}, 0);
472+
}
463473
}
464474

465475
componentWillLoad() {

core/src/components/alert/alert.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
present,
1818
safeCall,
1919
setOverlayId,
20+
cleanupRootFocusTrapAccessibility,
2021
} from '@utils/overlays';
2122
import { sanitizeDOMString } from '@utils/sanitization';
2223
import { getClassMap } from '@utils/theme';
@@ -368,6 +369,15 @@ export class Alert implements ComponentInterface, OverlayInterface {
368369
this.gesture.destroy();
369370
this.gesture = undefined;
370371
}
372+
373+
if (this.presented) {
374+
const el = this.el;
375+
setTimeout(() => {
376+
if (!el.isConnected) {
377+
cleanupRootFocusTrapAccessibility();
378+
}
379+
}, 0);
380+
}
371381
}
372382

373383
componentDidLoad() {

core/src/components/loading/loading.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
createDelegateController,
1313
createTriggerController,
1414
setOverlayId,
15+
cleanupRootFocusTrapAccessibility,
1516
} from '@utils/overlays';
1617
import { sanitizeDOMString } from '@utils/sanitization';
1718
import { getClassMap } from '@utils/theme';
@@ -242,6 +243,15 @@ export class Loading implements ComponentInterface, OverlayInterface {
242243

243244
disconnectedCallback() {
244245
this.triggerController.removeClickListener();
246+
247+
if (this.presented) {
248+
const el = this.el;
249+
setTimeout(() => {
250+
if (!el.isConnected) {
251+
cleanupRootFocusTrapAccessibility();
252+
}
253+
}, 0);
254+
}
245255
}
246256

247257
/**

core/src/components/modal/modal.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
createTriggerController,
1818
setOverlayId,
1919
FOCUS_TRAP_DISABLE_CLASS,
20+
cleanupRootFocusTrapAccessibility,
2021
} from '@utils/overlays';
2122
import { getClassMap } from '@utils/theme';
2223
import { deepReady, waitForMount } from '@utils/transition';
@@ -458,6 +459,21 @@ export class Modal implements ComponentInterface, OverlayInterface {
458459
// Also called in dismiss() — intentional dual cleanup covers both
459460
// dismiss-then-remove and direct DOM removal without dismiss.
460461
this.cleanupSafeAreaOverrides();
462+
463+
// If the modal was removed from the DOM while still presented
464+
// (e.g., a framework unmounted it during a route change without
465+
// calling dismiss()), clean up aria-hidden on the root outlet
466+
// so the app remains accessible.
467+
// Deferred to a microtask so DOM moves (disconnect + reconnect in
468+
// the same tick, e.g. React portals) don't prematurely clear it.
469+
if (this.presented) {
470+
const el = this.el;
471+
setTimeout(() => {
472+
if (!el.isConnected) {
473+
cleanupRootFocusTrapAccessibility();
474+
}
475+
}, 0);
476+
}
461477
}
462478

463479
componentWillLoad() {

core/src/components/popover/popover.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
present,
1414
setOverlayId,
1515
FOCUS_TRAP_DISABLE_CLASS,
16+
cleanupRootFocusTrapAccessibility,
1617
} from '@utils/overlays';
1718
import { isPlatform } from '@utils/platform';
1819
import { getClassMap } from '@utils/theme';
@@ -367,6 +368,15 @@ export class Popover implements ComponentInterface, PopoverInterface {
367368
this.headerResizeObserver.disconnect();
368369
this.headerResizeObserver = undefined;
369370
}
371+
372+
if (this.presented) {
373+
const el = this.el;
374+
setTimeout(() => {
375+
if (!el.isConnected) {
376+
cleanupRootFocusTrapAccessibility();
377+
}
378+
}, 0);
379+
}
370380
}
371381

372382
componentWillLoad() {

core/src/utils/overlays.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,28 @@ export const setRootAriaHidden = (hidden = false) => {
515515
}
516516
};
517517

518+
/**
519+
* Cleans up root `aria-hidden` and `backdrop-no-scroll` when
520+
* an overlay is removed from the DOM without going through
521+
* the `dismiss()` flow (e.g., when a framework unmounts the
522+
* overlay during a route change).
523+
*
524+
* Should be called from an overlay's `disconnectedCallback`
525+
* when the overlay was still presented at the time of removal.
526+
*/
527+
export const cleanupRootFocusTrapAccessibility = () => {
528+
const remainingOverlays = getPresentedOverlays(document);
529+
const hasRemainingLocking = remainingOverlays.some((o) => {
530+
const el = o as OverlayWithFocusTrapProps;
531+
return el.tagName !== 'ION-TOAST' && el.focusTrap !== false && isBackdropAlwaysBlocking(el);
532+
});
533+
534+
if (!hasRemainingLocking) {
535+
setRootAriaHidden(false);
536+
document.body.classList.remove(BACKDROP_NO_SCROLL);
537+
}
538+
};
539+
518540
export const present = async <OverlayPresentOptions>(
519541
overlay: OverlayInterface,
520542
name: keyof IonicConfig,

packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,16 @@ export class ReactRouterViewStack extends ViewStacks {
270270
return true;
271271
}
272272

273+
// Reuse empty-path routes (path="") that are not index routes.
274+
// These function like index routes and should be reused to prevent
275+
// duplicate views when navigating back to the default path.
276+
if (existingPath === '' && routePath === '' && !existingIsIndexRoute && !newIsIndexRoute) {
277+
return true;
278+
}
279+
273280
// Reuse view items with the same path
274281
// Special case: reuse tabs/* and other specific wildcard routes
275-
// Don't reuse index routes (empty path) or generic catch-all wildcards (*)
282+
// Don't reuse generic catch-all wildcards (*)
276283
if (existingPath === routePath && existingPath !== '' && existingPath !== '*') {
277284
// Parameterized routes need pathname matching to ensure /details/1 and /details/2
278285
// get separate view items. For wildcard routes (e.g., user/:userId/*), compare

packages/react-router/test/base/src/App.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ import IndexRouteReuse from './pages/index-route-reuse/IndexRouteReuse';
5555
import TailSliceAmbiguity from './pages/tail-slice-ambiguity/TailSliceAmbiguity';
5656
import WildcardNoHeuristic from './pages/wildcard-no-heuristic/WildcardNoHeuristic';
5757
import RouteContextShape from './pages/route-context-shape/RouteContextShape';
58+
import EmptyPathDeactivation from './pages/empty-path-deactivation/EmptyPathDeactivation';
59+
import ModalAriaHidden from './pages/modal-aria-hidden/ModalAriaHidden';
5860

5961
setupIonicReact();
6062

@@ -99,6 +101,8 @@ const App: React.FC = () => {
99101
<Route path="/tail-slice-ambiguity/*" element={<TailSliceAmbiguity />} />
100102
<Route path="/wildcard-no-heuristic/*" element={<WildcardNoHeuristic />} />
101103
<Route path="/route-context-shape/*" element={<RouteContextShape />} />
104+
<Route path="/empty-path-deactivation/*" element={<EmptyPathDeactivation />} />
105+
<Route path="/modal-aria-hidden/*" element={<ModalAriaHidden />} />
102106
</IonRouterOutlet>
103107
</IonReactRouter>
104108
</IonApp>

packages/react-router/test/base/src/pages/Main.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,12 @@ const Main: React.FC = () => {
110110
<IonItem routerLink="/route-context-shape">
111111
<IonLabel>Route Context Shape</IonLabel>
112112
</IonItem>
113+
<IonItem routerLink="/empty-path-deactivation">
114+
<IonLabel>Empty Path Deactivation</IonLabel>
115+
</IonItem>
116+
<IonItem routerLink="/modal-aria-hidden">
117+
<IonLabel>Modal Aria Hidden</IonLabel>
118+
</IonItem>
113119
</IonList>
114120
</IonContent>
115121
</IonPage>
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import {
2+
IonButton,
3+
IonContent,
4+
IonHeader,
5+
IonLabel,
6+
IonPage,
7+
IonRouterOutlet,
8+
IonTitle,
9+
IonToolbar,
10+
} from '@ionic/react';
11+
import React from 'react';
12+
import { Route, useLocation, useParams } from 'react-router-dom';
13+
14+
/**
15+
* Test page for empty-path route deactivation behavior.
16+
*
17+
* Route structure:
18+
* /empty-path-deactivation/*
19+
* └── <IonRouterOutlet>
20+
* ├── path="" → DefaultPage (matches at /empty-path-deactivation)
21+
* ├── path="details/:id" → DetailsPage
22+
* └── path="*" → CatchAllPage
23+
*
24+
* Bug scenario:
25+
* 1. Navigate to /empty-path-deactivation → DefaultPage shows (path="" matches)
26+
* 2. Navigate to /empty-path-deactivation/details/42 → DetailsPage shows
27+
* 3. Navigate to /empty-path-deactivation/extra/details/99
28+
* 4. derivePathnameToMatch tail-slices ["details","99"] → false match on details/:id
29+
* 5. hasSpecificMatch becomes true → path="" view set to mount=false + ion-page-hidden
30+
* 6. Navigate back to /empty-path-deactivation → DefaultPage may remain hidden
31+
*
32+
* Uses path="" (not index) to test a different code path from the
33+
* tail-slice-ambiguity test which uses <Route index>.
34+
*/
35+
const EmptyPathDeactivation: React.FC = () => (
36+
<IonRouterOutlet id="empty-path-outlet">
37+
<Route path="" element={<DefaultPage />} />
38+
<Route path="details/:id" element={<DetailsPage />} />
39+
<Route path="*" element={<CatchAllPage />} />
40+
</IonRouterOutlet>
41+
);
42+
43+
const DefaultPage: React.FC = () => (
44+
<IonPage data-pageid="empty-path-default">
45+
<IonHeader>
46+
<IonToolbar>
47+
<IonTitle>Default</IonTitle>
48+
</IonToolbar>
49+
</IonHeader>
50+
<IonContent>
51+
<IonLabel data-testid="default-label">Default Page Content</IonLabel>
52+
<IonButton routerLink="/empty-path-deactivation/details/42" id="go-to-details">
53+
Details 42
54+
</IonButton>
55+
<IonButton routerLink="/empty-path-deactivation/extra/details/99" id="go-to-ambiguous">
56+
Ambiguous Path
57+
</IonButton>
58+
</IonContent>
59+
</IonPage>
60+
);
61+
62+
const DetailsPage: React.FC = () => {
63+
const { id } = useParams<{ id: string }>();
64+
return (
65+
<IonPage data-pageid="empty-path-details">
66+
<IonHeader>
67+
<IonToolbar>
68+
<IonTitle>Details</IonTitle>
69+
</IonToolbar>
70+
</IonHeader>
71+
<IonContent>
72+
<IonLabel data-testid="details-id">Details ID: {id}</IonLabel>
73+
<IonButton routerLink="/empty-path-deactivation" id="back-to-default">
74+
Back to Default
75+
</IonButton>
76+
</IonContent>
77+
</IonPage>
78+
);
79+
};
80+
81+
const CatchAllPage: React.FC = () => {
82+
const location = useLocation();
83+
return (
84+
<IonPage data-pageid="empty-path-catchall">
85+
<IonHeader>
86+
<IonToolbar>
87+
<IonTitle>Catch All</IonTitle>
88+
</IonToolbar>
89+
</IonHeader>
90+
<IonContent>
91+
<IonLabel data-testid="catchall-path">Catch-all: {location.pathname}</IonLabel>
92+
<IonButton routerLink="/empty-path-deactivation" id="catchall-back-to-default">
93+
Back to Default
94+
</IonButton>
95+
</IonContent>
96+
</IonPage>
97+
);
98+
};
99+
100+
export default EmptyPathDeactivation;

0 commit comments

Comments
 (0)