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

[Popover] Fix update position action #17097

Merged
merged 6 commits into from
Aug 23, 2019

Conversation

netochaves
Copy link
Contributor

Fix #16901

@mui-pr-bot
Copy link

mui-pr-bot commented Aug 21, 2019

Details of bundle changes.

Comparing: 343ae8e...365300c

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core -0.01% +0.01% 🔺 329,350 329,333 89,946 89,953
@material-ui/core/Paper 0.00% 0.00% 68,774 68,774 20,490 20,490
@material-ui/core/Paper.esm 0.00% 0.00% 62,148 62,148 19,215 19,215
@material-ui/core/Popper 0.00% 0.00% 28,468 28,468 10,177 10,177
@material-ui/core/Textarea 0.00% 0.00% 5,094 5,094 2,137 2,137
@material-ui/core/TrapFocus 0.00% 0.00% 3,834 3,834 1,614 1,614
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,386 16,386 5,829 5,829
@material-ui/core/useMediaQuery 0.00% 0.00% 2,541 2,541 1,058 1,058
@material-ui/lab 0.00% 0.00% 153,314 153,314 46,716 46,716
@material-ui/styles 0.00% 0.00% 51,492 51,492 15,307 15,307
@material-ui/system 0.00% 0.00% 15,658 15,658 4,370 4,370
Button 0.00% 0.00% 78,778 78,778 24,072 24,072
Modal 0.00% 0.00% 14,346 14,346 5,010 5,010
Portal 0.00% 0.00% 2,907 2,907 1,319 1,319
Rating 0.00% 0.00% 70,245 70,245 21,940 21,940
Slider 0.00% 0.00% 74,483 74,483 23,065 23,065
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 53,510 53,510 13,854 13,854
docs.main -0.00% +0.01% 🔺 597,188 597,176 190,715 190,731
packages/material-ui/build/umd/material-ui.production.min.js -0.01% +0.01% 🔺 300,265 300,248 86,390 86,396

Generated by 🚫 dangerJS against 365300c

@oliviertassinari
Copy link
Member

@netochaves Could you add a failing test case? This should help to prevent similar regressions in the future.

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: Popover The React component. labels Aug 21, 2019
@netochaves
Copy link
Contributor Author

@oliviertassinari Sure! It will fail if the popover prop open is false, do you know another case?

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

Great! I have updated the test case to be failing without the fix. I think that we are good to go 🚢.

@oliviertassinari
Copy link
Member

@netochaves Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: Popover The React component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Popover] updatePosition action doesn't work
3 participants