[Emotion] Convert EuiSearchBar by cee-chen · Pull Request #7490 · elastic/eui

cee-chen · 2024-01-26T19:35:20Z

Summary

This is a pretty quick/mildly dirty Emotion conversion of EuiSearchBar, due to the nature of the class component (harder to use hooks for) and very minor CSS (only 2 rules, nothing on the main wrapper, just child elements). I strongly recommend viewing the diff with whitespace changes hidden.

While I was here, I also refactored some of the rendered JSX to not use a variable unnecessarily (which has react perf implications) but instead as an inline JSX conditional.

QA

Compare staging against production and confirm no UI regressions

General checklist

Browser QA
- Checked in both light and dark modes
- Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
Docs site QA - N/A
Code quality checklist - N/A
Release checklist
- A changelog entry exists and is marked appropriately.
Designer checklist - N/A

Emotion checklist

Kibana usage

Pre-emptively check how your conversion will impact the next Kibana upgrade. This entails searching/grepping through Kibana (excluding **/target, **/*.snap, **/*.storyshot for less noise) for eui{Component} (case sensitive) to find:
~~[ ] Search Kibana's codebase for {euiComponent}- (case sensitive) to check for usage of modifier classes~~ - No modifier classes changed
Any test/query selectors that will need to be updated - None
Any Sass or CSS that will need to be updated, particularly if a component Sass var was deleted - None
Any direct className usages that will need to be refactored (e.g. someone calling the euiBadge class on a div instead of simply using the EuiBadge component) - None

General

Output CSS matches the previous CSS (works as expected in all browsers)
Rendered className(s) read as expected in snapshots and browsers
~~[ ] Checked component playground~~ No playground

Unit tests

~~[ ] shouldRenderCustomStyles() test was added and passes with parent component and any nested childProps (e.g. tooltipProps)~~
- N/A, this component doesn't appear to pass any custom classNames or styles
Enzyme tests converted to RTL

Sass/Emotion conversion process

Converted all global Sass vars/mixins to JS (e.g. $euiSize to euiTheme.size.base)
~~[ ] Removed or converted component-specific Sass vars/mixins to exported JS versions~~
~~[ ] Listed var/mixin removals in changelog~~
~~[ ] Simplified calc() to mathWithUnits if possible (if mixing different unit types, this may not be possible)~~
~~[ ] Added an @warn deprecation message within the global_styling/mixins/{component}.scss file~~
All SCSS files have been removed from the component(s) directory
~~[ ] All SCSS overrides have been removed from the Amsterdam theme~~ No overrides

CSS tech debt

~~[ ] Wrapped all animations or transitions in euiCanAnimate~~
~~[ ] Used gap property to add margin between items if using flex~~
Converted side specific padding, margin, and position to -inline and -block logical properties (check inline styles as well as CSS)

DOM Cleanup

Did NOT remove any block/element classNames (e.g. euiComponent, euiComponent__child)
~~[ ] SEARCH KIBANA FIRST: Deleted any modifier classNames or maps if not being used in Kibana.~~ No DOM or className changes/removals

Extras/nice-to-have

~~[ ] Reduced specificity where possible (usually by reducing nesting and class name chaining)~~ Wasn't necessary
~~[ ] Documentation pass~~
~~[ ] Check for issues in the backlog that could be a quick fix for that component~~
~~[ ] Optional component/code cleanup: consider splitting up the component into multiple children if it's overly verbose or difficult to reason about~~

cee-chen · 2024-01-26T19:38:07Z

src/components/search_bar/search_bar.styles.ts

