Skip to content

[Emotion] Memoize EuiPanel + color/padding hook utilities + EuiResizableContainer/Panel#7584

Merged
cee-chen merged 12 commits intoelastic:mainfrom
cee-chen:emotion/memoization-panels
Mar 21, 2024
Merged

[Emotion] Memoize EuiPanel + color/padding hook utilities + EuiResizableContainer/Panel#7584
cee-chen merged 12 commits intoelastic:mainfrom
cee-chen:emotion/memoization-panels

Conversation

@cee-chen
Copy link
Copy Markdown
Contributor

@cee-chen cee-chen commented Mar 15, 2024

Summary

This PR is an Emotion performance pass of the following components/utilities:

  • EuiPanel
  • EuiResizablePanel (+ all other EuiResizableContainer components while here)
  • Padding utilties (useEuiPaddingCSS)
  • Color utilities (useEuiBackgroundColorCSS & useEuiBorderColorCSS)

As always, I recommend following along by commit.

QA

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in Chrome, Safari, Edge, and Firefox
      - [ ] Checked in mobile
      - [ ] Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA - N/A
  • Code quality checklist - N/A, tech debt
  • Release checklist - N/A, tech debt and shouldn't affect consumers or end-users
  • Designer checklist - N/A

- in anticipation of upcoming memoization
+ remove unnecessary fn from static styles
…orCSS`

- Skipping `useEuiBackgroundColor` the extra `method` throws a wrench into things and it's not actually used by any components
- because this style has a 3rd level of nesting and our typing only goes down to 2, I've opted to just get rid of the strict typing completely. The return type correctly interprets the output in any case

- this also lets us get rid of a `<any>` usage
- valid for Emotion
- also, not sure how this hasn't come up before??
@cee-chen cee-chen added skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) emotion performance labels Mar 15, 2024
@cee-chen cee-chen marked this pull request as ready for review March 15, 2024 00:33
@cee-chen cee-chen requested a review from a team as a code owner March 15, 2024 00:33
- used by Kibana after all, and color computations are expensive, so we should memoize it
- at some minor/potential cost to performance
@cee-chen cee-chen force-pushed the emotion/memoization-panels branch from a301ab9 to 4fba5e7 Compare March 20, 2024 18:06
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Mar 20, 2024

💚 Build Succeeded

History

@kibanamachine
Copy link
Copy Markdown

Preview staging links for this PR:

Copy link
Copy Markdown
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 🐈‍⬛ LGTM!
Thanks for the additional updates for improved readability. 🙏

@cee-chen
Copy link
Copy Markdown
Contributor Author

Thanks so much for the fantastic PR feedback Lene!! You made this code so much more readable! 🙇

@cee-chen cee-chen merged commit d75bccd into elastic:main Mar 21, 2024
@cee-chen cee-chen deleted the emotion/memoization-panels branch March 21, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants