From 0ed023ec467a25d742af7ecb7e5551a0e5bda9ba Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Thu, 11 Apr 2024 09:15:33 +0200 Subject: [PATCH] fix: postpone menu-bar initial render to prevent re-layout (#7312) (#7320) Co-authored-by: Serhii Kulykov --- .../tests/component-relayout-page.test.js | 16 ++++++++++++++-- packages/menu-bar/src/vaadin-menu-bar-mixin.js | 11 ++++++++++- packages/menu-bar/test/a11y.test.js | 5 ++++- .../menu-bar/test/visual/lumo/menu-bar.test.js | 12 +++++++++--- .../test/visual/material/menu-bar.test.js | 12 +++++++++--- 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/integration/tests/component-relayout-page.test.js b/integration/tests/component-relayout-page.test.js index 591e853adb..a7401df927 100644 --- a/integration/tests/component-relayout-page.test.js +++ b/integration/tests/component-relayout-page.test.js @@ -1,8 +1,17 @@ import { expect } from '@esm-bundle/chai'; import { fixtureSync, nextRender } from '@vaadin/testing-helpers'; import { ContextMenu } from '@vaadin/context-menu'; - -[{ tagName: ContextMenu.is }].forEach(({ tagName }) => { +import { MenuBar } from '@vaadin/menu-bar'; + +[ + { tagName: ContextMenu.is }, + { + tagName: MenuBar.is, + callback: (el) => { + el.items = [{ text: 'Item' }]; + }, + }, +].forEach(({ tagName, callback }) => { describe(`${tagName} re-layout`, () => { let wrapper; @@ -16,6 +25,9 @@ import { ContextMenu } from '@vaadin/context-menu'; } wrapper.appendChild(document.createElement(tagName)); + if (callback) { + callback(wrapper.firstElementChild); + } for (let i = 0; i < 100; i++) { const btn = document.createElement('button'); diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.js b/packages/menu-bar/src/vaadin-menu-bar-mixin.js index 1df0ab4d8a..64e1bbd30a 100644 --- a/packages/menu-bar/src/vaadin-menu-bar-mixin.js +++ b/packages/menu-bar/src/vaadin-menu-bar-mixin.js @@ -251,7 +251,12 @@ export const MenuBarMixin = (superClass) => container.addEventListener('click', this.__onButtonClick.bind(this)); container.addEventListener('mouseover', (e) => this._onMouseOver(e)); - this._container = container; + // 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; + }); } /** @@ -444,6 +449,10 @@ export const MenuBarMixin = (superClass) => /** @private */ __detectOverflow() { + if (!this._container) { + return; + } + const overflow = this._overflow; const buttons = this._buttons.filter((btn) => btn !== overflow); const oldOverflowCount = this.__getOverflowCount(overflow); diff --git a/packages/menu-bar/test/a11y.test.js b/packages/menu-bar/test/a11y.test.js index a6f4577ced..aee737836a 100644 --- a/packages/menu-bar/test/a11y.test.js +++ b/packages/menu-bar/test/a11y.test.js @@ -7,7 +7,7 @@ describe('a11y', () => { describe('focus restoration', () => { let menuBar, overlay, buttons; - beforeEach(() => { + beforeEach(async () => { menuBar = fixtureSync(``); menuBar.items = [ { @@ -21,6 +21,7 @@ describe('a11y', () => { ], }, ]; + await nextRender(); overlay = menuBar._subMenu._overlayElement; buttons = menuBar.querySelectorAll('vaadin-menu-bar-button'); buttons[0].focus(); @@ -61,6 +62,7 @@ describe('a11y', () => { await nextRender(); // Select Item 0/0 enter(getDeepActiveElement()); + await nextRender(); expect(getDeepActiveElement()).to.equal(buttons[0]); }); @@ -76,6 +78,7 @@ describe('a11y', () => { await nextRender(); // Select Item 0/1/0 enter(getDeepActiveElement()); + await nextRender(); expect(getDeepActiveElement()).to.equal(buttons[0]); }); }); diff --git a/packages/menu-bar/test/visual/lumo/menu-bar.test.js b/packages/menu-bar/test/visual/lumo/menu-bar.test.js index 384115fc40..ee354b58ab 100644 --- a/packages/menu-bar/test/visual/lumo/menu-bar.test.js +++ b/packages/menu-bar/test/visual/lumo/menu-bar.test.js @@ -19,11 +19,13 @@ describe('menu-bar', () => { }); describe('basic', () => { - beforeEach(() => { + beforeEach(async () => { div = document.createElement('div'); div.style.padding = '10px'; element = fixtureSync('', div); + await nextRender(); + element.items = [ { text: 'Home' }, { @@ -47,11 +49,13 @@ describe('menu-bar', () => { }); describe('single button', () => { - beforeEach(() => { + beforeEach(async () => { div = document.createElement('div'); div.style.padding = '10px'; element = fixtureSync('', div); + await nextRender(); + element.items = [{ text: 'Actions' }]; }); @@ -77,11 +81,13 @@ describe('menu-bar', () => { return item; } - beforeEach(() => { + beforeEach(async () => { div = document.createElement('div'); div.style.padding = '10px'; element = fixtureSync('', div); + await nextRender(); + element.items = [ { component: 'u', text: 'Home' }, { diff --git a/packages/menu-bar/test/visual/material/menu-bar.test.js b/packages/menu-bar/test/visual/material/menu-bar.test.js index 91d7fdddc0..4cbcedec7d 100644 --- a/packages/menu-bar/test/visual/material/menu-bar.test.js +++ b/packages/menu-bar/test/visual/material/menu-bar.test.js @@ -19,11 +19,13 @@ describe('menu-bar', () => { }); describe('basic', () => { - beforeEach(() => { + beforeEach(async () => { div = document.createElement('div'); div.style.padding = '10px'; element = fixtureSync('', div); + await nextRender(); + element.items = [ { text: 'Home' }, { @@ -47,11 +49,13 @@ describe('menu-bar', () => { }); describe('single button', () => { - beforeEach(() => { + beforeEach(async () => { div = document.createElement('div'); div.style.padding = '10px'; element = fixtureSync('', div); + await nextRender(); + element.items = [{ text: 'Actions' }]; element.setAttribute('theme', 'outlined'); }); @@ -78,11 +82,13 @@ describe('menu-bar', () => { return item; } - beforeEach(() => { + beforeEach(async () => { div = document.createElement('div'); div.style.padding = '10px'; element = fixtureSync('', div); + await nextRender(); + element.items = [ { component: 'u', text: 'Home' }, {