Skip to content

Commit

Permalink
fix: postpone menu-bar initial render to prevent re-layout (#7312) (#…
Browse files Browse the repository at this point in the history
…7320)

Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
  • Loading branch information
vaadin-bot and web-padawan authored Apr 11, 2024
1 parent daa7981 commit 0ed023e
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 10 deletions.
16 changes: 14 additions & 2 deletions integration/tests/component-relayout-page.test.js
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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');
Expand Down
11 changes: 10 additions & 1 deletion packages/menu-bar/src/vaadin-menu-bar-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}

/**
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion packages/menu-bar/test/a11y.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('a11y', () => {
describe('focus restoration', () => {
let menuBar, overlay, buttons;

beforeEach(() => {
beforeEach(async () => {
menuBar = fixtureSync(`<vaadin-menu-bar></vaadin-menu-bar>`);
menuBar.items = [
{
Expand All @@ -21,6 +21,7 @@ describe('a11y', () => {
],
},
];
await nextRender();
overlay = menuBar._subMenu._overlayElement;
buttons = menuBar.querySelectorAll('vaadin-menu-bar-button');
buttons[0].focus();
Expand Down Expand Up @@ -61,6 +62,7 @@ describe('a11y', () => {
await nextRender();
// Select Item 0/0
enter(getDeepActiveElement());
await nextRender();
expect(getDeepActiveElement()).to.equal(buttons[0]);
});

Expand All @@ -76,6 +78,7 @@ describe('a11y', () => {
await nextRender();
// Select Item 0/1/0
enter(getDeepActiveElement());
await nextRender();
expect(getDeepActiveElement()).to.equal(buttons[0]);
});
});
Expand Down
12 changes: 9 additions & 3 deletions packages/menu-bar/test/visual/lumo/menu-bar.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ describe('menu-bar', () => {
});

describe('basic', () => {
beforeEach(() => {
beforeEach(async () => {
div = document.createElement('div');
div.style.padding = '10px';

element = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>', div);
await nextRender();

element.items = [
{ text: 'Home' },
{
Expand All @@ -47,11 +49,13 @@ describe('menu-bar', () => {
});

describe('single button', () => {
beforeEach(() => {
beforeEach(async () => {
div = document.createElement('div');
div.style.padding = '10px';

element = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>', div);
await nextRender();

element.items = [{ text: 'Actions' }];
});

Expand All @@ -77,11 +81,13 @@ describe('menu-bar', () => {
return item;
}

beforeEach(() => {
beforeEach(async () => {
div = document.createElement('div');
div.style.padding = '10px';

element = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>', div);
await nextRender();

element.items = [
{ component: 'u', text: 'Home' },
{
Expand Down
12 changes: 9 additions & 3 deletions packages/menu-bar/test/visual/material/menu-bar.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ describe('menu-bar', () => {
});

describe('basic', () => {
beforeEach(() => {
beforeEach(async () => {
div = document.createElement('div');
div.style.padding = '10px';

element = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>', div);
await nextRender();

element.items = [
{ text: 'Home' },
{
Expand All @@ -47,11 +49,13 @@ describe('menu-bar', () => {
});

describe('single button', () => {
beforeEach(() => {
beforeEach(async () => {
div = document.createElement('div');
div.style.padding = '10px';

element = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>', div);
await nextRender();

element.items = [{ text: 'Actions' }];
element.setAttribute('theme', 'outlined');
});
Expand All @@ -78,11 +82,13 @@ describe('menu-bar', () => {
return item;
}

beforeEach(() => {
beforeEach(async () => {
div = document.createElement('div');
div.style.padding = '10px';

element = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>', div);
await nextRender();

element.items = [
{ component: 'u', text: 'Home' },
{
Expand Down

0 comments on commit 0ed023e

Please sign in to comment.