Skip to content

[Emotion][perf] Memoize EuiButton styles & mixins#7541

Merged
cee-chen merged 5 commits intoelastic:mainfrom
cee-chen:emotion/memoize-button-styles
Feb 27, 2024
Merged

[Emotion][perf] Memoize EuiButton styles & mixins#7541
cee-chen merged 5 commits intoelastic:mainfrom
cee-chen:emotion/memoize-button-styles

Conversation

@cee-chen
Copy link
Copy Markdown
Contributor

@cee-chen cee-chen commented Feb 22, 2024 •
edited by 1Copenut
Loading

Summary

This PR is a performance enhancement (+ one missing Emotion animation conversion) on the EuiButton component(s) and should not regress or change any output UI or generated classNames.

As always, I recommend code reviewing by commit.

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
      - [ ] Checked in mobile
  • Docs site QA - N/A
  • Code quality checklist
  • Release checklist
    • [ ] A changelog entry exists and is marked appropriately. - Skipping the changelog on these PRs as they shouldn't affect either end-users or consumers
  • Designer checklist - N/A

@cee-chen
[mixins] Memoize useEuiButtonFocusCSS
3a88b12 
+ fix animation relying on existing Sass CSS instead of generating actual keyframes from Emotion
@cee-chen
[mixins] Memoize useEuiButtonColorCSS
c44cbc2 
- use nested forEach instead of a function
- invert the color/display mapping to make it easier to just return the expected colors obj
- use switch

+ manually tweak Emotion label output to be the same as prod
@cee-chen
Basic memoization of button styles
bb99273
@cee-chen
[EuiButtonIcon] memoize empty hover color styles
b1e9e8c
@cee-chen
[EuiButtonGroup] Remove unnecessary function from euiButtonGroupStyles
bcca47d
@cee-chen cee-chen changed the title [Emotion][perf] Memoize EuiButton styles [Emotion][perf] Memoize EuiButton styles & mixins Feb 22, 2024
@cee-chen cee-chen added emotion performance skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) labels Feb 22, 2024
@cee-chen cee-chen marked this pull request as ready for review February 22, 2024 21:55
@cee-chen cee-chen requested a review from a team as a code owner February 22, 2024 21:55
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Feb 22, 2024

Preview staging links for this PR:

@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Feb 22, 2024

💚 Build Succeeded

1Copenut
1Copenut approved these changes Feb 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

👍 LGTM!

@cee-chen cee-chen merged commit dd3ff4b into elastic:main Feb 27, 2024
@cee-chen cee-chen deleted the emotion/memoize-button-styles branch February 27, 2024 11:04
@cee-chen cee-chen mentioned this pull request Mar 7, 2024
Open
22 tasks
@cee-chen cee-chen mentioned this pull request Mar 29, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@1Copenut 1Copenut 1Copenut approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cee-chen @kibanamachine @elasticmachine @1Copenut