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

[EuiAccordion] Remove will-change CSS #6235

Merged
merged 5 commits into from
Sep 19, 2022

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Sep 13, 2022

Summary

EuiAccordion's will-change CSS creates a stacking context which causes nested positioned components to behave unexpectedly, e.g. fullscreen mode in EuiDataGrid:

We're opting removing this CSS for several reasons:

  • [EuiAccordion] Replace translateZ with will-change #5806:
    • "It's also very possible that we don't need to engage the GPU at all and even transform: translatez(0); was a preoptimization. In that case, we can remove will-change, too"
  • It looks like translateZ was first added by 5 years ago in a relatively unrelated component PR, so we're guessing the addition this was an unnecessary pre-optimization that was popular in the frontend world at the time
  • Mozilla/MDN has since recommended against using will-change:
    • "Warning: will-change is intended to be used as a last resort, in order to try to deal with existing performance problems. It should not be used to anticipate performance problems"

QA

Checklist

  • A changelog entry exists and is marked appropriately
  • Revert [REVERT ME] commit

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6235/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6235/

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Change LGTM, tested the accordion examples across browsers and don't see any difference

@cee-chen cee-chen enabled auto-merge (squash) September 19, 2022 16:12
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6235/

@cee-chen cee-chen merged commit ab2693a into elastic:main Sep 19, 2022
@cee-chen cee-chen deleted the accordion/will-change branch September 19, 2022 17:11
@cee-chen cee-chen mentioned this pull request Sep 20, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants