Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Emotion][perf] Memoize low-impact components (D-F) #7637

Closed
wants to merge 7 commits into from

Conversation

cee-chen
Copy link
Member

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

Summary

Basically every component that isn't on the medium impact list (#7561) and was easier to do a quick skim through. Some of the commits/components have extra cleanup in terms of useMemo usage, preferring inline JSX, and removing array/obj definitions. As always, I recommend following by commit.

QA

  • Quick smoke test of all the affected components

General checklist

  • Browser QA - Skipping cross-browser testing in favor of smoke testing
  • Docs site QA - N/A
  • Code quality checklist - N/A
  • Release checklist - N/A, skipping changelog due to this being perf tech debt that shouldn't impact consumers
  • Designer checklist - N/A

+ simplify styles array

- not sure I love the extra inline styles always being memoized even if they aren't used, but c'est la vie
+ 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
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

Copy link

👋 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.

Copy link

👋 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.

Copy link

github-actions bot commented Oct 4, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants