fix!: debounce updating overflow to improve performance (#9618) (CP: 24.7)

packages/menu-bar/src/vaadin-menu-bar-mixin.js

@@ -7,7 +7,9 @@ import { DisabledMixin } from '@vaadin/a11y-base/src/disabled-mixin.js';
 import { FocusMixin } from '@vaadin/a11y-base/src/focus-mixin.js';
 import { isElementFocused, isElementHidden, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
 import { KeyboardDirectionMixin } from '@vaadin/a11y-base/src/keyboard-direction-mixin.js';
+import { microTask } from '@vaadin/component-base/src/async.js';
 import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
+import { Debouncer } from '@vaadin/component-base/src/debounce.js';
 import { ResizeMixin } from '@vaadin/component-base/src/resize-mixin.js';
 import { SlotController } from '@vaadin/component-base/src/slot-controller.js';
 
@@ -295,13 +297,7 @@ export const MenuBarMixin = (superClass) =>
       const container = this.shadowRoot.querySelector('[part="container"]');
       container.addEventListener('click', this.__onButtonClick.bind(this));
       container.addEventListener('mouseover', (e) => this._onMouseOver(e));
-
-      // Delay setting container to avoid rendering buttons immediately,
-      // which would also trigger detecting overflow and force re-layout
-      // See https://github.com/vaadin/web-components/issues/7271
-      queueMicrotask(() => {
-        this._container = container;
-      });
+      this._container = container;
     }
 
     /**
@@ -330,7 +326,7 @@ export const MenuBarMixin = (superClass) =>
      * @override
      */
     _onResize() {
-      this.__detectOverflow();
+      this.__scheduleOverflow();
     }
 
     /**
@@ -360,7 +356,7 @@ export const MenuBarMixin = (superClass) =>
     _themeChanged(theme, overflow, container) {
       if (overflow && container) {
         this._buttons.forEach((btn) => this._setButtonTheme(btn, theme));
-        this.__detectOverflow();
+        this.__scheduleOverflow();
       }
 
       if (theme) {
@@ -378,7 +374,7 @@ export const MenuBarMixin = (superClass) =>
      */
     _reverseCollapseChanged(_reverseCollapse, overflow, container) {
       if (overflow && container) {
-        this.__detectOverflow();
+        this.__scheduleOverflow();
       }
     }
 
@@ -523,11 +519,14 @@ export const MenuBarMixin = (superClass) =>
     }
 
     /** @private */
-    __detectOverflow() {
-      if (!this._container) {
-        return;
-      }
+    __scheduleOverflow() {
+      this._overflowDebouncer = Debouncer.debounce(this._overflowDebouncer, microTask, () => {
+        this.__detectOverflow();
+      });
+    }
 
+    /** @private */
+    __detectOverflow() {
       const overflow = this._overflow;
       const buttons = this._buttons.filter((btn) => btn !== overflow);
       const oldOverflowCount = this.__getOverflowCount(overflow);
@@ -653,7 +652,7 @@ export const MenuBarMixin = (superClass) =>
         this._setButtonTheme(button, this._theme);
       });
 
