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

[Popper] Add popperRef prop #16069

Merged
merged 8 commits into from
Jun 8, 2019

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jun 5, 2019

Closes #15802.

These changes are extracted from #15703:

  • First, the Popper.js instance is now persisted between two renders (perf fix). You gonna like this one @eps1lon. We were using {} as default prop value. This is a new reference at each render. Most of the time, it's fine. The problem, in this case, is that we are using the value as an effect dependency. It reruns systematically. I have noticed it with a poor slider update performance.
  • It exposes a popperRef prop that behaves like a reference. It can be used to programmatically update the position of the popper, e.g. when the slider's thumb value changes.

@oliviertassinari oliviertassinari added new feature New feature or request component: Popper The React component. See <Popup> for the latest version. labels Jun 5, 2019
@oliviertassinari oliviertassinari force-pushed the popper-popperRef branch 3 times, most recently from 1aa1961 to ae902d1 Compare June 5, 2019 13:13
@mui-pr-bot
Copy link

mui-pr-bot commented Jun 5, 2019

Details of bundle changes.

Comparing: 41f1722...d62b730

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.07% 🔺 +0.09% 🔺 319,427 319,665 87,505 87,586
@material-ui/core/Paper 0.00% 0.00% 67,964 67,964 20,210 20,210
@material-ui/core/Paper.esm 0.00% 0.00% 61,258 61,258 19,019 19,019
@material-ui/core/Popper +0.83% 🔺 +0.65% 🔺 28,728 28,966 10,349 10,416
@material-ui/core/Textarea 0.00% 0.00% 5,513 5,513 2,373 2,373
@material-ui/core/TrapFocus 0.00% 0.00% 3,755 3,755 1,577 1,577
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,016 16,016 5,820 5,820
@material-ui/core/useMediaQuery 0.00% 0.00% 2,529 2,529 1,087 1,087
@material-ui/lab +0.17% 🔺 +0.18% 🔺 138,362 138,600 42,526 42,601
@material-ui/styles 0.00% 0.00% 51,384 51,384 15,186 15,186
@material-ui/system 0.00% 0.00% 14,825 14,825 4,240 4,240
Button 0.00% 0.00% 83,893 83,893 25,495 25,495
Modal 0.00% 0.00% 20,343 20,343 6,690 6,690
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 55,089 55,089 13,848 13,848
docs.main +0.04% 🔺 +0.05% 🔺 652,739 653,006 206,467 206,576
packages/material-ui/build/umd/material-ui.production.min.js +0.08% 🔺 +0.08% 🔺 292,892 293,117 83,438 83,504

Generated by 🚫 dangerJS against d62b730

eps1lon
eps1lon previously requested changes Jun 5, 2019
eps1lon
eps1lon previously requested changes Jun 6, 2019
@oliviertassinari oliviertassinari force-pushed the popper-popperRef branch 6 times, most recently from 36f40fc to 59c81f8 Compare June 6, 2019 23:00
@oliviertassinari oliviertassinari dismissed eps1lon’s stale review June 6, 2019 23:05

This one was a hard one for me!

@oliviertassinari oliviertassinari requested a review from eps1lon June 7, 2019 08:32
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Keep forgetting I have pending reviews.

@oliviertassinari oliviertassinari merged commit 9e96a44 into mui:master Jun 8, 2019
@oliviertassinari oliviertassinari deleted the popper-popperRef branch June 8, 2019 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Popper The React component. See <Popup> for the latest version. new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Popper can't update position when content height changed
4 participants