Skip to content

[Popper] Add popperRef prop#16069

Merged
oliviertassinari merged 8 commits into
mui:masterfrom
oliviertassinari:popper-popperRef
Jun 8, 2019
Merged

[Popper] Add popperRef prop#16069
oliviertassinari merged 8 commits into
mui:masterfrom
oliviertassinari:popper-popperRef

Conversation

@oliviertassinari

@oliviertassinari oliviertassinari commented Jun 5, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

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 type: new feature Expand the scope of the product to solve a new problem. 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
oliviertassinari
oliviertassinari commented Jun 5, 2019
View reviewed changes
Comment thread packages/material-ui/src/Popper/Popper.d.ts Outdated
@mui-pr-bot

mui-pr-bot commented Jun 5, 2019 •
edited
Loading

Copy link
Copy Markdown
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
View reviewed changes
Comment thread packages/material-ui/src/Popper/Popper.d.ts Outdated
Comment thread packages/material-ui/src/Popper/Popper.js Outdated
@oliviertassinari oliviertassinari mentioned this pull request Jun 5, 2019
Merged
@oliviertassinari
oliviertassinari requested a review from eps1lon June 6, 2019 20:58
eps1lon
eps1lon previously requested changes Jun 6, 2019
View reviewed changes
Comment thread packages/material-ui/src/Popper/Popper.js Outdated
Comment thread packages/material-ui/src/Popper/Popper.d.ts Outdated
@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
eps1lon
eps1lon reviewed Jun 7, 2019
View reviewed changes

@eps1lon eps1lon left a comment

Copy link
Copy Markdown
Member

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.

Comment thread packages/material-ui/src/Popper/Popper.js Outdated
Comment thread packages/material-ui/src/Popper/Popper.js
@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

Reviewers

@eps1lon eps1lon eps1lon left review comments
+1 more reviewer
@merceyz merceyz merceyz left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component: Popper The React component. See <Popup> for the latest version. type: new feature Expand the scope of the product to solve a new problem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Popper can't update position when content height changed

4 participants

@oliviertassinari @mui-pr-bot @merceyz @eps1lon