From b7759659283782749b7860f95c2d584a6edd9a09 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Gueriaud <51313578+jcgueriaud1@users.noreply.github.com>
Date: Mon, 12 Aug 2024 11:52:05 +0300
Subject: [PATCH] feat: add tabNavigation option for using menu-bar as button
 group (#7525)

---
 .../menu-bar/src/vaadin-menu-bar-mixin.d.ts   |  7 +++
 .../menu-bar/src/vaadin-menu-bar-mixin.js     | 62 +++++++++++++++++--
 packages/menu-bar/test/a11y.common.js         | 37 +++++++++++
 .../dom/__snapshots__/menu-bar.test.snap.js   |  6 +-
 packages/menu-bar/test/menu-bar.common.js     | 59 ++++++++++++++++++
 packages/menu-bar/test/overflow.common.js     | 16 +++++
 packages/menu-bar/test/sub-menu.common.js     | 35 +++++++++++
 .../menu-bar/test/typings/menu-bar.types.ts   |  1 +
 8 files changed, 216 insertions(+), 7 deletions(-)

diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.d.ts b/packages/menu-bar/src/vaadin-menu-bar-mixin.d.ts
index a3606da846..2a90840d07 100644
--- a/packages/menu-bar/src/vaadin-menu-bar-mixin.d.ts
+++ b/packages/menu-bar/src/vaadin-menu-bar-mixin.d.ts
@@ -145,6 +145,13 @@ export declare class MenuBarMixinClass {
    */
   reverseCollapse: boolean | null | undefined;
 
+  /**
+   * If true, the top-level menu items is traversable by tab
+   * instead of arrow keys (i.e. disabling roving tabindex)
+   * @attr {boolean} tab-navigation
+   */
+  tabNavigation: boolean | null | undefined;
+
   /**
    * Closes the current submenu.
    */
diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.js b/packages/menu-bar/src/vaadin-menu-bar-mixin.js
index 76154b503b..8e492760db 100644
--- a/packages/menu-bar/src/vaadin-menu-bar-mixin.js
+++ b/packages/menu-bar/src/vaadin-menu-bar-mixin.js
@@ -5,7 +5,7 @@
  */
 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';
+import { isElementFocused, isElementHidden, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
 import { KeyboardDirectionMixin } from '@vaadin/a11y-base/src/keyboard-direction-mixin.js';
 import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
 import { ResizeMixin } from '@vaadin/component-base/src/resize-mixin.js';
@@ -144,6 +144,15 @@ export const MenuBarMixin = (superClass) =>
           type: Boolean,
         },
 
+        /**
+         * If true, the top-level menu items is traversable by tab
+         * instead of arrow keys (i.e. disabling roving tabindex)
+         * @attr {boolean} tab-navigation
+         */
+        tabNavigation: {
+          type: Boolean,
+        },
+
         /**
          * @type {boolean}
          * @protected
@@ -173,6 +182,7 @@ export const MenuBarMixin = (superClass) =>
         '__i18nChanged(i18n, _overflow)',
         '_menuItemsChanged(items, _overflow, _container)',
         '_reverseCollapseChanged(reverseCollapse, _overflow, _container)',
+        '_tabNavigationChanged(tabNavigation, _overflow, _container)',
       ];
     }
 
@@ -349,6 +359,22 @@ export const MenuBarMixin = (superClass) =>
       }
     }
 
+    /** @private */
+    _tabNavigationChanged(tabNavigation, overflow, container) {
+      if (overflow && container) {
+        const target = this.querySelector('[tabindex="0"]');
+        this._buttons.forEach((btn) => {
+          if (target) {
+            this._setTabindex(btn, btn === target);
+          } else {
+            this._setTabindex(btn, false);
+          }
+          btn.setAttribute('role', tabNavigation ? 'button' : 'menuitem');
+        });
+      }
+      this.setAttribute('role', tabNavigation ? 'group' : 'menubar');
+    }
+
     /** @private */
     __hasOverflowChanged(hasOverflow, overflow) {
       if (overflow) {
@@ -540,7 +566,7 @@ export const MenuBarMixin = (superClass) =>
 
     /** @protected */
     _initButtonAttrs(button) {
-      button.setAttribute('role', 'menuitem');
+      button.setAttribute('role', this.tabNavigation ? 'button' : 'menuitem');
 
       if (button === this._overflow || (button.item && button.item.children)) {
         button.setAttribute('aria-haspopup', 'true');
@@ -667,7 +693,11 @@ export const MenuBarMixin = (superClass) =>
 
     /** @protected */
     _setTabindex(button, focused) {
-      button.setAttribute('tabindex', focused ? '0' : '-1');
+      if (this.tabNavigation && !button.disabled) {
+        button.setAttribute('tabindex', '0');
+      } else {
+        button.setAttribute('tabindex', focused ? '0' : '-1');
+      }
     }
 
     /**
@@ -715,7 +745,12 @@ export const MenuBarMixin = (superClass) =>
      */
     _setFocused(focused) {
       if (focused) {
-        const target = this.querySelector('[tabindex="0"]');
+        let target = this.querySelector('[tabindex="0"]');
+        if (this.tabNavigation) {
+          // Switch submenu on menu button Tab / Shift Tab
+          target = this.querySelector('[focused]');
+          this.__switchSubMenu(target);
+        }
         if (target) {
           this._buttons.forEach((btn) => {
             this._setTabindex(btn, btn === target);
@@ -839,6 +874,25 @@ export const MenuBarMixin = (superClass) =>
           // Prevent ArrowLeft from being handled in context-menu
           e.stopImmediatePropagation();
           this._onKeyDown(e);
+        } else if (e.keyCode === 9 && this.tabNavigation) {
+          // Switch opened submenu on submenu item Tab / Shift Tab
+          const items = this._getItems() || [];
+          const currentIdx = items.indexOf(this.focused);
+          const increment = e.shiftKey ? -1 : 1;
+          let idx = currentIdx + increment;
+          idx = this._getAvailableIndex(items, idx, increment, (item) => !isElementHidden(item));
+          this.__switchSubMenu(items[idx]);
+        }
+      }
+    }
+
+    /** @private */
+    __switchSubMenu(target) {
+      const wasExpanded = this._expandedButton != null && this._expandedButton !== target;
+      if (wasExpanded) {
+        this._close();
+        if (target.item && target.item.children) {
+          this.__openSubMenu(target, true, { keepFocus: true });
         }
       }
     }
diff --git a/packages/menu-bar/test/a11y.common.js b/packages/menu-bar/test/a11y.common.js
index 05f4af74cc..16dc840557 100644
--- a/packages/menu-bar/test/a11y.common.js
+++ b/packages/menu-bar/test/a11y.common.js
@@ -35,6 +35,43 @@ describe('a11y', () => {
       });
     });
 
+    it('should set role attribute on host element in tabNavigation', async () => {
+      menu.tabNavigation = true;
+      await nextRender(menu);
+      expect(menu.getAttribute('role')).to.equal('group');
+    });
+
+    it('should set role attribute on menu bar buttons in tabNavigation', async () => {
+      menu.tabNavigation = true;
+      await nextRender(menu);
+      buttons.forEach((btn) => {
+        expect(btn.getAttribute('role')).to.equal('button');
+      });
+    });
+
+    it('should update role attribute on menu bar buttons when changing items', async () => {
+      menu.items = [...menu.items, { text: 'New item' }];
+      await nextRender(menu);
+      menu._buttons.forEach((btn) => {
+        expect(btn.getAttribute('role')).to.equal('menuitem');
+      });
+    });
+
+    it('should update role attribute on menu bar buttons when changing items in tabNavigation', async () => {
+      menu.tabNavigation = true;
+      await nextRender(menu);
+      menu.items = [...menu.items, { text: 'New item' }];
+      await nextRender(menu);
+      menu._buttons.forEach((btn) => {
+        expect(btn.getAttribute('role')).to.equal('button');
+      });
+      menu.tabNavigation = false;
+      await nextRender(menu);
+      menu._buttons.forEach((btn) => {
+        expect(btn.getAttribute('role')).to.equal('menuitem');
+      });
+    });
+
     it('should set aria-haspopup attribute on buttons with nested items', () => {
       buttons.forEach((btn) => {
         const hasPopup = btn === overflow || btn.item.children ? 'true' : null;
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
index df80685f07..c949d5c55e 100644
--- a/packages/menu-bar/test/dom/__snapshots__/menu-bar.test.snap.js
+++ b/packages/menu-bar/test/dom/__snapshots__/menu-bar.test.snap.js
@@ -15,7 +15,7 @@ snapshots["menu-bar basic"] =
     aria-expanded="false"
     aria-haspopup="true"
     role="menuitem"
-    tabindex="0"
+    tabindex="-1"
   >
     Reports
   </vaadin-menu-bar-button>
@@ -31,7 +31,7 @@ snapshots["menu-bar basic"] =
     class="help"
     last-visible=""
     role="menuitem"
-    tabindex="0"
+    tabindex="-1"
   >
     <vaadin-menu-bar-item aria-selected="false">
       <strong>
@@ -46,7 +46,7 @@ snapshots["menu-bar basic"] =
     hidden=""
     role="menuitem"
     slot="overflow"
-    tabindex="0"
+    tabindex="-1"
   >
     <div aria-hidden="true">
       ···
diff --git a/packages/menu-bar/test/menu-bar.common.js b/packages/menu-bar/test/menu-bar.common.js
index 7dd8f56416..66e67506da 100644
--- a/packages/menu-bar/test/menu-bar.common.js
+++ b/packages/menu-bar/test/menu-bar.common.js
@@ -9,6 +9,7 @@ import {
   nextRender,
   nextUpdate,
 } from '@vaadin/testing-helpers';
+import { sendKeys } from '@web/test-runner-commands';
 import sinon from 'sinon';
 
 describe('custom element definition', () => {
@@ -89,6 +90,29 @@ describe('root menu layout', () => {
     });
   });
 
+  it('should set tabindex to 0 when the button is not disabled in tab navigation', async () => {
+    menu.tabNavigation = true;
+    await nextUpdate(menu);
+    buttons.forEach((btn) => {
+      if (btn.disabled) {
+        expect(btn.getAttribute('tabindex')).to.equal('-1');
+      } else {
+        expect(btn.getAttribute('tabindex')).to.equal('0');
+      }
+    });
+  });
+
+  it('should reset tabindex after switching back from tab navigation', async () => {
+    menu.tabNavigation = true;
+    await nextUpdate(menu);
+    menu.tabNavigation = false;
+    await nextUpdate(menu);
+    expect(buttons[0].getAttribute('tabindex')).to.equal('0');
+    buttons.slice(1).forEach((btn) => {
+      expect(btn.getAttribute('tabindex')).to.equal('-1');
+    });
+  });
+
   it('should not throw when changing items before the menu bar is attached', () => {
     expect(() => {
       const menuBar = document.createElement('vaadin-menu-bar');
@@ -206,6 +230,41 @@ describe('root menu layout', () => {
     });
   });
 
+  describe('tab navigation mode', () => {
+    beforeEach(() => {
+      menu.tabNavigation = true;
+    });
+
+    it('should move focus to next button on Tab keydown', async () => {
+      buttons[0].focus();
+      await sendKeys({ press: 'Tab' });
+      expect(buttons[1].hasAttribute('focused')).to.be.true;
+    });
+
+    it('should move focus to prev button on Shift Tab keydown', async () => {
+      buttons[1].focus();
+      await sendKeys({ down: 'Shift' });
+      await sendKeys({ press: 'Tab' });
+      await sendKeys({ up: 'Shift' });
+      expect(buttons[0].hasAttribute('focused')).to.be.true;
+    });
+
+    it('should move focus to fourth button if third is disabled on Tab keydown', async () => {
+      await updateItemsAndButtons();
+      buttons[1].focus();
+      await sendKeys({ press: 'Tab' });
+      expect(buttons[3].hasAttribute('focused')).to.be.true;
+    });
+
+    it('should not move the focus to the next button if tab navigation is disabled', async () => {
+      menu.tabNavigation = false;
+      menu.focus();
+      expect(document.activeElement).to.equal(buttons[0]);
+      await sendKeys({ press: 'Tab' });
+      expect(document.activeElement).to.not.equal(buttons[1]);
+    });
+  });
+
   describe('updating items', () => {
     it('should remove buttons when setting empty array', async () => {
       menu.items = [];
diff --git a/packages/menu-bar/test/overflow.common.js b/packages/menu-bar/test/overflow.common.js
index 8e702b5d18..c5a3d5da41 100644
--- a/packages/menu-bar/test/overflow.common.js
+++ b/packages/menu-bar/test/overflow.common.js
@@ -179,6 +179,22 @@ describe('overflow', () => {
       await onceResized(menu);
 
       expect(buttons[0].getAttribute('tabindex')).to.equal('0');
+      expect(buttons[1].getAttribute('tabindex')).to.equal('-1');
+    });
+
+    it('should set tabindex -1 on the overflow menu in tab navigation', async () => {
+      menu.tabNavigation = true;
+      buttons[0].focus();
+      arrowRight(buttons[0]);
+
+      expect(buttons[0].getAttribute('tabindex')).to.equal('0');
+      expect(buttons[1].getAttribute('tabindex')).to.equal('0');
+
+      menu.style.width = '150px';
+      await onceResized(menu);
+
+      expect(buttons[0].getAttribute('tabindex')).to.equal('0');
+      expect(buttons[1].getAttribute('tabindex')).to.equal('-1');
     });
 
     it('should set the aria-label of the overflow button according to the i18n of the menu bar', async () => {
diff --git a/packages/menu-bar/test/sub-menu.common.js b/packages/menu-bar/test/sub-menu.common.js
index 8e4503c9aa..dbf4cb5d9a 100644
--- a/packages/menu-bar/test/sub-menu.common.js
+++ b/packages/menu-bar/test/sub-menu.common.js
@@ -17,6 +17,7 @@ import {
   touchend,
   touchstart,
 } from '@vaadin/testing-helpers';
+import { sendKeys } from '@web/test-runner-commands';
 import sinon from 'sinon';
 import { setCancelSyntheticClickEvents } from '@polymer/polymer/lib/utils/settings.js';
 import { isTouch } from '@vaadin/component-base/src/browser-utils.js';
@@ -261,6 +262,40 @@ describe('sub-menu', () => {
     expect(buttons[1].hasAttribute('focus-ring')).to.be.true;
   });
 
+  it('should switch menubar button with items and open submenu on Tab in tab navigation', async () => {
+    menu.tabNavigation = true;
+    menu.items = [...menu.items, { text: 'Menu Item 4', children: [{ text: 'Menu Item 4 1' }] }];
+    await nextUpdate(menu);
+    buttons = menu._buttons;
+    buttons[2].focus();
+    arrowDown(buttons[2]);
+    await oneEvent(subMenu, 'opened-changed');
+
+    await sendKeys({ press: 'Tab' });
+    await nextRender(subMenu);
+
+    expect(subMenu.opened).to.be.true;
+    expect(subMenu.listenOn).to.equal(buttons[3]);
+  });
+
+  it('should switch menubar button with items and open submenu on Shift Tab in tab navigation', async () => {
+    menu.tabNavigation = true;
+    menu.items = [...menu.items, { text: 'Menu Item 4', children: [{ text: 'Menu Item 4 1' }] }];
+    await nextUpdate(menu);
+    buttons = menu._buttons;
+    buttons[3].focus();
+    arrowDown(buttons[3]);
+    await oneEvent(subMenu, 'opened-changed');
+
+    await sendKeys({ down: 'Shift' });
+    await sendKeys({ press: 'Tab' });
+    await sendKeys({ up: 'Shift' });
+    await nextRender(subMenu);
+
+    expect(subMenu.opened).to.be.true;
+    expect(subMenu.listenOn).to.equal(buttons[2]);
+  });
+
   it('should focus first item on arrow down after opened on arrow left', async () => {
     arrowDown(buttons[0]);
     await oneEvent(subMenu, 'opened-changed');
diff --git a/packages/menu-bar/test/typings/menu-bar.types.ts b/packages/menu-bar/test/typings/menu-bar.types.ts
index 39e02cbf86..07e8f4aca7 100644
--- a/packages/menu-bar/test/typings/menu-bar.types.ts
+++ b/packages/menu-bar/test/typings/menu-bar.types.ts
@@ -16,6 +16,7 @@ const menu = document.createElement('vaadin-menu-bar');
 const assertType = <TExpected>(actual: TExpected) => actual;
 
 assertType<boolean | null | undefined>(menu.openOnHover);
+assertType<boolean | null | undefined>(menu.tabNavigation);
 assertType<MenuItem[]>(menu.items);
 assertType<string>(menu.overlayClass);