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] Forward refs #14737

Merged
merged 3 commits into from
Mar 5, 2019
Merged

[Popover] Forward refs #14737

merged 3 commits into from
Mar 5, 2019

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Mar 4, 2019

Continues #14536

@eps1lon eps1lon added this to the v4 milestone Mar 4, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Mar 4, 2019

Details of bundle changes.

Comparing: a574657...68bf089

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.01% 🔺 +0.02% 🔺 371,763 371,793 91,901 91,917
@material-ui/core/Paper 0.00% +0.03% 🔺 76,649 76,649 19,301 19,306
@material-ui/core/Paper.esm 0.00% +0.04% 🔺 71,599 71,599 18,782 18,790
@material-ui/core/Popper 0.00% 0.00% 30,462 30,462 10,583 10,583
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 17,286 17,286 5,717 5,717
@material-ui/core/useMediaQuery 0.00% 0.00% 2,486 2,486 1,047 1,047
@material-ui/lab 0.00% +0.02% 🔺 184,282 184,282 50,606 50,618
@material-ui/styles 0.00% 0.00% 57,443 57,443 16,176 16,176
@material-ui/system 0.00% 0.00% 17,062 17,062 4,485 4,485
Button 0.00% +0.04% 🔺 99,409 99,409 26,604 26,614
Modal 0.00% +0.03% 🔺 98,711 98,711 26,183 26,192
colorManipulator 0.00% 0.00% 3,232 3,232 1,296 1,296
docs.landing 0.00% 0.00% 51,773 51,773 11,244 11,244
docs.main +0.01% 🔺 +0.01% 🔺 678,764 678,803 206,223 206,234
packages/material-ui/build/umd/material-ui.production.min.js 0.00% -0.01% 322,447 322,460 85,066 85,061

Generated by 🚫 dangerJS against 68bf089

@@ -659,10 +655,6 @@ describe('<Popover />', () => {
positioningStyle = instance.getPositioningStyle(element);
});

after(() => {
instance.getAnchorOffset = stub().returns(anchorOffset);
Copy link
Member Author

Choose a reason for hiding this comment

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

Was c&p from following tests that stubbed instance.getAnchorOffset differently.

@@ -331,7 +333,6 @@ class Popover extends React.Component {
onExit={onExit}
onExited={onExited}
onExiting={onExiting}
role={role}
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why we would want the role on the TransitionComponent. There was no test for it nor internal usage. Change that added this is missing.

Copy link
Member

Choose a reason for hiding this comment

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

This logic hasn't changed since the first commit. I'm happy to remove it 👍

@eps1lon eps1lon marked this pull request as ready for review March 4, 2019 16:34
@eps1lon eps1lon force-pushed the feat/Popover/forwardRef branch from 06d2709 to 68bf089 Compare March 4, 2019 17:57
@eps1lon eps1lon merged commit bcb4bec into mui:next Mar 5, 2019
@eps1lon eps1lon deleted the feat/Popover/forwardRef branch March 5, 2019 07:54
@oliviertassinari oliviertassinari changed the title [Popover] forward refs [Popover] Forward refs Mar 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants