Skip to content

[Emotion] Memoize styles + opinionated clean up more components#8172

Closed
cee-chen wants to merge 10 commits intoelastic:mainfrom
cee-chen:emotion/memoization-4
Closed

[Emotion] Memoize styles + opinionated clean up more components#8172
cee-chen wants to merge 10 commits intoelastic:mainfrom
cee-chen:emotion/memoization-4

Conversation

@cee-chen
Copy link
Copy Markdown
Contributor

Summary

Part of #7561

TODO: probably split this up to 2 PRs

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist
    • If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

+ remove unnecessary function for styles w/ no theme token

+ inline JSX where possible
- disabled opacity is fairly static and can be DRYed out

- loading spinner: no styles being applied so this isn't necessary
+ remove passed `layout` arg in favor of static definitions

- rewrite vertical vs horizontal gutters utils to be slightly less confusing

+ replace `calc()` with `mathWithUnits()`
+ prefer inline JSX
+ rerun VRT to confirm no regressions. changes are minute/from font aliasing
- replace `EuiHideFor/ShowFor` usage with a breakpoint hook instead
+ remove unnecessary fragments

+ remove some unnecessary callback consts, just inline them if they're not reused and we're not going to memoize them
- `list_group_item` has too much conditional JSX to deal with right now, should likely be split up into subcomponents in the future

+ remove unnecessary style functions, just use a static obj instead

+ re-run VRT screenshots
+ reduce # of style fns for horizontal steps, and remove unnecessary onClick fn in favor of just inlining it (if we're not reusing or memoizing it)

+ update VRT
- instead of another div and repeated CSS, just use `> *` (which has zero specificity and is very overrideable) to bump the icon/avatar above the vertical line

+ inline `iconRender` and `ariaLabel`, it's not reused and not particularly increasing readability by being a const
@cee-chen cee-chen added skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) tech debt emotion performance labels Nov 21, 2024
@kibanamachine
Copy link
Copy Markdown

Preview staging links for this PR:

@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

@github-actions
Copy link
Copy Markdown

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@github-actions github-actions bot added the stale-pr (Don't delete - used for automation) label Feb 19, 2025
@github-actions
Copy link
Copy Markdown

❌ We're automatically closing this PR due to lack of activity. Please comment if you feel this was done in error.

@github-actions github-actions bot added the stale-pr-closed (Don't delete - used for automation) label Feb 26, 2025
@github-actions github-actions bot closed this Feb 26, 2025
@JasonStoltz JasonStoltz reopened this Feb 26, 2025
@github-actions
Copy link
Copy Markdown

👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually?

@github-actions github-actions bot added community contribution (Don't delete - used for automation) and removed stale-pr-closed (Don't delete - used for automation) stale-pr (Don't delete - used for automation) labels Feb 26, 2025
@tkajtoch
Copy link
Copy Markdown
Member

tkajtoch commented Apr 8, 2025

Superseded by #8565

@tkajtoch tkajtoch closed this Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community contribution (Don't delete - used for automation) emotion performance skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants