[EuiResizableFlyout] Optimize resizable flyout browser reflows

[davismcphee]

Summary

This PR optimizes browser reflows for EuiResizableFlyout by no longer toggling the euiBody--hasFlyout class or modifying body padding when the flyout size changes. This has a significant impact on the performance of the flyout in heavy pages such as Discover.

In my tests of frantically resizing the Discover flyout for 10 seconds while profiling, these changes brought the time Chrome spent on Recalculate Style from 4.8s to 1.1s:

Unoptimized:

Optimized:

And a couple of videos from Discover for reference:

Unoptimized:

https://github.com/elastic/eui/assets/25592674/0c07ed6d-c189-484a-b88e-f58852c5e179

Optimized:

https://github.com/elastic/eui/assets/25592674/7f029823-7c33-4867-8229-24a4c5cb18ec

QA

General checklist

• Browser QA
  • [x] Checked in both light and dark modes
  • [x] Checked in mobile
  • [x] Checked in Chrome, Safari, Edge, and Firefox
  • [ ] ~Checked for accessibility including keyboard-only and screenreader modes~
• Docs site QA
  • [ ] ~Added documentation~
  • [ ] ~Props have proper autodocs (using @default if default values are missing) and playground toggles~
  • [ ] ~Checked Code Sandbox works for any docs examples~
• Code quality checklist
  • [x] Added or updated jest and cypress tests
• Release checklist
  • [ ] ~A changelog entry exists and is marked appropriately.~
  • [ ] ~If applicable, added the breaking change issue label (and filled out the breaking change checklist)~
• Designer checklist
  • [ ] ~Updated the Figma library counterpart~

[cee-chen]

Hey @davismcphee! Just wanted to check in on this PR - it looks great to me honestly, should I go ahead and mark its as ready for review + add a changelog to it? Happy to take over and bring it across the finish line if you're busy with other things!

[kertal]

We are currently evaluating EuiDataGrid for the UnifiedDocViewer, and created an instance which applies resizable flyouts and a basic migration of the DocViewer, to measure the performance impact

your can test is here https://kertal-pr-176273-doc-viewer-push-and-eui-data-grid.kbndev.co/app/discover#/?_g=(filters:!(),refreshInterval:(pause:!t,value:60000),time:(from:now-15m,to:now))&_a=(columns:!(),filters:!(),index:ff959d40-b880-11e8-a6d9-e546fe2bba5f,interval:auto,query:(language:kuery,query:''),sort:!(!(order_date,desc)))

We merged also this fix in this deployment, it resolved the issue in Chrome 🥳 , but still, Firefox + Safari 😿 don't provide the expected performance, Chrome - Left, Firefox - Right

https://github.com/elastic/eui/assets/463851/84e39e5d-7232-4af3-b31c-f7b8e117e032

[cee-chen]

Adding a throttler/debouncer to the width changes would be the next step to improving performance - would that be something y'all would like to request as a feature/prop as well?

[kertal]

Adding a throttler/debouncer to the width changes would be the next step to improving performance - would that be something y'all would like to request as a feature/prop as well?

@cee-chen is this the reason for the performance difference between Chrome and the rest of the gang? If this is the case I think throttling resizing should be applied (not optional)

[cee-chen]

It's the most obvious approach that I can think of, but I have no idea if that's the difference between browsers, and it isn't guaranteed to fix the issue per se.

The other approach would be to check/optimize the number of rerenders on the rest of the page that's reflowing, as that's likely the culprit of repaint/reflow performance issues, but that's obviously far more involved & harder to do.

[davismcphee]

@cee-chen Apologies for taking so long getting back to you on this one! I finally had a chance to revisit this PR and track down the main culprit for the performance issue.

It turns out that in addition to toggling the euiBody--hasFlyout hurting performance, the most impactful issue was the useResizeObserver hook. Internally it uses getBoundingClientRect, which causes layout thrashing when combined with frequent writes to DOM elements, like when we set the body padding on resize. To fix it I updated EuiFlyout to use a ResizeObserver directly instead of depending on the hook, which resolves the performance issue in all browsers.

It looks like useResizeObserver relies on getBoundingClientRect because contentRect only provides the content sizing for elements instead of box sizing: https://github.com/elastic/eui/blob/635f860520e8cd981175438748705f27a51eb0ac/src/components/observer/resize_observer/resize_observer.tsx#L106-L109

But I assume this code is old and isn't actually needed anymore since ResizeObserverEntry now supports borderBoxSize to access box sizing. So technically this could have been solved directly within useResizeObserver, but I ended up just addressing EuiFlyout since I wasn't sure if we'd be worried about the impact of changing the hook (although it should technically work the same).

[cee-chen]

It looks like useResizeObserver relies on getBoundingClientRect because contentRect only provides the content sizing for elements instead of box sizing: [...] But I assume this code is old and isn't actually needed anymore since ResizeObserverEntry now supports borderBoxSize to access box sizing. So technically this could have been solved directly within useResizeObserver, but I ended up just addressing EuiFlyout since I wasn't sure if we'd be worried about the impact of changing the hook (although it should technically work the same).

Super interesting, thanks for discovering this Davis! I actually think this is important enough to address at the useResizeObserver level rather than just for EuiFlyout. I'd be open to another PR that fixes this for all usages of our resize observer(s) as I think that should greatly improve performance across the board.

WDYT?

[davismcphee]

@cee-chen Thanks for opening #7575! I think it's a great idea to implement this at the resize observer level. We'll probably even see performance gains elsewhere in Kibana just from that.

For this PR, the resize observer change would of course need to be reverted, but would you be open to still taking the original euiBody--hasFlyout class optimization? It seemed less impactful in Firefox and Safari in my tests, but still had a noticeable improvement on performance in Chrome.

[cee-chen]

@davismcphee Yes, 100%, I definitely want that change! 😄

[cee-chen]

@davismcphee Alrighty, I've updated the PR with latest main and pushed up the minor changes I had - did you want to re-test the performance impacts to make sure everything's still shipshape, or is this good to merge in your opinion?

[kibanamachine]

Preview staging links for this PR:

• Docs site: https://eui.elastic.co/pr_7462/
• Storybook: https://eui.elastic.co/pr_7462/storybook

[elasticmachine]

:green_heart: Build Succeeded

• Buildkite Build
• Commit: 313ae2df24dc392a63c3189c7c44d9690b898f24

History

• :green_heart: Build #1436 succeeded f85b22f7753e5c25524aea957b127d8c68a1261b
• :green_heart: Build #1432 succeeded 0624fc85cb9a10c3132ec176a475f2007353d638
• :broken_heart: Build #1431 failed ef70c47f2e5a368670b5d53c1a0cae145f6c6822
• :green_heart: Build #1277 succeeded b6a5a049b79ce6d925448114661301aa55367817