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.

Sign up for GitHub

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

Jump to bottom

[Modal] Forward refs #14738

Merged
merged 3 commits into from
Mar 5, 2019
Merged

[Modal] Forward refs #14738

merged 3 commits into from
Mar 5, 2019

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Mar 4, 2019

Continues #14536

@eps1lon eps1lon added breaking change component: modal This is the name of the generic UI component, not the React module! labels Mar 4, 2019
@eps1lon eps1lon added this to the v4 milestone Mar 4, 2019
eps1lon
eps1lon commented Mar 4, 2019
View reviewed changes
packages/material-ui/src/Modal/Modal.js
@@ -50,8 +52,6 @@ export const styles = theme => ({
* This component shares many concepts with [react-overlays](https://react-bootstrap.github.io/react-overlays/#modals).
*/
class Modal extends React.Component {
mounted = false;
Copy link
Member Author

Choose a reason for hiding this comment

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

This was never read. Only written to.

Copy link
Member

Choose a reason for hiding this comment

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

dead code :)

@mui-pr-bot
Copy link

mui-pr-bot commented Mar 4, 2019 •
edited

Details of bundle changes.

Comparing: bcb4bec...b4387d9

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.01% 🔺 +0.01% 🔺 371,793 371,833 91,917 91,926
@material-ui/core/Paper 0.00% +0.01% 🔺 76,649 76,649 19,306 19,307
@material-ui/core/Paper.esm 0.00% -0.03% 71,599 71,599 18,790 18,784
@material-ui/core/Popper 0.00% +0.02% 🔺 30,462 30,462 10,583 10,585
@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.09% 184,282 184,282 50,618 50,570
@material-ui/styles 0.00% +0.01% 🔺 57,443 57,443 16,176 16,177
@material-ui/system 0.00% -0.04% 17,062 17,062 4,485 4,483
Button 0.00% -0.01% 99,409 99,409 26,614 26,612
Modal +0.28% 🔺 +0.26% 🔺 98,711 98,984 26,192 26,260
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.00% 678,803 678,857 206,234 206,238
packages/material-ui/build/umd/material-ui.production.min.js 0.00% -0.02% 322,460 322,466 85,061 85,044

Generated by 🚫 dangerJS against b4387d9

@eps1lon
[Modal] Improve tests
39d5594 
Tossed some out that had no meaningful assertion in it
or insufficient description of what the test is supposed to
do
@eps1lon
[Modal] Remove unused instance field
45278c8
@eps1lon
[Modal] Forward refs
b4387d9
@eps1lon eps1lon merged commit d4425c6 into mui:next Mar 5, 2019
@eps1lon eps1lon deleted the feat/Modal/forwardRef branch March 5, 2019 08:11
@eps1lon eps1lon mentioned this pull request Mar 6, 2019
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviertassinari oliviertassinari oliviertassinari approved these changes

Assignees
No one assigned
Labels
breaking change component: modal This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
v4
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@eps1lon @mui-pr-bot @oliviertassinari