Carbon: [Audit] Select

[lovemecomputer]

variants:

• Select
• Inline

🖼 design spec (internal link): https://ibm.ent.box.com/file/329981608564

changes:

Select:

• [x] dev: use type tokens (currently commented-out as TODO items in code)
  
  
  
  • label: $label-01 ✅
  
  
  • text in field: $body-short-01 ✅
  
  
• [x] dev: verify that we're using spacing tokens in field & label margin
• [x] dev: disabled label text should be $disabled-02 ✅ . verify which token/hex is being used
  
  
  
  • helper text should be same color as label when disabled
    
    
    
    
    
    • helper text type token: $label-01 ✅
    
    
    
    • helper text color tokens: enabled = $text-02; disabled = $disabled-02 ✅
    
    
    
  
  
• [x] dev: space between label & helper text is off—should be 2px
• [x] dev: verify error icon is there (it's not there on website)
• [x] ~screenshot: is this overlap in the error state an issue with the component, or its display on the website?~ yes, this is expected (we currently expect devs to apply bottom margin styles to accommodate)
  

Inline:

• [x] ~dev: width should always be the content width plus padding (fluid width) to match design spec. (it will get small with short content)~ de-scoped, see comment below
• [x] dev: disabled label using wrong color/token
  
  
  
  • helper text should be same color as label when disabled
  
  

React

• [ ] do an additional audit pass for react

FYI @aagonzales

[emyarod]

text in field: body-01

can we double check the name of this token?

dev: space between label & helper text is off—should be 2px

I believe this is covered by #1968

dev: verify that we're using spacing tokens in field & label margin

aside from some custom values it looks like we are using the spacing tokens

dev: verify error icon is there (it's not there on website)

if you are referring to https://next.carbondesignsystem.com/ then I think the dependencies are outdated

screenshot: is this overlap in the error state an issue with the component, or its display on the website?

where is this screenshot taken from?

dev: width should always be the content width plus padding (fluid width) to match design spec. (it will get small with short content)

would we have the same behavior for long content?

[asudoh]

CC @IBM/carbon-designers to see if our designers have answers to some of above questions. Thanks!

[aagonzales]

I edited the original comment with the correct token names and added a ✅ as verified. You can double check both color and type tokens here: https://ibm.box.com/s/3prwmp421wu6umgayvejehiy0r1hi9m4

[lovemecomputer]

thanks @aagonzales !! going thru these double-checks now.

also looking into other outstanding questions. the screenshotted source of the Select input error screenshot seems to have been updated, i'm looking into reproducing this bug locally to test if it was resolved

[lovemecomputer]

update: screenshotted issue marked as resolved; we assume devs apply appropriate margin on wrapper class themselves. may change this in future version

[lovemecomputer]

issue for Select page on website: https://github.com/carbon-design-system/carbon-website/issues/1276

[lovemecomputer]

✅ color token usage verified

✅ spacing token usage verified, aside from custom values, of which there are a few:

.#{$prefix}--select-input {
    …
    height: rem(40px);
    …
    width: rem(224px);
    min-width: rem(128px);
    max-width: rem(448px);
    padding: 0 rem(34px) 0 $spacing-md; // 1.5rem + chevron width
    …
}

[emyarod]

just want to make sure we're not duplicating work, I began a draft PR to address these a few days ago but needed clarification on several of the points that came out of the audit https://github.com/IBM/carbon-components/pull/1978

[aagonzales]

just assign me as reviewer in whichever PR when its ready

[emyarod]

the screenshotted error disappeared after we merged component previews https://github.com/IBM/carbon-components/pull/1983, so the error still persists if we remove the demo styles. opened https://github.com/IBM/carbon-components/issues/2023 to track this

[emyarod]

dev: width should always be the content width plus padding (fluid width) to match design spec. (it will get small with short content)

just to clarify on this point: if we have items of varying width as options in the menu list, do we want the width to change as the user selects different options?

[lovemecomputer]

@emyarod i think it is specced so that the currently-displayed value will set what the width of the displayed select field will be. which implies changing widths when other values are selected. @aagonzales or @laurenmrice might be able to answer that question more accurately tho.

[laurenmrice]

@emyarod @lovemecomputer I believe that only the Inline Select would vary in length based on the text within the field. For Normal Selects you should be able to control what width you want to set regardless of text length.

[lovemecomputer]

@laurenmrice thanks!! just to verify: will inline select be sized based on the largest option available, or will it be sized based on the currently selected option? (the latter implying that it will change width as you select different options with different text lengths)

[aagonzales]

will it be sized based on the currently selected option?

This one. The select field and the open menu widths should NOT be dependent on each other.

[emyarod]

Is this bit of custom logic something we want to roll into our library? I'm not sure there's a reliable way to implement this given our current restrictions (relying on native <select> and avoiding custom logic as much as possible). I ran into a similar situation in #1756, and while I'm fine with adding some custom logic into our component to get this presentational behavior, I want to confirm that this is the direction we want to pursue

[asudoh]

@aagonzales I think our select content historically has had a requirement for user to specify its size explicitly, because how its visual is constructed doesn't allow us to auto-size the trigger button upon the content. I see a desire for auto-sizing from designer's perspective, but wondering if we can defer this until we find a trick? CC @tw15egan in case he has an idea.

[aagonzales]

If the developer can some how control the size that would be acceptable. Especially, with the inline selects, the arrow can become very disassociated with the field input. This makes for very bad ux.

[lovemecomputer]

after discussion btwn @emyarod and @aagonzales, we decided to leave Inline Select width as-is (fixed width). product teams will be able to set their own width manually for the time being.

further fixes and development to achieve dynamic width for Select can be tracked post-v10-launch here: https://github.com/IBM/carbon-components/issues/2063

note: also added https://github.com/IBM/carbon-components/issues/2066 to track adding content on the website to clarify difference between Select and Dropdown

@emyarod 's PR for Select, which should resolve vanilla issues, listed in this Audit post: https://github.com/IBM/carbon-components/pull/1978

thanks y'all! close to completion aside from checking react

[emyarod]

@aagonzales @lovemecomputer thanks for the clarification, I've marked the PR as ready for review

[carbon-bot]

:tada: This issue has been resolved in version 9.84.7 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: