refactor!: update menu-bar to use vaadin-menu-bar-item (#5339)

Co-authored-by: web-padawan <[email protected]>
vaadin · Jan 24, 2023 · 3800700 · 3800700
1 parent 6e6f992
commit 3800700
Show file tree

Hide file tree

Showing 17 changed files with 157 additions and 107 deletions.
diff --git a/packages/context-menu/src/vaadin-contextmenu-items-mixin.js b/packages/context-menu/src/vaadin-contextmenu-items-mixin.js
@@ -181,7 +181,7 @@ export const ItemsMixin = (superClass) =>
         if (item.component instanceof HTMLElement) {
           component = item.component;
         } else {
-          component = document.createElement(item.component || 'vaadin-context-menu-item');
+          component = document.createElement(item.component || `${this._tagNamePrefix}-item`);
         }
 
         if (component instanceof Item) {
@@ -306,7 +306,7 @@ export const ItemsMixin = (superClass) =>
         });
         const openSubMenu = (
           e,
-          itemElement = e.composedPath().find((e) => e.localName === 'vaadin-context-menu-item'),
+          itemElement = e.composedPath().find((e) => e.localName === `${this._tagNamePrefix}-item`),
         ) => {
           // Delay enabling the mouseover listener to avoid it from triggering on parent menu open
           if (!this.__openListenerActive) {

diff --git a/packages/menu-bar/src/vaadin-menu-bar-button.js b/packages/menu-bar/src/vaadin-menu-bar-button.js
@@ -17,7 +17,7 @@ registerStyles(
       margin-inline-end: 0;
     }
 
-    [part='label'] ::slotted(vaadin-context-menu-item) {
+    [part='label'] ::slotted(vaadin-menu-bar-item) {
       position: relative;
       z-index: 1;
     }

diff --git a/packages/menu-bar/src/vaadin-menu-bar-item.js b/packages/menu-bar/src/vaadin-menu-bar-item.js
@@ -0,0 +1,55 @@
+/**
+ * @license
+ * Copyright (c) 2019 - 2023 Vaadin Ltd.
+ * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
+ */
+import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
+import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js';
+import { ItemMixin } from '@vaadin/item/src/vaadin-item-mixin.js';
+import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
+
+/**
+ * An element used internally by `<vaadin-menu-bar>`. Not intended to be used separately.
+ *
+ * @extends HTMLElement
+ * @mixes DirMixin
+ * @mixes ItemMixin
+ * @mixes ThemableMixin
+ * @protected
+ */
+class MenuBarItem extends ItemMixin(ThemableMixin(DirMixin(PolymerElement))) {
+  static get is() {
+    return 'vaadin-menu-bar-item';
+  }
+
+  static get template() {
+    return html`
+      <style>
+        :host {
+          display: inline-block;
+        }
+
+        :host([hidden]) {
+          display: none !important;
+        }
+      </style>
+      <span part="checkmark" aria-hidden="true"></span>
+      <div part="content">
+        <slot></slot>
+      </div>
+    `;
+  }
+
+  /** @protected */
+  connectedCallback() {
+    super.connectedCallback();
+
+    // Set role in `connectedCallback()` instead of `ready()`
+    // because the role is removed when teleporting to button.
+    this.setAttribute('role', 'menuitem');
+  }
+}
+
+customElements.define(MenuBarItem.is, MenuBarItem);
+
+export { MenuBarItem };
diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.js b/packages/menu-bar/src/vaadin-menu-bar-mixin.js
@@ -360,17 +360,16 @@ export const MenuBarMixin = (superClass) =>
 
       const isElement = itemComponent instanceof HTMLElement;
       // Use existing item component, if any
-      if (isElement && itemComponent.localName === 'vaadin-context-menu-item') {
+      if (isElement && itemComponent.localName === 'vaadin-menu-bar-item') {
         component = itemComponent;
       } else {
-        component = document.createElement('vaadin-context-menu-item');
+        component = document.createElement('vaadin-menu-bar-item');
         component.appendChild(isElement ? itemComponent : document.createElement(itemComponent));
       }
       if (item.text) {
         const node = component.firstChild || component;
         node.textContent = item.text;
       }
-      component.setAttribute('theme', 'menu-bar-item');
       return component;
     }
 

diff --git a/packages/menu-bar/src/vaadin-menu-bar-submenu.js b/packages/menu-bar/src/vaadin-menu-bar-submenu.js
@@ -3,6 +3,7 @@
  * Copyright (c) 2019 - 2023 Vaadin Ltd.
  * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
  */
+import './vaadin-menu-bar-item.js';
 import './vaadin-menu-bar-list-box.js';
 import './vaadin-menu-bar-overlay.js';
 import { html } from '@polymer/polymer/lib/utils/html-tag.js';

diff --git a/packages/menu-bar/src/vaadin-menu-bar.d.ts b/packages/menu-bar/src/vaadin-menu-bar.d.ts
@@ -20,7 +20,7 @@ export interface MenuBarItem {
   tooltip?: string;
   /**
    * The component to represent the button content.
-   * Either a tagName or an element instance. Defaults to "vaadin-context-menu-item".
+   * Either a tagName or an element instance. Defaults to "vaadin-menu-bar-item".
    */
   component?: HTMLElement | string;
   /**
@@ -101,7 +101,7 @@ export interface MenuBarEventMap extends HTMLElementEventMap, MenuBarCustomEvent
  * components are themable:
  *
  * - `<vaadin-menu-bar-button>` - has the same API as [`<vaadin-button>`](#/elements/vaadin-button).
- * - `<vaadin-context-menu-item>` - has the same API as [`<vaadin-item>`](#/elements/vaadin-item).
+ * - `<vaadin-menu-bar-item>` - has the same API as [`<vaadin-item>`](#/elements/vaadin-item).
  * - `<vaadin-menu-bar-list-box>` - has the same API as [`<vaadin-list-box>`](#/elements/vaadin-list-box).
  * - `<vaadin-menu-bar-overlay>` - has the same API as [`<vaadin-overlay>`](#/elements/vaadin-overlay).
  *

diff --git a/packages/menu-bar/src/vaadin-menu-bar.js b/packages/menu-bar/src/vaadin-menu-bar.js
@@ -52,7 +52,7 @@ import { MenuBarMixin } from './vaadin-menu-bar-mixin.js';
  * components are themable:
  *
  * - `<vaadin-menu-bar-button>` - has the same API as [`<vaadin-button>`](#/elements/vaadin-button).
- * - `<vaadin-context-menu-item>` - has the same API as [`<vaadin-item>`](#/elements/vaadin-item).
+ * - `<vaadin-menu-bar-item>` - has the same API as [`<vaadin-item>`](#/elements/vaadin-item).
  * - `<vaadin-menu-bar-list-box>` - has the same API as [`<vaadin-list-box>`](#/elements/vaadin-list-box).
  * - `<vaadin-menu-bar-overlay>` - has the same API as [`<vaadin-overlay>`](#/elements/vaadin-overlay).
  *
@@ -108,7 +108,7 @@ class MenuBar extends MenuBarMixin(DisabledMixin(ElementMixin(ThemableMixin(Poly
        * @property {string} tooltip - Text to be set as the menu button's tooltip.
        * Requires a `<vaadin-tooltip slot="tooltip">` element to be added inside the `<vaadin-menu-bar>`.
        * @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".
+       * Either a tagName or an element instance. Defaults to "vaadin-menu-bar-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.
@@ -119,7 +119,7 @@ class MenuBar extends MenuBarMixin(DisabledMixin(ElementMixin(ThemableMixin(Poly
        * @type {object}
        * @property {string} text - Text to be set as the menu item component's textContent.
        * @property {union: string | object} component - The component to represent the item.
-       * Either a tagName or an element instance. Defaults to "vaadin-context-menu-item".
+       * Either a tagName or an element instance. Defaults to "vaadin-menu-bar-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 {SubMenuItem[]} children - Array of child submenu items.

diff --git a/packages/menu-bar/test/dom/__snapshots__/menu-bar.test.snap.js b/packages/menu-bar/test/dom/__snapshots__/menu-bar.test.snap.js
@@ -29,14 +29,11 @@ snapshots["menu-bar basic"] =
     role="menuitem"
     tabindex="0"
   >
-    <vaadin-context-menu-item
-      aria-selected="false"
-      theme="menu-bar-item"
-    >
+    <vaadin-menu-bar-item aria-selected="false">
       <strong>
         Help
       </strong>
-    </vaadin-context-menu-item>
+    </vaadin-menu-bar-item>
   </vaadin-menu-bar-button>
   <vaadin-menu-bar-button
     aria-expanded="false"
@@ -68,22 +65,22 @@ snapshots["menu-bar overlay"] =
     aria-orientation="vertical"
     role="menu"
   >
-    <vaadin-context-menu-item
+    <vaadin-menu-bar-item
       aria-haspopup="false"
       aria-selected="false"
       role="menuitem"
       tabindex="0"
     >
       View Reports
-    </vaadin-context-menu-item>
-    <vaadin-context-menu-item
+    </vaadin-menu-bar-item>
+    <vaadin-menu-bar-item
       aria-haspopup="false"
       aria-selected="false"
       role="menuitem"
       tabindex="-1"
     >
       Generate Report
-    </vaadin-context-menu-item>
+    </vaadin-menu-bar-item>
   </vaadin-menu-bar-list-box>
   <vaadin-menu-bar-submenu hidden="">
   </vaadin-menu-bar-submenu>
@@ -105,22 +102,22 @@ snapshots["menu-bar overlay class"] =
     aria-orientation="vertical"
     role="menu"
   >
-    <vaadin-context-menu-item
+    <vaadin-menu-bar-item
       aria-haspopup="false"
       aria-selected="false"
       role="menuitem"
       tabindex="0"
     >
       View Reports
-    </vaadin-context-menu-item>
-    <vaadin-context-menu-item
+    </vaadin-menu-bar-item>
+    <vaadin-menu-bar-item
       aria-haspopup="false"
       aria-selected="false"
       role="menuitem"
       tabindex="-1"
     >
       Generate Report
-    </vaadin-context-menu-item>
+    </vaadin-menu-bar-item>
   </vaadin-menu-bar-list-box>
   <vaadin-menu-bar-submenu hidden="">
   </vaadin-menu-bar-submenu>

diff --git a/packages/menu-bar/test/dom/menu-bar.test.js b/packages/menu-bar/test/dom/menu-bar.test.js
@@ -23,7 +23,7 @@ describe('menu-bar', () => {
       { text: 'Dashboard', disabled: true },
       {
         component: (() => {
-          const item = document.createElement('vaadin-context-menu-item');
+          const item = document.createElement('vaadin-menu-bar-item');
           const bold = document.createElement('strong');
           bold.textContent = 'Help';
           item.appendChild(bold);

diff --git a/packages/menu-bar/test/menu-bar.test.js b/packages/menu-bar/test/menu-bar.test.js
@@ -667,17 +667,17 @@ describe('item components', () => {
       { text: 'Item 2' },
       { component: makeComponent('3') },
       { text: 'Item 4 text', component: makeComponent('4') },
-      { text: 'Item 5', component: document.createElement('vaadin-context-menu-item') },
+      { text: 'Item 5', component: document.createElement('vaadin-menu-bar-item') },
     ];
     await nextRender(menu);
     buttons = menu._buttons;
     overflow = buttons[buttons.length - 1];
   });
 
-  it('should render the component inside the context-menu item', () => {
+  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-context-menu-item');
+    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');
@@ -688,15 +688,15 @@ describe('item components', () => {
   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-context-menu-item');
+    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 context-menu item as a component', () => {
+  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);