carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.85k stars 1.81k forks source link

Combo box styling using input[value='...'] is not framework agnostic #3290

Closed lee-chase closed 5 years ago

lee-chase commented 5 years ago

2983 What package(s) are you using?

Creating @carbon/vue issue is with styling for combo box based on an input's value attribute.

Detailed description

The ComboBox has css targetting the text input value attribute. Relying on the value attribute to be the same as the dom node value property is an outdated way of achieving this, which requires extra work in frameworks that have do not mirror these two values automatically.

Is this issue related to a specific component? Yes the ComboBox and probably MutliSelect too.

What did you expect to happen? What happened instead? What would you like to see changed?

CSS should target :placeholder-shown

https://developer.mozilla.org/en-US/docs/Web/CSS/:placeholder-shown

What browser are you working in? Chrome

What version of the Carbon Design System are you using? 10 latest still an issue in dev

What offering/product do you work on? Any pressing ship or release dates we should be aware of?

Steps to reproduce the issue

  1. Here http://react.carbondesignsystem.com/?path=/story/combobox--default
  2. Set the combo box to invalid and check the padding of the input in the developer tools.
  3. Note CSS selectors .bx--list-boxfield .bx--text-input[value=''] .bx--list-box[data-invalid] .bx--list-boxfield .bx--text-input[value='']
  4. Add some text and not .bx--list-boxfield .bx--text-input[value] .bx--list-box[data-invalid] .bx--list-boxfield .bx--text-input[value]

Additional information

lee-chase commented 5 years ago

Alternatively, the input field could be set to required and :valid or :invalid checked.

emyarod commented 5 years ago

@asudoh is ::placeholder-shown pseudo class fine to target? Edge does not support it but Chromium based Edge is on the horizon

asudoh commented 5 years ago

@emyarod Your question reminds me of IE... What happens with IE?

emyarod commented 5 years ago

@asudoh seems it's supported in IE 10+ with a prefix

lee-chase commented 5 years ago

On a slightly more fundamental level, what is the purpose of having less padding when the input value is empty? Should the CSS not simply be

.bx--list-box__field .bx--text-input { padding-right: 4.5rem; }

emyarod commented 5 years ago

fixed padding regardless of whether or not the input field is populated is certainly an option to consider, but I believe the original intent was to account for the "clear input field" button being inserted and removed

image

asudoh commented 5 years ago

@asudoh seems it's supported in IE 10+ with a prefix

OK sounds that autoprefixer will take care of that for us! 😉