Skip to content

[Modal] Migrate to hooks#16254

Merged
oliviertassinari merged 1 commit into
mui:masterfrom
oliviertassinari:modal-hooks
Jun 19, 2019
Merged

[Modal] Migrate to hooks#16254
oliviertassinari merged 1 commit into
mui:masterfrom
oliviertassinari:modal-hooks

Conversation

@oliviertassinari

@oliviertassinari oliviertassinari commented Jun 16, 2019

Copy link
Copy Markdown
Member

The bundle size of the Modal is going from 6,689 B to 5,100 B 😍🔥 (-23.47%)

Demo: https://deploy-preview-16254--material-ui.netlify.com/components/modal/.

Related to #15231

@mui-pr-bot

mui-pr-bot commented Jun 16, 2019

Copy link
Copy Markdown

@material-ui/core: parsed: -0.33% 😍, gzip: -0.10% 😍

Details of bundle changes.

Comparing: 75d9664...7941a32

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core -0.33% -0.10% 318,737 317,690 87,520 87,430
@material-ui/core/Paper -0.00% +0.06% 🔺 68,280 68,279 20,356 20,368
@material-ui/core/Paper.esm -0.00% +0.08% 🔺 61,577 61,574 19,136 19,151
@material-ui/core/Popper 0.00% 0.00% 28,945 28,945 10,405 10,405
@material-ui/core/Textarea 0.00% +0.08% 🔺 5,513 5,513 2,373 2,375
@material-ui/core/TrapFocus -0.05% -0.13% 3,755 3,753 1,580 1,578
@material-ui/core/styles/createMuiTheme -0.02% -0.07% 16,012 16,008 5,794 5,790
@material-ui/core/useMediaQuery 0.00% -0.18% 2,597 2,597 1,103 1,101
@material-ui/lab 0.00% +0.16% 🔺 140,015 140,018 43,325 43,396
@material-ui/styles -0.01% +0.03% 🔺 51,703 51,699 15,342 15,346
@material-ui/system 0.00% +0.05% 🔺 15,303 15,303 4,339 4,341
Button 0.00% +0.27% 🔺 84,278 84,279 25,631 25,699
Modal -30.61% -23.47% 20,330 14,107 6,664 5,100
Slider -0.00% +0.09% 🔺 74,703 74,702 23,219 23,240
colorManipulator 0.00% 0.00% 3,904 3,904 1,544 1,544
docs.landing 0.00% 0.00% 55,232 55,232 13,946 13,946
docs.main -0.16% -0.17% 650,902 649,835 205,053 204,712
packages/material-ui/build/umd/material-ui.production.min.js -0.35% -0.27% 292,060 291,025 83,522 83,297

Generated by 🚫 dangerJS against 7941a32

@joshwooding joshwooding mentioned this pull request Jun 16, 2019
29 tasks
and keeping it there until the modal is closed.
- ♿️ Adds the appropriate ARIA roles automatically.
- 📦 [6.5 kB gzipped](/size-snapshot).
- 📦 [5 kB gzipped](/size-snapshot).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🚀

@joshwooding joshwooding left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Modal changes look good, let's move the useTheme() changes out of this PR though :)

@oliviertassinari

oliviertassinari commented Jun 16, 2019

Copy link
Copy Markdown
Member Author

I propose the following split:

  1. One pull request with everything excepted the Modal.js changes
  2. A pull request with the Modal.js changes

Would it work? This way we can differentiate the test changes from the refactorization?

@joshwooding

Copy link
Copy Markdown
Collaborator

@oliviertassinari Perfect :)

@oliviertassinari

Copy link
Copy Markdown
Member Author

Done in #16262.

@oliviertassinari

Copy link
Copy Markdown
Member Author

I have rebased on master.

@oliviertassinari

Copy link
Copy Markdown
Member Author

Next step will be #15839.

@joshwooding joshwooding left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@oliviertassinari
oliviertassinari merged commit 4103d02 into mui:master Jun 19, 2019
@oliviertassinari
oliviertassinari deleted the modal-hooks branch June 19, 2019 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance scope: modal Changes related to the modal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants