Skip to content

Commit fa4593d

Browse files
authored
fix(react): bind events properly for overlays rendered within a nav (#31159)
Issue number: resolves #27843 --------- ## What is the current behavior? `createInlineOverlayComponent` renders inline overlays (modal, popover, etc.) inside a `<template>` at their declared JSX position. When the overlay presents, `CoreDelegate` teleports the DOM node into `ion-app`, but React's synthetic event delegation root stays at the original JSX parent. Once the overlay lives outside that subtree, React no longer dispatches events to children inside it, so `onClick`, `onChange`, and other handlers inside an `IonModal` rendered within an `IonNav` silently stop firing ## What is the new behavior? Top-level inline overlays now render through `createPortal` into the same `ion-app` container that `CoreDelegate` teleports into, so React's event root follows the DOM ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information This issue also reports the same problem in Vue, but that was fixed in #30227 Preview: [navigation-modal](https://ionic-framework-git-fw-6314-ionic1.vercel.app/react/navigation-modal) Dev build: ``` 8.8.8-dev.11779302602.17decfbf ```
1 parent 0182bba commit fa4593d

4 files changed

Lines changed: 202 additions & 16 deletions

File tree

packages/react/src/components/createInlineOverlayComponent.tsx

Lines changed: 104 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import type { HTMLIonOverlayElement, OverlayEventDetail } from '@ionic/core/components';
2+
import { componentOnReady } from '@ionic/core/components';
23
import React, { createElement } from 'react';
4+
import { createPortal } from 'react-dom';
35

46
import {
57
attachProps,
@@ -17,6 +19,15 @@ type InlineOverlayState = {
1719
isOpen: boolean;
1820
};
1921

22+
/**
23+
* Set to `true` when rendering inside another inline overlay. Nested
24+
* overlays render at their JSX position (no portal) so that core's
25+
* `el.closest('ion-popover')`-style nesting detection keeps working,
26+
* and the outer overlay's portal already gives the subtree the correct
27+
* React event-delegation root.
28+
*/
29+
const NestedOverlayContext = React.createContext(false);
30+
2031
interface IonicReactInternalProps<ElementType> extends React.HTMLAttributes<ElementType> {
2132
forwardedRef?: React.ForwardedRef<ElementType>;
2233
ref?: React.Ref<any>;
@@ -36,12 +47,18 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
3647
defineCustomElement();
3748
}
3849
const displayName = dashToPascalCase(tagName);
39-
const ReactComponent = class extends React.Component<IonicReactInternalProps<PropType>, InlineOverlayState> {
50+
51+
type InternalProps = IonicReactInternalProps<PropType> & { isNested?: boolean };
52+
53+
const ReactComponent = class extends React.Component<InternalProps, InlineOverlayState> {
4054
ref: React.RefObject<HTMLIonOverlayElement>;
4155
wrapperRef: React.RefObject<HTMLElement>;
56+
markerRef: React.RefObject<HTMLTemplateElement>;
4257
stableMergedRefs: React.RefCallback<HTMLElement>;
58+
portalTarget: HTMLElement | null;
59+
isUnmounted = false;
4360

44-
constructor(props: IonicReactInternalProps<PropType>) {
61+
constructor(props: InternalProps) {
4562
super(props);
4663
// Create a local ref to to attach props to the wrapped element.
4764
this.ref = React.createRef();
@@ -51,29 +68,64 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
5168
this.state = { isOpen: false };
5269
// Create a local ref to the inner child element.
5370
this.wrapperRef = React.createRef();
71+
// Marker stays at the JSX location so we can recover the immediate
72+
// JSX parent after the overlay has been portaled to ion-app.
73+
this.markerRef = React.createRef();
74+
/**
75+
* Resolve the portal target to the same container CoreDelegate
76+
* teleports overlays into. Portaling here keeps the overlay inside
77+
* React's tree so React's synthetic events still dispatch to its
78+
* children, even after CoreDelegate moves the DOM node out of the
79+
* declared JSX parent.
80+
*/
81+
this.portalTarget = typeof document !== 'undefined' ? document.querySelector('ion-app') || document.body : null;
5482
}
5583

5684
componentDidMount() {
85+
// Reset for React 18 StrictMode: the dev-mode unmount/remount cycle
86+
// re-uses this instance and leaves the flag set from the prior
87+
// componentWillUnmount.
88+
this.isUnmounted = false;
89+
5790
this.componentDidUpdate(this.props);
5891

5992
this.ref.current?.addEventListener('ionMount', this.handleIonMount);
6093
this.ref.current?.addEventListener('willPresent', this.handleWillPresent);
6194
this.ref.current?.addEventListener('didDismiss', this.handleDidDismiss);
95+
96+
/**
97+
* The overlay is portaled to `portalTarget`, so Stencil caches that
98+
* container as `cachedOriginalParent`. Modal features (sheet
99+
* child-route passthrough, parent-removal auto-dismiss) walk up
100+
* from `cachedOriginalParent` to find the enclosing `.ion-page`,
101+
* so we redirect it at the marker's JSX parent.
102+
*/
103+
const overlay = this.ref.current;
104+
if (overlay) {
105+
componentOnReady(overlay as HTMLElement, () => {
106+
if (this.isUnmounted) return;
107+
const markerParent = this.markerRef.current?.parentElement ?? null;
108+
if (markerParent && markerParent !== this.portalTarget) {
109+
(overlay as any).cachedOriginalParent = markerParent;
110+
}
111+
});
112+
}
62113
}
63114

64-
componentDidUpdate(prevProps: IonicReactInternalProps<PropType>) {
115+
componentDidUpdate(prevProps: InternalProps) {
65116
const node = this.ref.current! as HTMLElement;
66117
/**
67118
* onDidDismiss and onWillPresent have manual implementations that
68119
* will invoke the original handler. We need to filter those out
69120
* so they don't get attached twice and called twice.
70121
*/
71122
// eslint-disable-next-line @typescript-eslint/no-unused-vars
72-
const { onDidDismiss, onWillPresent, ...cProps } = this.props;
123+
const { onDidDismiss, onWillPresent, isNested, ...cProps } = this.props;
73124
attachProps(node, cProps, prevProps);
74125
}
75126

76127
componentWillUnmount() {
128+
this.isUnmounted = true;
77129
const node = this.ref.current;
78130
/**
79131
* If the overlay is being unmounted, but is still
@@ -97,14 +149,28 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
97149
* avoid memory leaks.
98150
*/
99151
node.removeEventListener('didDismiss', this.handleDidDismiss);
100-
node.remove();
152+
if (this.props.isNested) {
153+
/**
154+
* Nested overlays render inline (no portal). CoreDelegate may
155+
* have moved the node out of its React parent, so React's
156+
* unmount won't reach it. Remove it directly.
157+
*/
158+
node.remove();
159+
} else if (node.isConnected && this.portalTarget && node.parentNode !== this.portalTarget) {
160+
/**
161+
* Portaled path: move the overlay back into `portalTarget` so
162+
* React's portal removeChild can find it. CoreDelegate (or user
163+
* code in onWillPresent) may have moved it elsewhere while open.
164+
*/
165+
this.portalTarget.appendChild(node);
166+
}
101167
detachProps(node, this.props);
102168
}
103169
}
104170

105171
render() {
106172
// eslint-disable-next-line @typescript-eslint/no-unused-vars
107-
const { children, forwardedRef, style, className, ref, ...cProps } = this.props;
173+
const { children, forwardedRef, style, className, ref, isNested, ...cProps } = this.props;
108174

109175
const propsToPass = Object.keys(cProps).reduce((acc, name) => {
110176
if (name.indexOf('on') === 0 && name[2] === name[2].toUpperCase()) {
@@ -136,17 +202,16 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
136202
return DELEGATE_HOST;
137203
};
138204

139-
return createElement(
140-
'template',
141-
{},
205+
const overlayElement = createElement(
206+
tagName,
207+
newProps,
208+
// Children, not the overlay host, observe `isNested = true`.
142209
createElement(
143-
tagName,
144-
newProps,
210+
NestedOverlayContext.Provider,
211+
{ value: true },
145212
/**
146-
* We only want the inner component
147-
* to be mounted if the overlay is open,
148-
* so conditionally render the component
149-
* based on the isOpen state.
213+
* We only want the inner component to be mounted if the overlay
214+
* is open, so conditionally render based on `isOpen` state.
150215
*/
151216
this.state.isOpen || this.props.keepContentsMounted
152217
? createElement(
@@ -160,6 +225,21 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
160225
: null
161226
)
162227
);
228+
229+
// Top-level overlays portal into `portalTarget` with a marker
230+
// `<template>` at the JSX location to recover the immediate JSX
231+
// parent after CoreDelegate teleports. Nested overlays and SSR
232+
// fall back to a `<template>` wrapper.
233+
if (!isNested && this.portalTarget) {
234+
return createElement(
235+
React.Fragment,
236+
null,
237+
createElement('template', { ref: this.markerRef }),
238+
createPortal(overlayElement, this.portalTarget)
239+
);
240+
}
241+
242+
return createElement('template', {}, overlayElement);
163243
}
164244

165245
static get displayName() {
@@ -206,7 +286,15 @@ export const createInlineOverlayComponent = <PropType, ElementType>(
206286
this.props.onDidDismiss && this.props.onDidDismiss(evt);
207287
};
208288
};
209-
return createForwardRef<PropType, ElementType>(ReactComponent, displayName);
289+
290+
// Forward the nesting context as a prop to avoid contextType on the class.
291+
const ReactComponentWithNesting: React.FC<IonicReactInternalProps<PropType>> = (props) =>
292+
createElement(NestedOverlayContext.Consumer, null, (isNested: boolean) =>
293+
createElement(ReactComponent, { ...(props as InternalProps), isNested })
294+
);
295+
ReactComponentWithNesting.displayName = displayName;
296+
297+
return createForwardRef<PropType, ElementType>(ReactComponentWithNesting, displayName);
210298
};
211299

212300
const DELEGATE_HOST = 'ion-delegate-host';

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import Main from './pages/Main';
2727
import Tabs from './pages/Tabs';
2828
import TabsBasic from './pages/TabsBasic';
2929
import NavComponent from './pages/navigation/NavComponent';
30+
import NavModalComponent from './pages/navigation/NavModalComponent';
3031
import TabsDirectNavigation from './pages/TabsDirectNavigation';
3132
import TabsSimilarPrefixes from './pages/TabsSimilarPrefixes';
3233
import IonModalConditional from './pages/overlay-components/IonModalConditional';
@@ -66,6 +67,7 @@ const App: React.FC = () => (
6667
/>
6768
<Route path="/keep-contents-mounted" component={KeepContentsMounted} />
6869
<Route path="/navigation" component={NavComponent} />
70+
<Route path="/navigation-modal" component={NavModalComponent} />
6971
<Route path="/tabs" component={Tabs} />
7072
<Route path="/tabs-basic" component={TabsBasic} />
7173
<Route path="/tabs-direct-navigation" component={TabsDirectNavigation} />
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import {
2+
IonButton,
3+
IonContent,
4+
IonHeader,
5+
IonModal,
6+
IonNav,
7+
IonPage,
8+
IonTitle,
9+
IonToolbar,
10+
} from '@ionic/react';
11+
import React, { useState } from 'react';
12+
13+
const ModalPage: React.FC = () => {
14+
const [isOpen, setIsOpen] = useState(false);
15+
const [clickCount, setClickCount] = useState(0);
16+
const [inputValue, setInputValue] = useState('');
17+
18+
return (
19+
<>
20+
<IonHeader>
21+
<IonToolbar>
22+
<IonTitle>Nav Modal Page</IonTitle>
23+
</IonToolbar>
24+
</IonHeader>
25+
<IonContent>
26+
<IonButton id="open-modal" onClick={() => setIsOpen(true)}>
27+
Open Modal
28+
</IonButton>
29+
<div id="page-click-count">{clickCount}</div>
30+
<div id="page-input-value">{inputValue}</div>
31+
32+
<IonModal isOpen={isOpen} onDidDismiss={() => setIsOpen(false)}>
33+
<IonContent>
34+
<IonButton id="increment-button" onClick={() => setClickCount((c) => c + 1)}>
35+
Increment
36+
</IonButton>
37+
<input
38+
id="text-input"
39+
type="text"
40+
value={inputValue}
41+
onChange={(e) => setInputValue(e.target.value)}
42+
/>
43+
<IonButton id="close-modal" onClick={() => setIsOpen(false)}>
44+
Close
45+
</IonButton>
46+
</IonContent>
47+
</IonModal>
48+
</IonContent>
49+
</>
50+
);
51+
};
52+
53+
const NavModalComponent: React.FC = () => {
54+
return (
55+
<IonPage>
56+
<IonNav root={() => <ModalPage />} />
57+
</IonPage>
58+
);
59+
};
60+
61+
export default NavModalComponent;
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
describe('IonNav with IonModal', () => {
2+
beforeEach(() => {
3+
cy.visit('/navigation-modal');
4+
});
5+
6+
// @see https://github.com/ionic-team/ionic-framework/issues/27843
7+
it('should fire click handlers inside a modal that lives in IonNav', () => {
8+
cy.get('#open-modal').click();
9+
cy.get('ion-modal').should('be.visible');
10+
11+
cy.get('#increment-button').click();
12+
cy.get('#increment-button').click();
13+
14+
cy.get('#page-click-count').should('have.text', '2');
15+
});
16+
17+
// @see https://github.com/ionic-team/ionic-framework/issues/27843
18+
it('should fire input change handlers inside a modal that lives in IonNav', () => {
19+
cy.get('#open-modal').click();
20+
cy.get('ion-modal').should('be.visible');
21+
22+
cy.get('#text-input').type('hello');
23+
24+
cy.get('#page-input-value').should('have.text', 'hello');
25+
});
26+
27+
// @see https://github.com/ionic-team/ionic-framework/issues/27843
28+
it('should dismiss the modal via a click handler inside it', () => {
29+
cy.get('#open-modal').click();
30+
cy.get('ion-modal').should('be.visible');
31+
32+
cy.get('#close-modal').click();
33+
cy.get('ion-modal').should('not.be.visible');
34+
});
35+
});

0 commit comments

Comments
 (0)