-
-
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
Grouped autocomplete color contrast #19574
Comments
What should we do? Aren't there options for that? |
Possible solutions:
Even better: a divider with a (subheader) in the theme's primary color. See this screenshot from the docs. Here the category is to the left of the list, but could be on its own line. |
Do we have examples of how Google solve this problem with its products? It could be an interesting source to benchmark with. |
@oliviertassinari Here is just the search results which shows grey text above giving a rough overview of the results. Could use a combination of greyed text and a grey underline. It would be quite subtle. I don't think I would like bold text. Bold text would be better used to put emphasise on words in found results (i.e. words matching the word you searched). In the search results section you can see bold text is used to put emphasis on words not inputed by the user. |
Sorry, I should have been more precise. I meant how the select + categories/groups problem is solved. |
I would advise against looking at what Google does.
First and foremost I would look at accessibility guidelines, and go by those. Material's own guidelines are quite good: https://material.io/design/usability/accessibility.html Quote: Color and contrast can be used to help users see and interpret your app’s content, interact with the right elements, and understand actions. Color can help communicate mood, tone, and critical information. Primary, secondary, and accent colors can be selected to support usability. Sufficient color contrast between elements can help users with low vision see and use your app. Contrast ratios Color contrast is important for users to distinguish various text and non-text elements. Color contrast is important for users to distinguish various text and non-text elements. end quote |
@dmitriid we won't be able to advance the resolution of this concern without benchmarking how others are solving this issue. I have noticed that select2 uses an indent. I would highly recommend to look at what Google is doing. I'm not that much interested in the consistency of their product nor how well they follow the specification. It's not a hard science. It's subject to interpretation. |
@oliviertassinari Good point. I've quickly looked around.
|
Thanks, very interesting. |
It looks like very few (easily Googlable) projects to grouping in autocompletes |
We have the same problem with the Select component. Here is an example with Angular Material: https://material.angular.io/components/select/overview#creating-groups-of-options. |
Looking more closely at the material design specification and the google products, it seems that groups are not something they support. So, I think that we can take some freedom. I would personally be in favor of introducing a light left indent (to follow native selects) and look at how we can solve #18200 at the same time. |
👍 1em or 2em will probably be more than enough (but needs to be checked with a variety of text lengths). |
It seems that long option names do not get any special handling with the current implementation, at least from my limited testing here: https://codesandbox.io/s/mui-select-long-names-448b3 |
I would propose the following diff: diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
index 463ff0f2f..3e7e1eb47 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
@@ -162,17 +162,14 @@ export const styles = theme => ({
...theme.typography.body1,
overflow: 'hidden',
margin: '4px 0',
- '& > ul': {
- maxHeight: '40vh',
- overflow: 'auto',
- },
},
/* Styles applied to the `listbox` component. */
listbox: {
listStyle: 'none',
margin: 0,
padding: '8px 0px',
- position: 'relative',
+ maxHeight: '40vh',
+ overflow: 'auto',
},
/* Styles applied to the loading wrapper. */
loading: {
@@ -223,6 +220,9 @@ export const styles = theme => ({
/* Styles applied to the group's ul elements. */
groupUl: {
padding: 0,
+ '& $option': {
+ paddingLeft: 24,
+ },
},
}); What do you guys think about it? @d0minikt Do you want to open a pull request? :) |
Looks really good 👍 Can you try with longer category names? Though it still should look good :) |
28 pixels? |
I can make a PR to close this and #18200 together if you would like. |
@embeddedt I think that we can handle the Select and Autocomplete component independently in this case. I'm not sure how the solution for the Select would look like, it will likely be way different. But it's worth exploring :) |
Okay; I am working on a PR for the Autocomplete component. Once that's done I will look at the Select component. |
Thank you! 👍💪 |
Filed as a bug, because it's more of a usability bug
Current Behavior 😯
Category names are indistinguishable from regular items as there's not enough color contrast between items (it looks better on retina screenshots than in real life). This will be even more noticeable when category names are longer.
It may be worse with disabled items thrown into the mix (haven't tested this though).
Expected Behavior 🤔
Category names should be more pronounced.
Could be solved with any of: different color, bold font, underline. A horizontal rule before the start of the category, perhaps. Offset could probably work, but needs testing with longer category names, might still be confusing.
Steps to Reproduce 🕹
Tested on https://material-ui.com/components/autocomplete/ -> Grouped
Context 🔦
Material Design in general suffers from very low contrast, or not enough contrast between elements
Your Environment 🌎
Safari 13.0.4 (15608.4.9.1.3), MacOS 10.15.2
The text was updated successfully, but these errors were encountered: