[Emotion] Convert EuiSelectable #7940
Conversation
| const lighterBorder = transparentize(euiTheme.border.color, 0.4); | ||
| return { | ||
| border: `${euiTheme.border.width.thin} solid ${lighterBorder}`, |
There was a problem hiding this comment.
This was an intentional change from the previous Sass styles:
I deliberately wanted to use a color that we already use for borders instead of a random shade, and I also liked transparentize over darken (which wasn't showing up at all in dark mode) as it shows up better for focused items.
| } | ||
|
|
||
| &[aria-disabled='true'] { | ||
| color: ${euiTheme.colors.disabledText}; |
There was a problem hiding this comment.
This was another intentional change from production:
We should also be using the token we specifically have for disabled text over a random shade
| line-height: ${euiFontSize(euiThemeContext, 'm').lineHeight}; | ||
| font-size: ${euiFontSize(euiThemeContext, 's').fontSize}; |
There was a problem hiding this comment.
Just to clarify, Caroline changed the euiFontSize() mixin in Emotion/JS compared to Sass, generally reducing line height. To preserve the previous production appearance of EuiSelectable, I had to split up the line-height and font size mixins.
- might as well get it out of the way now, because Enzyme snapshots will make the Emotion conversion annoying
- hooray for no theme tokens needed
+ add `shouldRenderCustomStyles` test
…hEuiTheme` - `EuiSelectableList` is going to need this because it's a class component with a generic :T
+ leftover __groupLabel element + fix `euiFontSize(s)` which has reduced line height in Emotion - not sure what the intent was here but just going to preserve the old line height for now + replace `.euiSelectableListItem__tooltipAnchor` with a CSS utility className + delete Sass variables opinionated changes: - change disabled text to use disabled text color specifically, not medium shade - change item border to use the border color specifically (but transparentized to match prev lightness) instead of a random shade
cee-chen
force-pushed
the
emotion/selectable
branch
from
ad567df
to
75f7964
Compare
💚 Build Succeeded
History
|
|
@mgadewoll I think I'm going to yolo ahead with merging this to get the next conversion (EuiSelectableTemplateSitewide) out, but feel free to follow up on the util feedback there or suggest a specific example/comment! |
`v95.6.0` ⏩ `v95.7.0` _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- ## [`v95.7.0`](https://github.com/elastic/eui/releases/v95.7.0) **CSS-in-JS conversions** - Converted `EuiSelectable` to Emotion ([#7940](elastic/eui#7940)) - Removed `$euiSelectableListItemBorder` - Removed `$euiSelectableListItemPadding` - Converted `EuiSelectableTemplateSitewide` to Emotion ([#7944](elastic/eui#7944)) - Removed `$euiSelectableTemplateFocusBackgroundLight` - Removed `$euiSelectableTemplateFocusBackgroundDark` - Removed `$euiSelectableTemplateSitewideTypes` - Converted `EuiComboBox` to Emotion ([#7950](elastic/eui#7950))
Summary
Converts the baseline
EuiSelectablecomponent. Does not convertEuiSelectableTemplateSitewide, because this PR is already large enough (although most of the diffs are snapshots)QA
General checklist
and screenreader modesAdded orupdated jestand cypress tests- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)Emotion checklist
General
className(s)read as expected in snapshots and browsersUnit tests
shouldRenderCustomStyles()test was added and passes with parent component and any nestedchildProps(e.g.tooltipProps)Sass/Emotion conversion process
src/components/index.scss$euiSizetoeuiTheme.size.base)[ ] Added an@warndeprecation message within theglobal_styling/mixins/{component}.scssfile[ ] Removed or converted component-specific Sass vars/mixins to exported JS versions[ ] Ranyarn compile-scssto update var/mixin JSON files[ ] Simplifiedcalc()tomathWithUnitsif possible (if mixing different unit types, this may not be possible)[ ] Deleted anysrc/amsterdam/overrides/{component}.scssfiles (styles within should have been converted to the baseline Emotion styles)CSS tech debt
[ ] Wrapped all animations or transitions ineuiCanAnimate[ ] Usedgapproperty to add margin between items if using flex-inlineand-blocklogical properties (check inline styles as well as CSS)DOM Cleanup
euiComponent,euiComponent__child)Kibana due diligence
{euiComponent}-(case sensitive) to check for usage of modifier classes[ ] If usage exists, consider converting to adataattribute so that consumers still have something to hook into**/target, **/*.snap, **/*.storyshotfor less noise) foreui{Component}(case sensitive) to find:euiBadgeclass on a div instead of simply using theEuiBadgecomponent)Extras/nice-to-have
[ ] Reduced specificity where possible (usually by reducing nesting and class name chaining)[ ] Optional component/code cleanup: consider splitting up the component into multiple children if it's overly verbose or difficult to reason about[ ] Documentation pass[ ] Check for issues in the backlog that could be a quick fix for that componentModifier classes removed:
euiSelectable-fullHeighteuiSelectableList-fullHeighteuiSelectableList-borderedeuiSelectableMessage--borderedeuiSelectableListItem--paddingSmalleuiSelectableListItem__text--truncate&euiSelectableListItem__text--wrap