Skip to content

Conversation

@TheUnlocked
Copy link
Contributor

@TheUnlocked TheUnlocked commented Oct 1, 2022

Issues

Changes

Previously popovers could have been rendered before they were positioned, resulting in them briefly appearing in the top-left corner particularly on slower devices and with low transitionDuration. This PR hides the popover paper until positioning is complete.

Drawbacks

On slower devices, the opening animation may be completely skipped since the animation would complete before positioning would finish. This can be demonstrated with the Chrome dev tools' CPU throttling feature. Due to the current way that popover is designed, there is a sort of catch-22 in which the the paper only mounts once the transition begins, but positioning can only occur after the paper mounts. Changing other parts of the code to get around that issue is certainly possible, but I have not attempted to do that in this PR.

Resolves #33308

@TheUnlocked TheUnlocked changed the title Fix select menu position [Popover] Fix paper position flash on open Oct 1, 2022
@mui-bot
Copy link

mui-bot commented Oct 1, 2022

Details of bundle changes

Generated by 🚫 dangerJS against 1338ddd

@michaldudak
Copy link
Member

Thanks for your work. Your fix seems to work. I'm going to test it more thoroughly.
As for the drawback you mentioned, I think we can accept it.

BTW, the affected components, such as Select will be implemented using MUI Base in the future, which doesn't suffer from this issue AFAIK. This way the implementation will be simpler.

@michaldudak michaldudak added type: bug It doesn't behave as expected. component: Popover The React component. package: material labels Oct 11, 2022
Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything seems to be in order! Thanks again for your contribution.

@michaldudak michaldudak merged commit 5cc3566 into mui:master Oct 12, 2022
alexfauquette pushed a commit to alexfauquette/material-ui that referenced this pull request Oct 14, 2022
daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
feliperli pushed a commit to jesrodri/material-ui that referenced this pull request Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: Popover The React component. type: bug It doesn't behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Select] menu position bug

3 participants