Skip to content
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

[material-ui][Modal] Bring back transition component in test that fails with React 19 #43312

Open
aarongarciah opened this issue Aug 15, 2024 · 0 comments
Assignees
Labels
component: modal This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material test

Comments

@aarongarciah
Copy link
Member

aarongarciah commented Aug 15, 2024

While upgrading to React 19 (PR), a Modal component test started to fail (failure example). The Modal is not unmounted. The component works as expected when tested manually, but the combination of Modal + Fade (and probably any other transition component) makes the test fail.

// Test case for https://github.com/mui/material-ui/issues/12831
it('should unmount the children ', () => {
const timeout = 50;
function TestCase() {
const [open, setOpen] = React.useState(true);
React.useEffect(() => {
setOpen(false);
}, []);
return (
<Modal open={open}>
<Fade in={open} timeout={timeout}>
<div id="modal-body">hello</div>
</Fade>
</Modal>
);
}
render(<TestCase />);
// exit transition started
clock.tick(timeout);
expect(document.querySelector('#modal-body')).to.equal(null);
});

The issue linked in the comment above the test points to a PR where a fix was implemented for a bug with the backdrop staying open. The fix consisted on:

-const [exited, setExited] = React.useState(!open);
+const [exited, setExited] = React.useState(true);

This fix was lost in translation once Modal was migrated to use useModal from Base UI. Last year, a user reported the bug again.

Re-applying this fix solves the failing test, but breaks one Drawer component (which uses Modal under the hood) use case: when the Drawer is initially open, it won't run the exit animation the first time is closed.

Note: Using true as the initial state is the approach used across the codebase, with Modal being the only component using !open as the initial state.

We decided to remove the Fade component as child in the test to make it pass (see #42824), but we want to investigate further and bring it back.

Search keywords: modal, react 19, transition, test

@aarongarciah aarongarciah added component: modal This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Aug 15, 2024
@aarongarciah aarongarciah self-assigned this Aug 15, 2024
@aarongarciah aarongarciah changed the title [material-ui][Modal] Fix test failing with React 19 [material-ui][Modal] Bring back transition component in test that fails with React 19 Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: modal This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material test
Projects
None yet
Development

No branches or pull requests

1 participant