+import { euiFormVariables } from '../form/form.styles';
+
+export const euiSearchBar__searchHolder = (euiThemeContext: UseEuiTheme) => {
+  const { maxWidth } = euiFormVariables(euiThemeContext);


This very likely has performance implications as well (see #7486) and we likely need to memoize euiFormVariables (and other CSS-in-JS component style vars) going forward. I'll address that in a separate PR from this one

kibanamachine · 2024-01-26T20:20:46Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7490/
Storybook: https://eui.elastic.co/pr_7490/storybook

elasticmachine · 2024-01-26T20:20:49Z

💚 Build Succeeded

Buildkite Build
Commit: 8781e63

JasonStoltz

LGTM. I had one question about logical CSS properties that's not a blocker.

src/components/search_bar/search_bar.styles.ts

JasonStoltz · 2024-02-02T16:58:24Z

src/components/search_bar/search_bar.tsx

+                }
+              />
+            </EuiFlexItem>
+            {filters && (


While I was here, I also refactored some of the rendered JSX to not use a variable unnecessarily (which as react perf implications) but instead as an inline JSX conditional.

That's wild that this has a performance implication, I had no idea. I assume this is what you're referring to. I'm going to link to this change specifically as a reference on #7499

Kevin chatted about me with this in relation to #7374, but iirc the issue is that React creates the conditional node let as a new React component every single rerender. @kqualters-elastic feel free to chime in if I've misquoted you!

`v92.2.1` ⏩ `v93.0.0` --- ## [`v93.0.0`](https://github.com/elastic/eui/releases/v93.0.0) **Bug fixes** - Fixed `EuiTextTruncate` component to clean up timer from side effect on unmount ([#7495](elastic/eui#7495)) **Breaking changes** - Removed deprecated `anchorClassName` prop from `EuiPopover`. Use `className` instead ([#7488](elastic/eui#7488)) - Removed deprecated `buttonRef` prop from `EuiPopover`. Use `popoverRef` instead ([#7488](elastic/eui#7488)) - Removed deprecated `toolTipTitle` and `toolTipPosition` props from `EuiContextMenuItem`. Use `toolTipProps.title` and `toolTipProps.position` instead ([#7489](elastic/eui#7489)) - Removed deprecated internal `setSelection` ref method from `EuiInMemoryTable` and `EuiBasicTable`. Use the new controlled `selection.selected` prop API instead. ([#7491](elastic/eui#7491)) - `EuiTourStep`'s `className` and `style` props now apply to the anchoring element instead of to the popover panel, to match `EuiPopover` behavior. ([#7497](elastic/eui#7497)) - Convert your existing usages to `panelClassName` and `panelStyle` respectively instead. **Performance** - Improved the amount of recomputed styles being generated by `EuiCode` and `EuiCodeBlock` ([#7486](elastic/eui#7486)) **CSS-in-JS conversions** - Converted `EuiSearchBar` to Emotion ([#7490](elastic/eui#7490)) - Converted `EuiEmptyPrompt` to Emotion ([#7494](elastic/eui#7494)) - Added `euiBorderColor` and `useEuiBorderColorCSS` style utilities ([#7494](elastic/eui#7494)) --------- Co-authored-by: Jon <jon@elastic.co>

`v92.2.1` ⏩ `v93.0.0` --- ## [`v93.0.0`](https://github.com/elastic/eui/releases/v93.0.0) **Bug fixes** - Fixed `EuiTextTruncate` component to clean up timer from side effect on unmount ([elastic#7495](elastic/eui#7495)) **Breaking changes** - Removed deprecated `anchorClassName` prop from `EuiPopover`. Use `className` instead ([elastic#7488](elastic/eui#7488)) - Removed deprecated `buttonRef` prop from `EuiPopover`. Use `popoverRef` instead ([elastic#7488](elastic/eui#7488)) - Removed deprecated `toolTipTitle` and `toolTipPosition` props from `EuiContextMenuItem`. Use `toolTipProps.title` and `toolTipProps.position` instead ([elastic#7489](elastic/eui#7489)) - Removed deprecated internal `setSelection` ref method from `EuiInMemoryTable` and `EuiBasicTable`. Use the new controlled `selection.selected` prop API instead. ([elastic#7491](elastic/eui#7491)) - `EuiTourStep`'s `className` and `style` props now apply to the anchoring element instead of to the popover panel, to match `EuiPopover` behavior. ([elastic#7497](elastic/eui#7497)) - Convert your existing usages to `panelClassName` and `panelStyle` respectively instead. **Performance** - Improved the amount of recomputed styles being generated by `EuiCode` and `EuiCodeBlock` ([elastic#7486](elastic/eui#7486)) **CSS-in-JS conversions** - Converted `EuiSearchBar` to Emotion ([elastic#7490](elastic/eui#7490)) - Converted `EuiEmptyPrompt` to Emotion ([elastic#7494](elastic/eui#7494)) - Added `euiBorderColor` and `useEuiBorderColorCSS` style utilities ([elastic#7494](elastic/eui#7494)) --------- Co-authored-by: Jon <jon@elastic.co>

cee-chen added 2 commits January 25, 2024 16:58

Convert EuiSearchBar Sass to Emotion

597907f

Delete Sass files

bf92f7a

cee-chen commented Jan 26, 2024

View reviewed changes

cee-chen added 2 commits January 26, 2024 11:38

changelog

d536600

[tech debt] convert Enzyme tests to RTL

8781e63

cee-chen added the emotion label Jan 26, 2024

cee-chen marked this pull request as ready for review January 26, 2024 21:32

cee-chen requested a review from a team as a code owner January 26, 2024 21:32

JasonStoltz self-assigned this Feb 2, 2024

JasonStoltz approved these changes Feb 2, 2024

View reviewed changes

cee-chen merged commit 3a257ad into elastic:main Feb 2, 2024

cee-chen deleted the emotion/search_bar branch February 2, 2024 17:34

This was referenced Feb 2, 2024

[Emotion] EuiSearchBar #6410

Closed

Upgrade EUI to v93.0.0 elastic/kibana#176246

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Emotion] Convert EuiSearchBar#7490

[Emotion] Convert EuiSearchBar#7490
cee-chen merged 4 commits intoelastic:mainfrom
cee-chen:emotion/search_bar

cee-chen commented Jan 26, 2024 •

edited

Loading

Uh oh!

cee-chen Jan 26, 2024 •

edited

Loading

Uh oh!

kibanamachine commented Jan 26, 2024

Uh oh!

elasticmachine commented Jan 26, 2024

Uh oh!

JasonStoltz left a comment

Uh oh!

Uh oh!

JasonStoltz Feb 2, 2024 •

edited

Loading

Uh oh!

JasonStoltz Feb 2, 2024

Uh oh!

cee-chen Feb 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

cee-chen commented Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

QA

General checklist

Emotion checklist

Uh oh!

cee-chen Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jan 26, 2024

Uh oh!

elasticmachine commented Jan 26, 2024

💚 Build Succeeded

Uh oh!

JasonStoltz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JasonStoltz Feb 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JasonStoltz Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

cee-chen Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cee-chen commented Jan 26, 2024 •

edited

Loading

cee-chen Jan 26, 2024 •

edited

Loading

JasonStoltz Feb 2, 2024 •

edited

Loading