-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[core] Batch small changes #22565
[core] Batch small changes #22565
Conversation
Motivated by mui#22395 (comment).
Any particular reason? They help with readability, and removing them doesn't save any meaningful bytes. |
If I remember the discussion correctly then it was about consistency. I don't have a strong opinion either way. However, if the goal is consistency then adding blank lines between tags could potentially hurt readability as the number of tags grows. Though from experience I'd advise against arguing about formatting that requires human interaction. I don't remember the pre-prettier days very fondly 😉 |
@@ -150,7 +150,7 @@ declare module '@material-ui/core/styles/createBreakpoints' { | |||
|
|||
#### Arguments | |||
|
|||
1. `key` (_String_ | _Number_): A breakpoint key (`xs`, `sm`, etc.) or a screen width number in pixels. | |||
1. `key` (_String_ | _Number_): A breakpoint key (`xs`, `sm`, etc.) or a screen width number in px. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. `key` (_String_ | _Number_): A breakpoint key (`xs`, `sm`, etc.) or a screen width number in px. | |
1. `key` (_String_ | _Number_): A breakpoint key (`xs`, `sm`, etc.) or a screen width number in px. |
Should we make these px
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are already px?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or did you mean px vs px
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, px
rather than px?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fair, why not
Regarding |
@default
. I have done the same change in material-ui-x.