-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[docs] Document how to unmount transition child #30382
[docs] Document how to unmount transition child #30382
Conversation
a828f79
to
cef4047
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
@@ -308,7 +308,7 @@ export default function AppLayoutDocsFooter() { | |||
</PaginationDiv> | |||
</React.Fragment> | |||
)} | |||
<Collapse in={commentOpen} onEntered={handleEntered}> | |||
<Collapse in={commentOpen} unmountOnExit onEntered={handleEntered}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would extract this to a separate PR, but probably not worth the effort :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean. What's the main value proposition of this PR? To be fair, I think that the WAVE error is wrong and should be ignored, the input is not interactive. I think that the real value of this prop is about reducing the upfront work to render the page, so aligned with the docs, it illustrates the use case with a real life win.
This win:
is a good side effect. But if you go to https://mui.com/components/textarea-autosize/, you will still see the error.
I have noticed this having a quick look at #30378. When you open WAVE, on any page, say https://mui.com/components/alert/, it will let you know that
the textbox in the rating popup is missing a text label. But why is this rendered in the DOM in the first place?
So, I'm removing it from the DOM with the prop. It should help with performance, only rendering what's needed.
Second, since this is kind of an obscure trick and I saw people asking about it 1-2 years ago on the issue, and that it's still not in the docs, I added it. https://deploy-preview-30382--material-ui.netlify.app/components/transitions/#performance-amp-seo