-      this.__detectOverflow();
+      this.__scheduleOverflow();
     }
 
     /**

packages/menu-bar/test/dom/__snapshots__/menu-bar.test.snap.js

@@ -15,7 +15,7 @@ snapshots["menu-bar basic"] =
     aria-expanded="false"
     aria-haspopup="true"
     role="menuitem"
-    tabindex="-1"
+    tabindex="0"
   >
     Reports
   </vaadin-menu-bar-button>
@@ -31,7 +31,7 @@ snapshots["menu-bar basic"] =
     class="help"
     last-visible=""
     role="menuitem"
-    tabindex="-1"
+    tabindex="0"
   >
     <vaadin-menu-bar-item aria-selected="false">
       <strong>
@@ -46,7 +46,7 @@ snapshots["menu-bar basic"] =
     hidden=""
     role="menuitem"
     slot="overflow"
-    tabindex="-1"
+    tabindex="0"
   >
     <div aria-hidden="true">
       ···

packages/menu-bar/test/dom/menu-bar.test.js

@@ -1,5 +1,6 @@
 import { expect } from '@vaadin/chai-plugins';
-import { fixtureSync, nextRender } from '@vaadin/testing-helpers';
+import { fixtureSync, nextRender, nextResize } from '@vaadin/testing-helpers';
+import '../not-animated-styles.js';
 import '../../src/vaadin-menu-bar.js';
 import '../not-animated-styles.js';
 
@@ -32,7 +33,7 @@ describe('menu-bar', () => {
         className: 'help',
       },
     ];
-    await nextRender();
+    await nextResize(menu);
   });
 
   it('basic', async () => {

packages/menu-bar/test/overflow.test.js

@@ -1,5 +1,6 @@
 import { expect } from '@vaadin/chai-plugins';
-import { arrowRight, fixtureSync, nextFrame, nextRender, nextResize, nextUpdate } from '@vaadin/testing-helpers';
+import { arrowRight, fixtureSync, nextRender, nextResize, nextUpdate } from '@vaadin/testing-helpers';
+import sinon from 'sinon';
 import './menu-bar-custom-styles.js';
 import './not-animated-styles.js';
 import '../vaadin-menu-bar.js';
@@ -30,15 +31,14 @@ describe('overflow', () => {
         </div>
       `);
       menu = wrapper.querySelector('vaadin-menu-bar');
-      await nextRender(menu);
       menu.items = [
         { text: 'Item 1' },
         { text: 'Item 2' },
         { text: 'Item 3' },
         { text: 'Item 4' },
         { text: 'Item 5', disabled: true },
       ];
-      await nextUpdate(menu);
+      await nextResize(menu);
       buttons = menu._buttons;
       overflow = buttons[buttons.length - 1];
     });
@@ -130,7 +130,7 @@ describe('overflow', () => {
     it('should hide overflow button and reset its items when all buttons fit after changing items', async () => {
       // See https://github.com/vaadin/vaadin-menu-bar/issues/133
       menu.items = [{ text: 'Item 1' }, { text: 'Item 2' }];
-      await nextRender(menu);
+      await nextResize(menu);
       buttons = menu._buttons;
       overflow = buttons[2];
       assertVisible(buttons[1]);
@@ -144,7 +144,7 @@ describe('overflow', () => {
       await nextResize(menu);
       expect(overflow.hasAttribute('hidden')).to.be.true;
       menu.setAttribute('theme', 'big');
-      await nextUpdate(menu);
+      await nextResize(menu);
       assertHidden(buttons[3]);
       assertHidden(buttons[4]);
       expect(overflow.hasAttribute('hidden')).to.be.false;
@@ -262,7 +262,7 @@ describe('overflow', () => {
     beforeEach(async () => {
       menu = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>');
       menu.items = [{ text: 'Item 1' }, { text: 'Item 2' }, { text: 'Item 3' }, { text: 'Item 4' }];
-      await nextRender(menu);
+      await nextResize(menu);
       buttons = menu._buttons;
       overflow = buttons[buttons.length - 1];
     });
@@ -298,13 +298,13 @@ describe('overflow', () => {
       await nextResize(menu);
 
       menu.setAttribute('theme', 'big');
-      await nextUpdate(menu);
+      await nextResize(menu);
       expect(menu.hasAttribute('has-single-button')).to.be.true;
     });
 
     it('should set when changing items to only have one button', async () => {
       menu.items = [{ text: 'Actions' }];
-      await nextUpdate(menu);
+      await nextResize(menu);
       expect(menu.hasAttribute('has-single-button')).to.be.true;
     });
 
@@ -313,16 +313,16 @@ describe('overflow', () => {
       await nextResize(menu);
 
       menu.items = [{ text: 'Actions' }];
-      await nextUpdate(menu);
+      await nextResize(menu);
       expect(menu.hasAttribute('has-single-button')).to.be.true;
     });
 
     it('should remove when changing items to have more than one button', async () => {
       menu.items = [{ text: 'Actions' }];
-      await nextFrame();
+      await nextResize(menu);
 
       menu.items = [{ text: 'Edit' }, { text: 'Delete' }];
-      await nextUpdate(menu);
+      await nextResize(menu);
       expect(menu.hasAttribute('has-single-button')).to.be.false;
     });
   });
@@ -538,4 +538,20 @@ describe('overflow', () => {
       expect(item.classList.contains('test-class-1')).to.be.true;
     });
   });
+
+  describe('performance', () => {
+    let menu, spy;
+
+    beforeEach(() => {
+      menu = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>');
+      spy = sinon.spy(menu, '_hasOverflow', ['get', 'set']);
+    });
+
+    it('should only detect overflow twice on initial render', async () => {
+      menu.items = [{ text: 'Item 1' }, { text: 'Item 2' }, { text: 'Item 3' }, { text: 'Item 4' }, { text: 'Item 5' }];
+      await nextResize(menu);
+      await nextUpdate(menu);
+      expect(spy.set.callCount).to.equal(2);
+    });
+  });
 });