fix!: debounce updating overflow to improve performance #9618
Conversation
web-padawan
force-pushed
the
menu-bar-overflow-debounce
branch
from
e7e185f to
2f65e55
Compare
web-padawan
force-pushed
the
menu-bar-overflow-debounce
branch
from
6cac3b3 to
e965037
Compare
web-padawan
changed the title
web-padawan
force-pushed
the
menu-bar-overflow-debounce
branch
from
08d102d to
ff2a92a
Compare
|
UPD: seems like deferring first call to |
|
Did some testing with the web component (creating 10 menu-bars on button click), here's the outcome. So there is some improvement as a result but I wouldn't call it really significant. Before
After
|
|
|
|
||
| beforeEach(() => { | ||
| menu = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>'); | ||
| spy = sinon.spy(menu, '_hasOverflow', ['get', 'set']); |
There was a problem hiding this comment.
I wasn't able to properly spy on offsetWidth of the container. Also using _hasOverflow is only reasonable for initial render test but not for subsequent resizes, as in those cases it will be set at least twice: first by calling __updateOverflow() with empty array, and then with the actual items. But for this test IMO it's ok.
|
Hi @web-padawan and @web-padawan, when i performed cherry-pick to this commit to 24.8, i have encountered the following issue. Can you take a look and pick it manually? |
|
This ticket/PR has been released with Vaadin 25.0.0-beta2. |
3 participants
Description
Fixes #9592
This reduces
__detectOverflow()counts on initial render from 5 per component to 2:Also reverted change from #7312 which delayed setting
_containeras no longer needed, since the idea behind it was to postpone initial call to__detectOverflow()and it's now scheduled in a different way, so the IT still passes.Note: needs some more testing with the Flow counterpart. ITs seem to pass with the latest version of the PR when running individual tests from IntelliJ, but I'm getting "script timeout error" from the
waitForResizeObserver()when running ITs for the component from the command line (could be related to the change but IIRC it happened before).Type of change
Note
This is a behavior altering chage as it affects timings, so I had to adjust e.g. visual tests and snapshot tests accordingly.
Could be especially relevant for Copilot which uses
vaadin-menu-barfor the dev mode menu element internally.Also this is likely to require some tweaks for both Flow component ITs and possibly also React components tests.