[Emotion] Convert EuiSelectableTemplateSitewide #7944
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- this requires passing the styles obj to the `euiSelectableTemplateSitewideFormatOptions` function, which isn't actually a react component :| + remove unnecessary `!important` + fix import order/unnecessary extra dir
- Ignore `$euiSelectableTemplateFocusBackgroundLight/Dark` - we can't meaningfully use this background in our chroma.js calculations as chroma can't calculate contrast w/ alpha transparency. Using the default body background color should suffice / generates the same output colors
- again, these files aren't React components so we have to continuously pass the memoized style obj around, and use `RenderWithEuiStylesMemoizer` :T - [syntax] convert `renderOptionMeta` to obj arg notation for easier DX and fallbacks
- render functions don't snapshot well, so we should just render the DOM + update the `searchValue` test to be marginally more useful/pointed
- not a lot we can for `__listItem`, but we can at least dogfood `euiSelectableTemplateSitewideRenderOptions` children
- doesn't appear to be needed anymore, not totally sure why tho
+ add a vrt-only one to allow for snapshotting options
mgadewoll
reviewed
Aug 6, 2024
...s/eui/src/components/selectable/selectable_templates/selectable_template_sitewide_option.tsx
Show resolved
Hide resolved
I think it's intended and it's due to the line-height changes/reductions Caroline implemented as part of the Emotion conversion. Also, it looks better and more balanced this way (just my 2 cents!) |
Yeah I'm fine with it too, I just didn't understand where it came from, so I figured better confirm. 👍 |
mgadewoll
approved these changes
Aug 6, 2024
💚 Build Succeeded
History
|
This was referenced Aug 12, 2024
cee-chen
added a commit
to elastic/kibana
that referenced
this pull request
Aug 14, 2024
`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))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Follow up to #7940
EuiSelectableTemplateSitewide has some weirdness where the options file doesn't actually contain any React components, so some extra shenanigans with passing around styles is needed. As always, I recommend following along by commit.
QA
General checklist
and screenreader modesclassNames directlyand 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 browsers[ ] Checked component playgroundUnit tests
[ ]- already done previouslyshouldRenderCustomStyles()test was added and passes with parent component and any nestedchildProps(e.g.tooltipProps)[ ] Converted Enzyme to RTL- already RTLSass/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)[ ] SEARCH KIBANA FIRST: Deleted any modifier classNames or maps if not being used in Kibana.- no modifiers removedKibana 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:[ ] Any test/query selectors that will need to be updated[ ] Any Sass or CSS that will need to be updated, particularly if a component Sass var was deletedeuiBadgeclass 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 component2 Kibana direct className usages that need to be checked: