-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[Popover] Support Collapse as a TransitionComponent #21283
Conversation
Details of bundle changes.Comparing: 3eb02f5...d843bdf Details of page changes
|
but incorrect when using components like Collapse
746382b
to
d843bdf
Compare
// specZ: The maximum height of a simple menu should be one or more rows less than the view | ||
// height. This ensures a tapable area outside of the simple menu with which to dismiss | ||
// the menu. | ||
maxHeight: 'calc(100% - 96px)', |
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.
How would we solve the menu overflow issue? Here is a reproduction on https://codesandbox.io/s/material-demo-3k97p?file=/package.json with codesandbox-ci. The options at the bottom can't be reached. This impacts a Select with many options. A "light" version of the problem is described in #11349.
Something is off with the box-shadow in the transition: Should we consider using a different Collapse transition component? There is one element I don't understand, what's wrong with the |
I'm closing, the resolution of the problem has been deprioritzed for a long time. Either its opportunity cost was too high, or we had a blocker. We can revisit in the future. |
Closes #11337
Popover
assumed that thePaper
insideTransitionComponent
was the outermost wrapper. This requirement was never documented and breaksCollapse
which renders some wrapper nodes.Preview: https://deploy-preview-21283--material-ui.netlify.app/components/menus/#change-transition
This is probably safe to land in v5 while renaming
classes.paper
toclasses.transition
. At the same time we should warn onPaperProps.className
(since it's ignored) and recommendTransitionProps.className
.