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

[misc] Batch small changes #19012

Merged
merged 6 commits into from
Dec 30, 2019
Merged

[misc] Batch small changes #19012

merged 6 commits into from
Dec 30, 2019

Conversation

mbrookes
Copy link
Member

@mbrookes mbrookes commented Dec 28, 2019

  • I have followed (at least) the PR section of the contributing guide.

  • [docs] Fix mini draw width: a9fcb29

  • [Typography] Improve component prop description: ba936de

  • [docs] Fix theme explorer query-string typo: 5cab340

  • [docs] arial-label -> aria-label: f14781e

  • [docs] Autocomplete typo & grammar: ae87f6a

Regarding a9fcb29, the list item icons used to have 24px padding at desktop widths, and 16px on small screens, so lined up nicely under the nave draw hamburger icon:

https://v1.material-ui.com/demos/drawers/#mini-variant-drawer

image

At some point the padding was changed to 16px to match the revised spec, but the mini variant drawer width was unchanged, so the icons were no longer centered:

image

This commit changes the width of the mini-drawer:

image

But I think a case could be made for reverting the padding change and using 24px on larger screens.

@mbrookes mbrookes added the umbrella For grouping multiple issues to provide a holistic view label Dec 28, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Dec 28, 2019

No bundle size changes comparing a5a7bdb...8eef1be

Generated by 🚫 dangerJS against 8eef1be

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 28, 2019

The change related to #14021 seems ambitious. The width looks off if you have a scrollbar. I might be better as a standalone pull request, I'm not sure there is a quick win potential.

Capture d’écran 2019-12-28 à 23 29 14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
umbrella For grouping multiple issues to provide a holistic view
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants