feat(list-box): update selection style #4753

Make sure we are using the 16px icon for chevron and the checkmark.
Hover and selected states look like the same color. Should be using hover-ui for hover and selected-ui for selected
Inline listbox in vanilla and inline dropdown in react: The checkmark icon should be vertically aligned with the chevron icon. Right now it’s too far left.
The checkmark icon looks like the wrong color in the dark themes. It should be calling icon-01
Combobox is getting a secondary invalid border when it is set to invalid and disabled.
When we have overflow content inside an item that is selected, we need to still keep enough padding on the right hand side to allow space between the ellipses and to keep the checkmark aligned vertically with the chevron. Right now the checkmark icon is overlapping the ellipses.
Needs to look like this:

joshblack · 2019-11-22T16:23:17Z

Hey @asudoh! 👋 This looks like an enhancement PR, I think as a group we're tackling fixes through end of year (in particular accessibility) Would be great to focus our attention on those and reserve these types of PR for the new year 👍

asudoh · 2019-11-23T01:00:54Z

Seems that the referring issue (#3988) is an attempt to address the color contrast issue with selected dropdown item. @laurenmrice Any thoughts on the highlights? Thanks!

P.S. for @laurenmrice will take a look the comments next week.

asudoh · 2019-11-25T09:09:21Z

Thanks again @laurenmrice for reviewing!

Yes, 16 pixels version of checkmark icon is in use
I can see that the hover highlight is using $hover-ui and the selected item is using $selected-ui. Probably the difference was not observed because they are close (#e5e5e5 for $hover-ui, #e0e0e0 for $selected-ui)
Fixed the selected icon position in the inline variant
Fixed the checkmark icon color
Fixed the right margin of the long (selected) menu item
Fixed the invalid outline for disabled/invalid combination

laurenmrice

-There is still something odd going on with the hover and selected states. Here is an example of when something is already is selected it gets a lighter hover change if you move the cursor on that selected item. Then when you hover to an item that is not selected it still gets the same color as the selected item.

-For long strings of text with ellipses, can we have 16px padding on the left and right before and after the checkmark, and when there is no checkmark present have 48px? Right now the elipses going full line and then changing to shorten even more when selected with the checkmark gets jumpy.

Spec:

asudoh · 2019-11-27T07:13:21Z

Thanks @laurenmrice for another round of review! Fixed the right margin and the background color of the selected item.

joshblack · 2019-12-04T23:12:05Z

@laurenmrice how should this behave if scrollbars are enabled? Was running into:

joshblack

Looks great! Just had the one question with scroll bars

laurenmrice

Vanilla and React

The rules separating the items in the dropdown is getting cutoff on the right. they should be the same distance away as they are on the left.

To joshs comment: Checkmark should still stay vertically aligned with chevron if there is a scrollbar. would rather always have them in place instead of them jumping around.

joshblack · 2019-12-08T18:25:04Z

@laurenmrice I'm not sure if that is do-able in CSS as we don't know if a fixed scrollbar is being displayed, or not since it's an OS-level setting. I think we also will run into this with Windows/Edge, and the scrollbars themselves may have variable width (e.g. 15 vs 16px for macOS vs Windows)

laurenmrice · 2019-12-16T19:31:36Z

@joshblack sorry for delayed response, have been consumed by patterns project. just curious, do you know how often people keep scrollbars persistant? just trying to guage how often this kind of scenario would occur

joshblack · 2019-12-16T21:42:45Z

@laurenmrice it tends to come up for Windows/Linux users and for folks who have a mouse connected to their Mac through a USB (not a trackpad). I personally fall under that last situation, for example, since I always use a mouse when at work/at home.

joshblack · 2020-01-07T21:45:53Z

bump @laurenmrice if you have a sec!

laurenmrice · 2020-01-07T21:57:09Z

@joshblack oh ! sorry for delayed response on this. im going to discuss this with design tomorrow to get a consensus. 👍🏻

laurenmrice · 2020-01-08T19:56:57Z

@joshblack https://opensource.adobe.com/spectrum-css/components/dropdown/ adobe is treating the select state of a dropdown similarly to what we are trying to accomplish. Could you test to see how their checkmark adjusts with a persistent scrollbar in the scenarios you are talking about? (i don't use a mouse or have one with a usb)

joshblack · 2020-01-08T19:59:15Z

@laurenmrice for sure!

So for people on Windows/Linux and for those on Mac with a mouse wheel they'll end up seeing this version when it overflows 😞

laurenmrice · 2020-01-10T20:13:05Z

Thanks Josh for trying that out ! I would like to see if @asudoh could tryout some ways to see if it is possible to keep the checkmark inline with the chevron even if scrollbars are enabled. If he ends up finding its def not possible to keep them aligned in that scenario, then we can go this route.

asudoh · 2020-01-11T02:45:57Z

@laurenmrice Thank you for clarifying the goal you had in mind. I used to work on a logic of detecting if there is a scroll bar or not, and found out that it's error-prone and has performance problems.

laurenmrice

Ok if thats the case from that issue that trying to align things with enabled scrollbars is finicking and my not be a consistent experience then we can just leave it.

A couple more changes though:

The rules separating the items needs to be 16px away from the right hand side. Right now they are too short.

Double check to make sure we are using the right checkmark icon from elements. https://carbon-elements.netlify.com/icons/examples/preview/#16%2Fcheckmark

asudoh · 2020-01-15T06:22:29Z

Thank you for your review @laurenmrice! Fixed dropdown item border. Wrt checkmark icon, confirmed that checkmark/16 is used.

laurenmrice

So everything regarding the new selection state looks good for listbox, dropdown, combobox in vanilla and react. (However I see lots of bugs with these components in other aspects in Vanilla. I can make separate issues for those.)

asudoh · 2020-01-15T22:37:20Z

Cool thanks @laurenmrice for reviewing! Don't hesitate to file other bugs you found!

feat(list-box): update selection style

6ac16b4

Fixes carbon-design-system#3988.

asudoh added status: ready for review 👀 status: visual review 🔍 labels Nov 22, 2019

asudoh requested review from emyarod, abbeyhrt and laurenmrice November 22, 2019 11:55

asudoh requested a review from a team as a code owner November 22, 2019 11:55

fix(list-box): update selected icon position

385b573

laurenmrice requested changes Nov 22, 2019

View reviewed changes

fix(list-box): address design review comments

6a9caf2

laurenmrice requested changes Nov 26, 2019

View reviewed changes

laurenmrice removed the status: visual review 🔍 label Nov 26, 2019

fix(list-box): fix active color and margin

d4e0ade

emyarod added the status: visual review 🔍 label Dec 3, 2019

joshblack approved these changes Dec 4, 2019

View reviewed changes

laurenmrice requested changes Dec 6, 2019

View reviewed changes

laurenmrice requested changes Jan 13, 2020

View reviewed changes

joshblack added status: waiting for author's response 💬 and removed status: ready for review 👀 labels Jan 13, 2020

fix(list-box): fix item border style

4f715c9

Merge branch 'master' into list-box-selected-icon

18a7361

emyarod added status: ready for review 👀 and removed status: waiting for author's response 💬 labels Jan 15, 2020

emyarod requested a review from laurenmrice January 15, 2020 15:30

laurenmrice approved these changes Jan 15, 2020

View reviewed changes

Merge branch 'master' into list-box-selected-icon

d09335c

asudoh added status: ready to merge 🎉 and removed status: ready for review 👀 status: visual review 🔍 labels Jan 15, 2020

Merge branch 'master' into list-box-selected-icon

091be2f

asudoh merged commit 9e6335f into carbon-design-system:master Jan 15, 2020

asudoh deleted the list-box-selected-icon branch January 15, 2020 23:05

laurenmrice mentioned this pull request Jan 17, 2020

[Combo box] all themes hover & select style wrong #3204

Closed

joshblack mentioned this pull request Jan 31, 2020

chore(project): update carbon to v10.10.0-rc.0 #5236

Closed

asudoh mentioned this pull request Feb 6, 2020

[Dropdown] Change default selection behavior #3531

Closed

laurenmrice mentioned this pull request Feb 11, 2020

[dropdown] add selected state icon carbon-design-system/carbon-design-kit#283

Closed

5 tasks

tw15egan mentioned this pull request Feb 19, 2020

[Dropdown, Mulitselect]: inline variants truncate text too early #5389

Closed

abbeyhrt mentioned this pull request Feb 19, 2020

fix(list-box): remove 32px margin right on option #5400

Merged

This was referenced Feb 25, 2020

chore(release): v10.10.0 #5432

Closed

v10.10.0 Release #5398

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(list-box): update selection style #4753

feat(list-box): update selection style #4753

asudoh commented Nov 22, 2019 •

edited

Loading

netlify bot commented Nov 22, 2019 •

edited

Loading

netlify bot commented Nov 22, 2019 •

edited

Loading

netlify bot commented Nov 22, 2019 •

edited

Loading

laurenmrice left a comment •

edited

Loading

joshblack commented Nov 22, 2019

asudoh commented Nov 23, 2019

asudoh commented Nov 25, 2019

laurenmrice left a comment •

edited

Loading

asudoh commented Nov 27, 2019

joshblack commented Dec 4, 2019

joshblack left a comment

laurenmrice left a comment •

edited

Loading

joshblack commented Dec 8, 2019

laurenmrice commented Dec 16, 2019 •

edited

Loading

joshblack commented Dec 16, 2019

joshblack commented Jan 7, 2020

laurenmrice commented Jan 7, 2020 •

edited

Loading

laurenmrice commented Jan 8, 2020

joshblack commented Jan 8, 2020

laurenmrice commented Jan 10, 2020

asudoh commented Jan 11, 2020

laurenmrice left a comment

asudoh commented Jan 15, 2020

laurenmrice left a comment •

edited

Loading

asudoh commented Jan 15, 2020

feat(list-box): update selection style #4753

feat(list-box): update selection style #4753

Conversation

asudoh commented Nov 22, 2019 • edited Loading

Changelog

Testing / Reviewing

netlify bot commented Nov 22, 2019 • edited Loading

netlify bot commented Nov 22, 2019 • edited Loading

netlify bot commented Nov 22, 2019 • edited Loading

laurenmrice left a comment • edited Loading

Choose a reason for hiding this comment

joshblack commented Nov 22, 2019

asudoh commented Nov 23, 2019

asudoh commented Nov 25, 2019

laurenmrice left a comment • edited Loading

Choose a reason for hiding this comment

asudoh commented Nov 27, 2019

joshblack commented Dec 4, 2019

joshblack left a comment

Choose a reason for hiding this comment

laurenmrice left a comment • edited Loading

Choose a reason for hiding this comment

joshblack commented Dec 8, 2019

laurenmrice commented Dec 16, 2019 • edited Loading

joshblack commented Dec 16, 2019

joshblack commented Jan 7, 2020

laurenmrice commented Jan 7, 2020 • edited Loading

laurenmrice commented Jan 8, 2020

joshblack commented Jan 8, 2020

laurenmrice commented Jan 10, 2020

asudoh commented Jan 11, 2020

laurenmrice left a comment

Choose a reason for hiding this comment

asudoh commented Jan 15, 2020

laurenmrice left a comment • edited Loading

Choose a reason for hiding this comment

asudoh commented Jan 15, 2020

asudoh commented Nov 22, 2019 •

edited

Loading

netlify bot commented Nov 22, 2019 •

edited

Loading

netlify bot commented Nov 22, 2019 •

edited

Loading

netlify bot commented Nov 22, 2019 •

edited

Loading

laurenmrice left a comment •

edited

Loading

laurenmrice left a comment •

edited

Loading

laurenmrice left a comment •

edited

Loading

laurenmrice commented Dec 16, 2019 •

edited

Loading

laurenmrice commented Jan 7, 2020 •

edited

Loading

laurenmrice left a comment •

edited

Loading