feat: theme property for individual menu items (#2401)

packages/vaadin-context-menu/src/interfaces.d.ts

@@ -5,6 +5,7 @@ export interface ContextMenuItem {
   component?: string | HTMLElement;
   disabled?: boolean;
   checked?: boolean;
+  theme?: string | string[];
   children?: ContextMenuItem[];
 }
 

packages/vaadin-context-menu/src/vaadin-context-menu.d.ts

@@ -22,7 +22,7 @@ import { ContextMenuEventMap, ContextMenuRenderer } from './interfaces';
  *
  * ```javascript
  * contextMenu.items = [
- *   {text: 'Menu Item 1', children:
+ *   {text: 'Menu Item 1', theme: 'primary', children:
  *     [
  *       {text: 'Menu Item 1-1', checked: true},
  *       {text: 'Menu Item 1-2'}

packages/vaadin-context-menu/src/vaadin-context-menu.js

@@ -28,7 +28,7 @@ import './vaadin-context-menu-overlay.js';
  *
  * ```javascript
  * contextMenu.items = [
- *   {text: 'Menu Item 1', children:
+ *   {text: 'Menu Item 1', theme: 'primary', children:
  *     [
  *       {text: 'Menu Item 1-1', checked: true},
  *       {text: 'Menu Item 1-2'}

packages/vaadin-context-menu/src/vaadin-contextmenu-items-mixin.d.ts

@@ -47,7 +47,7 @@ interface ItemsMixin {
    *
    * ```javascript
    * contextMenu.items = [
-   *   {text: 'Menu Item 1', children:
+   *   {text: 'Menu Item 1', theme: 'primary', children:
    *     [
    *       {text: 'Menu Item 1-1', checked: true},
    *       {text: 'Menu Item 1-2'}

packages/vaadin-context-menu/src/vaadin-contextmenu-items-mixin.js

@@ -50,6 +50,7 @@ export const ItemsMixin = (superClass) =>
          * Either a tagName or an element instance. Defaults to "vaadin-context-menu-item".
          * @property {boolean} disabled - If true, the item is disabled and cannot be selected
          * @property {boolean} checked - If true, the item shows a checkmark next to it
+         * @property {union: string | string[]} theme - If set, sets the given theme(s) as an attribute to the menu item component, overriding any theme set on the context menu.
          * @property {MenuItem[]} children - Array of child menu items
          */
 
@@ -64,7 +65,7 @@ export const ItemsMixin = (superClass) =>
          *
          * ```javascript
          * contextMenu.items = [
-         *   {text: 'Menu Item 1', children:
+         *   {text: 'Menu Item 1', theme: 'primary', children:
          *     [
          *       {text: 'Menu Item 1-1', checked: true},
          *       {text: 'Menu Item 1-2'}
@@ -230,7 +231,8 @@ export const ItemsMixin = (superClass) =>
         } else if (component.localName === 'hr') {
           component.setAttribute('role', 'separator');
         }
-        this.theme && component.setAttribute('theme', this.theme);
+
+        this._setMenuItemTheme(component, item, this.theme);
 
         component._item = item;
 
@@ -254,6 +256,22 @@ export const ItemsMixin = (superClass) =>
       });
     }
 
+    /** @protected */
+    _setMenuItemTheme(component, 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
+      if (item.theme != null) {
+        theme = Array.isArray(item.theme) ? item.theme.join(' ') : item.theme;
+      }
+
+      if (theme) {
+        component.setAttribute('theme', theme);
+      } else {
+        component.removeAttribute('theme');
+      }
+    }
+
     /** @private */
     __toggleMenuComponentAttribute(component, attribute, on) {
       if (on) {

packages/vaadin-context-menu/test/items.test.js

@@ -40,6 +40,19 @@ describe('items', () => {
     return menu.$.overlay.content.querySelector('vaadin-context-menu');
   };
 
+  const updateItemsAndReopen = async () => {
+    rootMenu.items = [...rootMenu.items];
+    rootMenu.close();
+    open();
+    await nextFrame();
+  };
+
+  const getMenuItems = (menu = rootMenu) => {
+    const overlay = menu.$.overlay;
+    const listBox = overlay.querySelector('vaadin-context-menu-list-box');
+    return Array.from(listBox.querySelectorAll('vaadin-context-menu-item'));
+  };
+
   afterEach(() => {
     rootMenu.close();
   });
@@ -492,7 +505,10 @@ describe('items', () => {
       rootMenu.openOn = 'mouseover';
       target = rootMenu.firstElementChild;
       rootMenu.items = [
-        { text: 'foo-0', children: [{ text: 'foo-0-0' }, { text: 'foo-0-1', children: [{ text: 'foo-0-1-0' }] }] },
+        {
+          text: 'foo-0',
+          children: [{ text: 'foo-0-0' }, { text: 'foo-0-1', children: [{ text: 'foo-0-1-0' }] }]
+        },
         { text: 'foo-1' }
       ];
       open();
@@ -548,5 +564,61 @@ describe('items', () => {
         expect(itemsDoNotHaveTheme).to.be.true;
       });
     });
+
+    it('should override the component theme with the item theme', async () => {
+      rootMenu.items[1].theme = 'bar-1';
+      rootMenu.items[0].children[0].theme = 'bar-0-0';
+      await updateItemsAndReopen();
+
+      open(menuComponents()[0]);
+      subMenu = getSubMenu();
+      await nextFrame();
+
+      const rootItems = getMenuItems();
+      const subItems = getMenuItems(subMenu);
+
+      expect(rootItems[0].getAttribute('theme')).to.equal('foo');
+      expect(rootItems[1].getAttribute('theme')).to.equal('bar-1');
+
+      expect(subItems[0].getAttribute('theme')).to.equal('bar-0-0');
+      expect(subItems[1].getAttribute('theme')).to.equal('foo');
+    });
+
+    it('should use the component theme if the item theme is removed', async () => {
+      rootMenu.items[1].theme = 'bar-1';
+      await updateItemsAndReopen();
+
+      let rootItems = getMenuItems();
+      expect(rootItems[1].getAttribute('theme')).to.equal('bar-1');
+
+      // An empty array should also override the component theme
+      rootMenu.items[1].theme = [];
+      await updateItemsAndReopen();
+
+      rootItems = getMenuItems();
+      expect(rootItems[1].hasAttribute('theme')).to.be.false;
+
+      // An empty string should also override the component theme
+      rootMenu.items[1].theme = '';
+      await updateItemsAndReopen();
+
+      rootItems = getMenuItems();
+      expect(rootItems[1].hasAttribute('theme')).to.be.false;
+
+      // If null or undefined, the parent component theme should be used
+      delete rootMenu.items[1].theme;
+      await updateItemsAndReopen();
+
+      rootItems = getMenuItems();
+      expect(rootItems[1].getAttribute('theme')).to.equal('foo');
+    });
+
+    it('should support multiple item themes in an array', async () => {
+      rootMenu.items[1].theme = ['bar-1', 'bar-2', 'bar-3'];
+      await updateItemsAndReopen();
+
+      let rootItems = getMenuItems();
+      expect(rootItems[1].getAttribute('theme')).to.equal('bar-1 bar-2 bar-3');
+    });
   });
 });

packages/vaadin-menu-bar/src/interfaces.d.ts

@@ -2,6 +2,7 @@ export interface MenuBarItem {
   text?: string;
   component?: string | HTMLElement;
   disabled?: boolean;
+  theme?: string | string[];
   children?: SubMenuItem[];
 }
 

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

@@ -249,17 +249,28 @@ export const ButtonsMixin = (superClass) =>
 
     /** @private */
     __applyTheme(theme) {
-      this._buttons.forEach((btn) => {
-        if (theme) {
-          btn.setAttribute('theme', theme);
-        } else {
-          btn.removeAttribute('theme');
-        }
-      });
+      this._buttons.forEach((btn) => this._setButtonTheme(btn, theme));
 
       this.__detectOverflow();
     }
 
+    /** @protected */
+    _setButtonTheme(btn, 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;
+      if (itemTheme != null) {
+        theme = Array.isArray(itemTheme) ? itemTheme.join(' ') : itemTheme;
+      }
+
+      if (theme) {
+        btn.setAttribute('theme', theme);
+      } else {
+        btn.removeAttribute('theme');
+      }
+    }
+
     /** @protected */
     _appendButton(button) {
       this._container.insertBefore(button, this._overflow);

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

@@ -57,10 +57,8 @@ export const InteractionsMixin = (superClass) =>
     /** @private */
     _themeChanged(theme) {
       if (theme) {
-        this._buttons.forEach((button) => button.setAttribute('theme', theme));
         this._subMenu.setAttribute('theme', theme);
       } else {
-        this._buttons.forEach((button) => button.removeAttribute('theme'));
         this._subMenu.removeAttribute('theme');
       }
     }

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

@@ -116,6 +116,7 @@ class MenuBarElement extends ButtonsMixin(InteractionsMixin(ElementMixin(Themabl
        * @property {union: string | object} component - The component to represent the button content.
        * Either a tagName or an element instance. Defaults to "vaadin-context-menu-item".
        * @property {boolean} disabled - If true, the button is disabled and cannot be activated.
+       * @property {union: string | string[]} theme - Theme(s) to be set as the theme attribute of the button, overriding any theme set on the menu bar.
        * @property {SubMenuItem[]} children - Array of submenu items.
        */
 

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

@@ -49,6 +49,11 @@ describe('custom element definition', () => {
 describe('root menu layout', () => {
   let menu, buttons;
 
+  function updateItemsAndButtons() {
+    menu.items = [...menu.items];
+    buttons = menu._buttons;
+  }
+
   beforeEach(async () => {
     menu = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>');
     menu.items = [{ text: 'Item 1' }, { text: 'Item 2' }, { text: 'Item 3', disabled: true }, { text: 'Item 4' }];
@@ -135,8 +140,7 @@ describe('root menu layout', () => {
 
       it('should move focus to second button if first is disabled on "home" keydown', () => {
         menu.items[0].disabled = true;
-        menu.items = [...menu.items];
-        buttons = menu._buttons;
+        updateItemsAndButtons();
         buttons[3].focus();
         const spy = sinon.spy(buttons[1], 'focus');
         home(buttons[3]);
@@ -154,8 +158,7 @@ describe('root menu layout', () => {
 
       it('should move focus to the closest enabled button if last is disabled on "end" keydown', () => {
         menu.items[3].disabled = true;
-        menu.items = [...menu.items];
-        buttons = menu._buttons;
+        updateItemsAndButtons();
         buttons[0].focus();
         const spy = sinon.spy(buttons[1], 'focus');
         end(buttons[0]);
@@ -249,6 +252,55 @@ describe('root menu layout', () => {
       menu.removeAttribute('theme');
       buttons.forEach((btn) => expect(btn.hasAttribute('theme')).to.be.false);
     });
+
+    it('should override the theme attribute of the component with the item.theme property', () => {
+      menu.setAttribute('theme', 'contained');
+      menu.items[1].theme = 'item-theme';
+      updateItemsAndButtons();
+
+      expect(buttons[0].getAttribute('theme')).to.equal('contained');
+      expect(buttons[1].getAttribute('theme')).to.equal('item-theme');
+
+      menu.removeAttribute('theme');
+
+      expect(buttons[0].hasAttribute('theme')).to.be.false;
+      expect(buttons[1].getAttribute('theme')).to.equal('item-theme');
+    });
+
+    it('should support setting multiple themes with an array', () => {
+      menu.items[1].theme = ['theme-1', 'theme-2'];
+      updateItemsAndButtons();
+
+      expect(buttons[1].getAttribute('theme')).to.equal('theme-1 theme-2');
+
+      menu.items[1].theme = [];
+      updateItemsAndButtons();
+
+      expect(buttons[1].hasAttribute('theme')).to.be.false;
+    });
+
+    it('should override the theme attribute of the component with an empty item.theme property', () => {
+      menu.setAttribute('theme', 'contained');
+      menu.items[0].theme = '';
+      updateItemsAndButtons();
+
+      expect(buttons[0].hasAttribute('theme')).to.be.false;
+
+      menu.items[0].theme = [];
+      updateItemsAndButtons();
+
+      expect(buttons[0].hasAttribute('theme')).to.be.false;
+
+      menu.items[0].theme = [''];
+      updateItemsAndButtons();
+
+      expect(buttons[0].hasAttribute('theme')).to.be.false;
+
+      menu.items[0].theme = null;
+      updateItemsAndButtons();
+
+      expect(buttons[0].getAttribute('theme')).to.equal('contained');
+    });
   });
 });
 

packages/vaadin-menu-bar/test/sub-menu.test.js

@@ -585,7 +585,7 @@ describe('theme attribute', () => {
       {
         text: 'Menu Item 1',
         children: [
-          { text: 'Menu Item 1 1' },
+          { text: 'Menu Item 1 1', theme: 'sub-item-theme' },
           {
             text: 'Menu Item 1 2'
           }
@@ -618,6 +618,23 @@ describe('theme attribute', () => {
     menu.removeAttribute('theme');
     expect(subMenu.hasAttribute('theme')).to.be.false;
   });
+
+  it('should override the component theme attribute with the item.theme property', async () => {
+    let items = subMenu.$.overlay.querySelectorAll('vaadin-context-menu-item');
+
+    expect(items[0].getAttribute('theme')).to.equal('sub-item-theme');
+    expect(items[1].getAttribute('theme')).to.equal('foo');
+
+    subMenu.close();
+    menu.removeAttribute('theme');
+    buttons[0].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
+    await nextRender(subMenu);
+
+    items = subMenu.$.overlay.querySelectorAll('vaadin-context-menu-item');
+
+    expect(items[0].getAttribute('theme')).to.equal('sub-item-theme');
+    expect(items[1].hasAttribute('theme')).to.be.false;
+  });
 });
 
 describe('touch', () => {