-
-
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
[docs] Increase the contrast of the demos #18396
[docs] Increase the contrast of the demos #18396
Conversation
Details of bundle changes.Comparing: 58f0d04...345dfc1
|
6a6ef50
to
931add6
Compare
931add6
to
12342e4
Compare
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.
Sorry to say, but I'm not convinced in its current form:
- The demos no longer "pop" from the page.
- They are inconsistent. Some are grey, some are outlined. The docs should follow a consistent set of styles to make them easy to follow.
- The fact that there are so many exceptions is a red flag.
<form className={classes.root} noValidate autoComplete="off"> | ||
<TextField id="standard-basic" label="Standard" /> | ||
<TextField id="filled-basic" label="Filled" variant="filled" /> | ||
<TextField id="outlined-basic" label="Outlined" variant="outlined" /> | ||
</form> |
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.
🎉
@mbrookes Thanks for the review. What do you think if I keep iterating with goal to have all the demos with a white background?
I believe that in the current form, we always end up with a white background. But I can explore how to make the border more consistent.
I can explore that too. |
How do you plan to differentiate the demo from the page? The border doesn't seem sufficient to me. Could we perhaps keep the border, but make the background a shade of grey that is lighter than it is currently? |
34c89fd
to
459f8f5
Compare
updated, I have worked on the "consistency" and "pop" aspects
459f8f5
to
965cb78
Compare
Except that you're comparing it to the MCW implementation, which, like several other components, doesn't follow the spec. (See also: #12889 (comment) 😄) The filled text field matches: (And most other examples) But not: (Which unhelpfully doesn't provide RGBA values to use.) Perhaps the spec has changed slightly since the last revision of the component? There's a record of contrast ratio in the discussion linked above, so it would be easy to tell. |
@mbrookes Every time I ask Una about a specification question, she sends me toward material-components-web and not the material.io website. I'm confused. (I start more and more to believe that our users are looking for a UI that looks like the Google products that are used to. I would be cautious not to upgrade to match a new spec until at least a few Google products do) |
965cb78
to
0fb3381
Compare
@mbrookes I think that it's really important that we display the elements on a white background as often as possible. While this change might introduce consistency. I still think that it puts us in a better place, overall. We likely want to use
|
c5ebc10
to
b42db25
Compare
b42db25
to
345dfc1
Compare
Use a white background for the demos as much as possible:
Before
https://material-ui.com/components/text-fields/#textfield
After
https://deploy-preview-18396--material-ui.netlify.com/components/text-fields/#textfield
The border vs box-shadow diff is related to #15728