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

[theme] Allow an arbitrary number of elevations #17659

Merged
merged 2 commits into from
Oct 2, 2019
Merged

[theme] Allow an arbitrary number of elevations #17659

merged 2 commits into from
Oct 2, 2019

Conversation

millnitzluan
Copy link
Contributor

@millnitzluan millnitzluan commented Oct 1, 2019

Fixes #17398

  • Elevation is looking for theme.shadows and logging a error message.

Doubts:

  • At line 36 it's not good to start at index 0 instead 1?

@millnitzluan
Copy link
Contributor Author

@oliviertassinari can you have a look at this?

@mui-pr-bot
Copy link

mui-pr-bot commented Oct 1, 2019

Details of bundle changes.

Comparing: e89abcd...bda3e92

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.01% 🔺 +0.02% 🔺 334,418 334,441 91,191 91,205
@material-ui/core/Paper +0.29% 🔺 +0.16% 🔺 69,049 69,248 20,524 20,556
@material-ui/core/Paper.esm +0.23% 🔺 +0.14% 🔺 62,377 62,520 19,276 19,303
@material-ui/core/Popper 0.00% 0.00% 28,405 28,405 10,179 10,179
@material-ui/core/Textarea 0.00% 0.00% 5,082 5,082 2,134 2,134
@material-ui/core/TrapFocus 0.00% 0.00% 3,766 3,766 1,597 1,597
@material-ui/core/styles/createMuiTheme 0.00% -0.03% 16,351 16,351 5,821 5,819
@material-ui/core/useMediaQuery 0.00% -0.19% 2,488 2,488 1,052 1,050
@material-ui/lab 0.00% +0.01% 🔺 143,406 143,407 43,818 43,822
@material-ui/styles 0.00% +0.03% 🔺 51,780 51,780 15,355 15,359
@material-ui/system 0.00% +0.07% 🔺 15,583 15,583 4,339 4,342
Button 0.00% -0.03% 79,148 79,149 24,122 24,115
Modal 0.00% +0.02% 🔺 14,542 14,542 5,113 5,114
Portal 0.00% 0.00% 2,907 2,907 1,316 1,316
Rating 0.00% +0.01% 🔺 70,239 70,240 21,941 21,944
Slider 0.00% -0.01% 75,380 75,381 23,272 23,270
colorManipulator 0.00% 0.00% 3,835 3,835 1,519 1,519
docs.landing 0.00% +0.01% 🔺 54,267 54,267 14,344 14,345
docs.main +0.01% 🔺 0.00% 588,954 588,984 188,297 188,304
packages/material-ui/build/umd/material-ui.production.min.js 0.00% +0.01% 🔺 305,373 305,380 87,836 87,841

Generated by 🚫 dangerJS against bda3e92

@oliviertassinari oliviertassinari changed the title [Paper][Theme] Elevation look at the existence of the index in theme.shadows [theme] Allow an arbitrary number of elevations Oct 1, 2019
@oliviertassinari oliviertassinari added the new feature New feature or request label Oct 1, 2019
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

It looks great! I have added another commit to remove the check from createMuiTheme.

@oliviertassinari
Copy link
Member

At line 36 it's not good to start at index 0 instead 1?

What do you mean?

@millnitzluan millnitzluan marked this pull request as ready for review October 1, 2019 21:43
@millnitzluan
Copy link
Contributor Author

At line 36 it's not good to start at index 0 instead 1?

What do you mean?

Forget it :p

@oliviertassinari oliviertassinari merged commit 2ec5f8f into mui:master Oct 2, 2019
@oliviertassinari
Copy link
Member

@millnitzluan Well done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Theme] Override number of + values of shadow array in theme override using createMuiTheme
4 participants