diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.js b/packages/menu-bar/src/vaadin-menu-bar-mixin.js index 7d385cd218f..52dfccec926 100644 --- a/packages/menu-bar/src/vaadin-menu-bar-mixin.js +++ b/packages/menu-bar/src/vaadin-menu-bar-mixin.js @@ -3,6 +3,9 @@ * Copyright (c) 2019 - 2025 Vaadin Ltd. * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ */ +import { html, noChange, nothing, render } from 'lit'; +import { Directive, directive } from 'lit/directive.js'; +import { ifDefined } from 'lit/directives/if-defined.js'; import { DisabledMixin } from '@vaadin/a11y-base/src/disabled-mixin.js'; import { FocusMixin } from '@vaadin/a11y-base/src/focus-mixin.js'; import { isElementFocused, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js'; @@ -12,6 +15,41 @@ import { I18nMixin } from '@vaadin/component-base/src/i18n-mixin.js'; import { ResizeMixin } from '@vaadin/component-base/src/resize-mixin.js'; import { SlotController } from '@vaadin/component-base/src/slot-controller.js'; +/** + * Custom Lit directive for rendering item components + * inspired by the `flowComponentDirective` logic. + */ +class ItemComponentDirective extends Directive { + update(part, [{ component, text }]) { + const { parentNode, startNode } = part; + + const newNode = component || (text ? document.createTextNode(text) : null); + const oldNode = this.getOldNode(part); + + if (oldNode === newNode) { + return noChange; + } else if (oldNode && newNode) { + parentNode.replaceChild(newNode, oldNode); + } else if (oldNode) { + parentNode.removeChild(oldNode); + } else if (newNode) { + startNode.after(newNode); + } + + return noChange; + } + + getOldNode(part) { + const { startNode, endNode } = part; + if (startNode.nextSibling === endNode) { + return null; + } + return startNode.nextSibling; + } +} + +const componentDirective = directive(ItemComponentDirective); + const DEFAULT_I18N = { moreOptions: 'More options', }; @@ -284,8 +322,11 @@ export const MenuBarMixin = (superClass) => dots.innerHTML = '·'.repeat(3); btn.appendChild(dots); + btn.setAttribute('aria-haspopup', 'true'); + btn.setAttribute('aria-expanded', 'false'); + btn.setAttribute('role', this.tabNavigation ? 'button' : 'menuitem'); + this._overflow = btn; - this._initButtonAttrs(btn); }, }); this.addController(this._overflowController); @@ -366,14 +407,16 @@ export const MenuBarMixin = (superClass) => */ _themeChanged(theme, overflow, container) { if (overflow && container) { - this._buttons.forEach((btn) => this._setButtonTheme(btn, theme)); + this.__renderButtons(this.items); this.__detectOverflow(); - } - if (theme) { - this._subMenu.setAttribute('theme', theme); - } else { - this._subMenu.removeAttribute('theme'); + if (theme) { + overflow.setAttribute('theme', theme); + this._subMenu.setAttribute('theme', theme); + } else { + overflow.removeAttribute('theme'); + this._subMenu.removeAttribute('theme'); + } } } @@ -421,11 +464,18 @@ export const MenuBarMixin = (superClass) => if (items !== this._oldItems) { this._oldItems = items; this.__renderButtons(items); + this.__detectOverflow(); } const subMenu = this._subMenu; if (subMenu && subMenu.opened) { - subMenu.close(); + const button = subMenu._overlayElement.positionTarget; + + // Close sub-menu if the corresponding button is no longer in the DOM, + // or if the item on it has been changed to no longer have children. + if (!button.isConnected || !Array.isArray(button.item.children) || button.item.children.length === 0) { + subMenu.close(); + } } } @@ -561,66 +611,17 @@ export const MenuBarMixin = (superClass) => }); } - /** @protected */ - _removeButtons() { - this._buttons.forEach((button) => { - if (button !== this._overflow) { - this.removeChild(button); - } - }); - } - - /** @protected */ - _initButton(item) { - const button = document.createElement('vaadin-menu-bar-button'); - - const itemCopy = { ...item }; - button.item = itemCopy; - - if (item.component) { - const component = this.__getComponent(itemCopy); - itemCopy.component = component; - // Save item for overflow menu - component.item = itemCopy; - button.appendChild(component); - } else if (item.text) { - button.textContent = item.text; - } - - if (item.className) { - button.className = item.className; - } - - button.disabled = item.disabled; - - return button; - } - - /** @protected */ - _initButtonAttrs(button) { - button.setAttribute('role', this.tabNavigation ? 'button' : 'menuitem'); - - if (button === this._overflow || (button.item && button.item.children)) { - button.setAttribute('aria-haspopup', 'true'); - button.setAttribute('aria-expanded', 'false'); - } - } - - /** @protected */ - _setButtonTheme(btn, hostTheme) { + /** @private */ + __getButtonTheme(item, hostTheme) { let theme = hostTheme; // Item theme takes precedence over host theme even if it's empty, as long as it's not undefined or null - const itemTheme = btn.item && btn.item.theme; + const itemTheme = item && item.theme; if (itemTheme != null) { theme = Array.isArray(itemTheme) ? itemTheme.join(' ') : itemTheme; } - if (theme) { - btn.setAttribute('theme', theme); - } else { - btn.removeAttribute('theme'); - } + return theme; } /** @private */ @@ -645,21 +646,47 @@ export const MenuBarMixin = (superClass) => /** @private */ __renderButtons(items = []) { - this._removeButtons(); - - /* Empty array, do nothing */ - if (items.length === 0) { - return; - } + render( + html` + ${items.map((item) => { + const itemCopy = { ...item }; + const hasChildren = Boolean(item && item.children); + + if (itemCopy.component) { + const component = this.__getComponent(itemCopy); + itemCopy.component = component; + component.item = itemCopy; + } - items.forEach((item) => { - const button = this._initButton(item); - this.insertBefore(button, this._overflow); - this._initButtonAttrs(button); - this._setButtonTheme(button, this._theme); - }); + return html` + ${componentDirective(itemCopy)} + `; + })} + `, + this, + { renderBefore: this._overflow }, + ); + } - this.__detectOverflow(); + /** @private */ + __onRootButtonClick(event) { + const button = event.target; + // Propagate click event from button to the item component if it was outside + // it e.g. by calling `click()` on the button (used by the Flow counterpart). + if (button.item && button.item.component && !event.composedPath().includes(button.item.component)) { + event.stopPropagation(); + button.item.component.click(); + } } /** diff --git a/packages/menu-bar/test/item-components.test.js b/packages/menu-bar/test/item-components.test.js new file mode 100644 index 00000000000..1ec8b2cccc5 --- /dev/null +++ b/packages/menu-bar/test/item-components.test.js @@ -0,0 +1,210 @@ +import { expect } from '@vaadin/chai-plugins'; +import { fixtureSync, nextRender } from '@vaadin/testing-helpers'; +import sinon from 'sinon'; +import '../src/vaadin-menu-bar.js'; + +describe('item components', () => { + describe('basic', () => { + let menu, buttons; + + function makeComponent(id) { + const div = document.createElement('div'); + div.style.width = '100px'; + div.textContent = `Item ${id}`; + return div; + } + + beforeEach(async () => { + menu = fixtureSync(''); + menu.items = [ + { component: makeComponent('1') }, + { text: 'Item 2 text', component: makeComponent('2') }, + { text: 'Item 3', component: document.createElement('vaadin-menu-bar-item') }, + { text: 'Item 4' }, + ]; + await nextRender(menu); + buttons = menu._buttons; + }); + + it('should wrap the provided item component with the menu-bar item', () => { + const item = buttons[0].firstElementChild; + expect(item).to.equal(buttons[0].item.component); + expect(item.localName).to.equal('vaadin-menu-bar-item'); + const div = item.firstChild; + expect(div).to.equal(menu.items[0].component); + expect(div.localName).to.equal('div'); + expect(div.textContent).to.equal('Item 1'); + expect(getComputedStyle(div).width).to.equal('100px'); + }); + + it('should override the component text when defined on the item', () => { + const item = buttons[1].firstElementChild; + expect(item).to.equal(buttons[1].item.component); + expect(item.localName).to.equal('vaadin-menu-bar-item'); + const div = item.firstElementChild; + expect(div).to.equal(menu.items[1].component); + expect(div.localName).to.equal('div'); + expect(div.textContent).to.equal('Item 2 text'); + expect(getComputedStyle(div).width).to.equal('100px'); + }); + + it('should render provided menu-bar item as a component', () => { + expect(buttons[2].firstElementChild).to.equal(buttons[2].item.component); + expect(buttons[2].item.component).to.equal(menu.items[2].component); + expect(buttons[2].item.component.textContent).to.equal('Item 3'); + }); + + it('should update buttons when replacing item component with text', async () => { + menu.items = [{ text: 'Item 0' }, ...menu.items.slice(1)]; + await nextRender(menu); + buttons = menu._buttons; + expect(buttons[0].textContent).to.equal('Item 0'); + }); + + it('should update buttons when replacing item text with component', async () => { + menu.items = [...menu.items.slice(0, 3), { component: makeComponent('4') }]; + await nextRender(menu); + buttons = menu._buttons; + expect(buttons[3].firstElementChild).to.equal(buttons[3].item.component); + expect(buttons[3].firstElementChild.textContent).to.equal('Item 4'); + }); + + it('should update buttons when replacing item component with empty object', async () => { + menu.items = [{}, ...menu.items.slice(1)]; + await nextRender(menu); + buttons = menu._buttons; + expect(buttons[0].textContent).to.equal(''); + }); + + it('should update buttons when replacing item text with empty object', async () => { + menu.items = [...menu.items.slice(0, 3), {}]; + await nextRender(menu); + buttons = menu._buttons; + expect(buttons[3].textContent).to.equal(''); + }); + + it('should propagate click on the button to the item component', () => { + const item = buttons[2].item.component; + const spy = sinon.spy(item, 'click'); + buttons[2].click(); + expect(spy).to.be.calledOnce; + }); + }); + + describe('item rendering', () => { + let div, menu, items; + + const components = new Map(); + + function makeItem(value) { + const item = document.createElement('vaadin-menu-bar-item'); + item.textContent = `Item ${value}`; + return item; + } + + async function updateItems() { + items.forEach(({ value, className }) => { + if (!components.has(value)) { + const component = makeItem(value); + components.set(value, component); + } + + // Mimic the Flow component behavior where components are moved + // to the virtual div when generating menu items, which causes + // DOM nodes for existing items to disappear if passed to Lit + // template as is (so we have to use a custom Lit directive). + const component = components.get(value); + div.appendChild(component); + + // Mimic the Flow connector logic for className on item components + component.className = className || ''; + }); + + menu.items = [...div.children].map((child) => { + const item = { + component: child, + className: child.className, + }; + + child._item = item; + return item; + }); + + await nextRender(); + } + + function expectItemsRendered() { + const buttons = menu._buttons; + + items.forEach(({ value, className }, idx) => { + const button = buttons[idx]; + const item = button.firstElementChild; + expect(item).to.equal(button.item.component); + expect(item.localName).to.equal('vaadin-menu-bar-item'); + expect(item.textContent).to.equal(`Item ${value}`); + + if (className) { + expect(item.className).to.equal(className); + expect(button.className).to.equal(className); + } else { + // Flow component ITs check for attribute presence + expect(button.hasAttribute('class')).to.be.false; + } + }); + } + + beforeEach(async () => { + div = document.createElement('div'); + menu = fixtureSync(''); + await nextRender(); + + items = [{ value: 'foo' }, { value: 'bar' }, { value: 'baz' }]; + await updateItems(); + expectItemsRendered(); + }); + + it('should re-render components when adding new menu-bar items', async () => { + items.push({ value: 'qux' }); + await updateItems(); + expectItemsRendered(); + }); + + it('should re-render components when updating existing menu-bar items', async () => { + items = [{ value: 'abc' }, ...items.slice(1)]; + await updateItems(); + expectItemsRendered(); + }); + + it('should re-render components when removing existing menu-bar items', async () => { + items = [items[0], items[2]]; + await updateItems(); + expectItemsRendered(); + }); + + it('should re-render components when setting the same menu-bar items', async () => { + items = [...items]; + await updateItems(); + expectItemsRendered(); + }); + + it('should update button className when re-rendering item components', async () => { + items = [{ value: 'foo', className: 'foo' }]; + await updateItems(); + expectItemsRendered(); + + items = [{ value: 'foo', className: 'bar' }]; + await updateItems(); + expectItemsRendered(); + }); + + it('should remove button className when re-rendering item components', async () => { + items = [{ value: 'foo', className: 'foo' }]; + await updateItems(); + expectItemsRendered(); + + items = [{ value: 'foo' }]; + await updateItems(); + expectItemsRendered(); + }); + }); +}); diff --git a/packages/menu-bar/test/menu-bar.test.js b/packages/menu-bar/test/menu-bar.test.js index 11e4ca1e634..6c1ad20a71e 100644 --- a/packages/menu-bar/test/menu-bar.test.js +++ b/packages/menu-bar/test/menu-bar.test.js @@ -199,60 +199,6 @@ describe('root menu layout', () => { }); }); -describe('item components', () => { - let menu, buttons; - - function makeComponent(id) { - const div = document.createElement('div'); - div.style.width = '100px'; - div.textContent = `Item ${id}`; - return div; - } - - beforeEach(async () => { - menu = fixtureSync(''); - menu.items = [ - { text: 'Item 1' }, - { text: 'Item 2' }, - { component: makeComponent('3') }, - { text: 'Item 4 text', component: makeComponent('4') }, - { text: 'Item 5', component: document.createElement('vaadin-menu-bar-item') }, - { component: makeComponent('6'), children: [{ text: 'SubItem6.1' }, { text: 'SubItem6.2' }] }, - ]; - await nextRender(menu); - buttons = menu._buttons; - }); - - it('should render the component inside the menu-bar item', () => { - const item = buttons[2].firstChild; - expect(item).to.equal(buttons[2].item.component); - expect(item.localName).to.equal('vaadin-menu-bar-item'); - const div = item.firstChild; - expect(div).to.equal(menu.items[2].component); - expect(div.localName).to.equal('div'); - expect(div.textContent).to.equal('Item 3'); - expect(getComputedStyle(div).width).to.equal('100px'); - }); - - it('should override the component text when defined on the item', () => { - const item = buttons[3].firstChild; - expect(item).to.equal(buttons[3].item.component); - expect(item.localName).to.equal('vaadin-menu-bar-item'); - const div = item.firstChild; - expect(div).to.equal(menu.items[3].component); - expect(div.localName).to.equal('div'); - expect(div.textContent).to.equal('Item 4 text'); - expect(getComputedStyle(div).width).to.equal('100px'); - }); - - it('should render provided menu-bar item as a component', () => { - expect(buttons[4].firstChild).to.equal(buttons[4].item.component); - expect(buttons[4].item.component).to.equal(menu.items[4].component); - expect(buttons[4].item.component.children.length).to.equal(0); - expect(buttons[4].item.component.textContent).to.equal('Item 5'); - }); -}); - describe('menu-bar in flex', () => { let wrapper; let menu; diff --git a/packages/menu-bar/test/overflow.test.js b/packages/menu-bar/test/overflow.test.js index de87c344a9d..c027a3a3d65 100644 --- a/packages/menu-bar/test/overflow.test.js +++ b/packages/menu-bar/test/overflow.test.js @@ -491,13 +491,13 @@ describe('overflow', () => { await nextUpdate(menu); menu.style.width = 'auto'; await nextResize(menu); - const item = buttons[2].firstChild; + const item = buttons[2].firstElementChild; expect(item).to.equal(buttons[2].item.component); expect(item.getAttribute('role')).to.not.equal('menuitem'); }); it('should restore menu bar item attribute state when moved from sub-menu back to menu bar', async () => { - const item = buttons[5].firstChild; + const item = buttons[5].firstElementChild; const itemAttributes = item.getAttributeNames(); overflow.click(); await nextRender(subMenu); @@ -510,7 +510,7 @@ describe('overflow', () => { it('should keep the class names when moved to sub-menu and back', async () => { // Simulate a custom class name being added through the Flow menu bar item component - const item = buttons[5].firstChild; + const item = buttons[5].firstElementChild; item.classList.add('test-class-1'); overflow.click(); await nextRender(subMenu); diff --git a/packages/menu-bar/test/sub-menu.test.js b/packages/menu-bar/test/sub-menu.test.js index 7591c1f9eac..5387b46953f 100644 --- a/packages/menu-bar/test/sub-menu.test.js +++ b/packages/menu-bar/test/sub-menu.test.js @@ -407,12 +407,30 @@ describe('sub-menu', () => { }); }); - it('should close sub-menu on items change', async () => { + it('should not close sub-menu on items change if item has not changed', async () => { buttons[0].click(); await nextRender(subMenu); menu.items = [...menu.items, { text: 'Menu Item 1' }]; await nextRender(subMenu); + expect(subMenu.opened).to.be.true; + }); + + it('should close sub-menu on items change if item no longer has children', async () => { + buttons[0].click(); + await nextRender(subMenu); + + menu.items = [{ text: 'Menu Item 0' }, ...menu.items]; + await nextRender(subMenu); + expect(subMenu.opened).to.be.false; + }); + + it('should close sub-menu on items change if item has empty children', async () => { + buttons[0].click(); + await nextRender(subMenu); + + menu.items = [{ text: 'Menu Item 0', children: [] }, ...menu.items]; + await nextRender(subMenu); expect(subMenu.opened).to.be.false; });