Skip to content

[Emotion][perf] Memoize loading components#7649

Merged
cee-chen merged 4 commits intoelastic:mainfrom
cee-chen:emotion/memoization-loading
Apr 4, 2024
Merged

[Emotion][perf] Memoize loading components#7649
cee-chen merged 4 commits intoelastic:mainfrom
cee-chen:emotion/memoization-loading

Conversation

@cee-chen
Copy link
Copy Markdown
Contributor

@cee-chen cee-chen commented Apr 3, 2024 •
edited by mgadewoll
Loading

Summary

Part of #7561 efforts

This PR:

  • Memoizes all EuiLoading* components
  • Refactors how EuiLoadingChart styles are written/generated to use static :nth-child CSS instead of dynamic JS styles (98ebe26). Also removes its --mono className modifier - no usages in Kibana
  • All other components were fairly straightforward

QA

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes. Also checked with Reduced motion setting
  • Docs site QA - N/A
  • Code quality checklist - N/A, all tests should pass
  • Release checklist - N/A, skipping a changelog as this is mostly tech debt and shouldn't affect either consumers or end-users
  • Designer checklist - N/A

cee-chen added 4 commits April 3, 2024 12:22
@cee-chen
Memoize loading spinner
c1468a5 
- not memoizing the inline style - usage is fairly low / it's not worth it
+ add story for easier testing
@cee-chen
Memoize loading logo + remove unnecessary extra style fn
4ac9a04 
- it's not super complex, so just pair it with the main styles
@cee-chen
Elastic logo - remove unnecessary styles fn
6da43a5 
- doesn't need theme tokens so can just be a static obj
@cee-chen
Refactor/memoize EuiLoadingChart
98ebe26 
- prefer static nth-child CSS over a dynamically generated function (since it's not like the bar count is customizable via prop)

- inline the bar JSX and replace for loop with Array.from

+ fix the duplicate loading i18n (should be using the shared string with the same copy)
@cee-chen cee-chen mentioned this pull request Apr 3, 2024
Open
22 tasks
@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 Apr 3, 2024
@cee-chen cee-chen marked this pull request as ready for review April 3, 2024 20:25
@cee-chen cee-chen requested a review from a team as a code owner April 3, 2024 20:25
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Apr 3, 2024

Preview staging links for this PR:

@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Apr 3, 2024

💚 Build Succeeded

@tkajtoch tkajtoch self-requested a review April 4, 2024 11:46
tkajtoch
tkajtoch approved these changes Apr 4, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

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

Changes look and work great!

@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Apr 4, 2024

Thanks Tomasz!

@cee-chen cee-chen merged commit bed1865 into elastic:main Apr 4, 2024
@cee-chen cee-chen deleted the emotion/memoization-loading branch April 4, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tkajtoch tkajtoch tkajtoch approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cee-chen @kibanamachine @elasticmachine @tkajtoch