mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2025-08-14 16:52:26 +08:00
fix(react): overlay content is shown with hook (#28109)
Issue number: resolves #28102 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> When one modal is added and another modal is removed, the modal that is removed does not account for the newly added modal when updating the overlay context in React. As a result, the inner contents of the newly added modal is not mounted. We originally tried to fix this in https://github.com/ionic-team/ionic-framework/pull/24553, but the fix was not complete. While storing the latest information in a React ref was correct, the way we updated the ref was done in a way such that data was still stale. In particular, the `overlaysRef` is updated whenever `IonOverlayManager` is re-rendered. State updates are batched, so updating the state twice in quick succession does not necessarily result in 2 separate renders. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - We need to make sure the ref is updated synchronously before any render so that `addOverlay` and `removeOverlay` always have access to the latest data. - Added a test ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Dev build: `7.3.3-dev.11693592339.18e000af` --------- Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com> Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
This commit is contained in:
@ -40,7 +40,6 @@ export const IonOverlayManager: React.FC<IonOverlayManagerProps> = ({ onAddOverl
|
||||
*/
|
||||
const [overlays, setOverlays] = useState<OverlaysList>({});
|
||||
const overlaysRef = useRef<OverlaysList>({});
|
||||
overlaysRef.current = overlays;
|
||||
|
||||
useEffect(() => {
|
||||
/* Setup the callbacks that get called from <IonApp /> */
|
||||
@ -51,12 +50,50 @@ export const IonOverlayManager: React.FC<IonOverlayManagerProps> = ({ onAddOverl
|
||||
const addOverlay = (id: string, component: ReactComponentOrElement, containerElement: HTMLDivElement) => {
|
||||
const newOverlays = { ...overlaysRef.current };
|
||||
newOverlays[id] = { component, containerElement };
|
||||
|
||||
/**
|
||||
* In order for this function to use the latest data
|
||||
* we need to update the ref synchronously.
|
||||
* However, updating a ref does not cause a re-render
|
||||
* which is why we still update the state.
|
||||
*
|
||||
* Note that updating the ref in the body
|
||||
* of IonOverlayManager is not sufficient
|
||||
* because that relies on overlaysRef being
|
||||
* updated as part of React's render loop. State updates
|
||||
* are batched, so updating the state twice in quick succession does
|
||||
* not necessarily result in 2 separate render calls.
|
||||
* This means if two modals are added one after the other,
|
||||
* the first modal will not have been added to
|
||||
* overlaysRef since React has not re-rendered yet.
|
||||
* More info: https://react.dev/reference/react/useState#setstate-caveats
|
||||
*/
|
||||
overlaysRef.current = newOverlays;
|
||||
setOverlays(newOverlays);
|
||||
};
|
||||
|
||||
const removeOverlay = (id: string) => {
|
||||
const newOverlays = { ...overlaysRef.current };
|
||||
delete newOverlays[id];
|
||||
|
||||
/**
|
||||
* In order for this function to use the latest data
|
||||
* we need to update the ref synchronously.
|
||||
* However, updating a ref does not cause a re-render
|
||||
* which is why we still update the state.
|
||||
*
|
||||
* Note that updating the ref in the body
|
||||
* of IonOverlayManager is not sufficient
|
||||
* because that relies on overlaysRef being
|
||||
* updated as part of React's render loop. State updates
|
||||
* are batched, so updating the state twice in quick succession does
|
||||
* not necessarily result in 2 separate render calls.
|
||||
* This means if two modals are added one after the other,
|
||||
* the first modal will not have been added to
|
||||
* overlaysRef since React has not re-rendered yet.
|
||||
* More info: https://react.dev/reference/react/useState#setstate-caveats
|
||||
*/
|
||||
overlaysRef.current = newOverlays;
|
||||
setOverlays(newOverlays);
|
||||
};
|
||||
|
||||
|
@ -30,6 +30,8 @@ const Body: React.FC<{
|
||||
<IonButton expand="block" onClick={() => onDismiss({ test: true }, 'close')}>
|
||||
Close
|
||||
</IonButton>
|
||||
|
||||
<button onClick={onDismiss} id="show-secondary-modal">Show Secondary Modal</button>
|
||||
</IonContent>
|
||||
</IonPage>
|
||||
);
|
||||
@ -83,6 +85,14 @@ const ModalHook: React.FC = () => {
|
||||
|
||||
const [presentModalWithContext] = useIonModal(ModalWithContext);
|
||||
|
||||
const [presentSecondaryModal] = useIonModal(ModalSecondary);
|
||||
const [presentRootModal, dismissRootModal] = useIonModal(Body, {
|
||||
onDismiss: () => {
|
||||
dismissRootModal();
|
||||
presentSecondaryModal();
|
||||
}
|
||||
});
|
||||
|
||||
return (
|
||||
<MyContext.Provider value={{ value: 'overriden value' }}>
|
||||
<IonPage>
|
||||
@ -134,6 +144,16 @@ const ModalHook: React.FC = () => {
|
||||
Show Modal with Context
|
||||
</IonButton>
|
||||
|
||||
<IonButton
|
||||
expand="block"
|
||||
onClick={() => {
|
||||
presentRootModal()
|
||||
}}
|
||||
id="show-root-modal"
|
||||
>
|
||||
Show Root Modal
|
||||
</IonButton>
|
||||
|
||||
<div>Count: {count}</div>
|
||||
<div>Dismissed with role: {dismissedRole}</div>
|
||||
<div>Data: {dismissedData && JSON.stringify(dismissedData)}</div>
|
||||
@ -143,6 +163,15 @@ const ModalHook: React.FC = () => {
|
||||
);
|
||||
};
|
||||
|
||||
const ModalSecondary: React.FC = () => {
|
||||
return (
|
||||
<div className="ion-padding">
|
||||
<h1>Secondary Modal</h1>
|
||||
<p>This text should be visible</p>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
const MyContext = React.createContext({
|
||||
value: 'default value',
|
||||
});
|
||||
|
@ -70,4 +70,16 @@ describe('useIonModal', () => {
|
||||
//verify context value is overriden value
|
||||
cy.get('div').contains('overriden value')
|
||||
});
|
||||
|
||||
it('should render nested modal when modals are added and removed at the same time', () => {
|
||||
cy.get('#show-root-modal').click();
|
||||
|
||||
cy.get('ion-modal').should('have.length', 1);
|
||||
|
||||
cy.get('ion-modal #show-secondary-modal').click();
|
||||
|
||||
cy.get('ion-modal').should('have.length', 1);
|
||||
|
||||
cy.get('ion-modal').contains('This text should be visible');
|
||||
});
|
||||
});
|
||||
|
Reference in New Issue
Block a user