refactor: restore the original fix, move tabindex logic

Author: web-padawan

packages/a11y-base/src/keyboard-direction-mixin.js

@@ -44,15 +44,23 @@ export const KeyboardDirectionMixin = (superclass) =>
      * @override
      */
     focus(options) {
-      const items = this._getItems();
-      if (Array.isArray(items)) {
-        const idx = this._getAvailableIndex(items, 0, null, (item) => !isElementHidden(item));
-        if (idx >= 0) {
-          this._focus(idx, options);
-        }
+      const idx = this._getFocusableIndex();
+      if (idx >= 0) {
+        this._focus(idx, options);
       }
     }
 
+    /**
+     * Get the index of a first focusable item, if any.
+     *
+     * @return {Element[]}
+     * @protected
+     */
+    _getFocusableIndex() {
+      const items = this._getItems();
+      return Array.isArray(items) ? this._getAvailableIndex(items, 0, null, (item) => !isElementHidden(item)) : -1;
+    }
+
     /**
      * Get the list of items participating in keyboard navigation.
      * By default, it treats all the light DOM children as items.

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

@@ -8,7 +8,7 @@ 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';
+import { isElementFocused, isElementHidden, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
 import { KeyboardDirectionMixin } from '@vaadin/a11y-base/src/keyboard-direction-mixin.js';
 import { microTask } from '@vaadin/component-base/src/async.js';
 import { Debouncer } from '@vaadin/component-base/src/debounce.js';
@@ -268,7 +268,6 @@ export const MenuBarMixin = (superClass) =>
     set _hasOverflow(hasOverflow) {
       if (this._overflow) {
         this._overflow.toggleAttribute('hidden', !hasOverflow);
-        this._overflow.setAttribute('tabindex', hasOverflow ? '0' : '-1');
       }
     }
 
@@ -502,9 +501,12 @@ export const MenuBarMixin = (superClass) =>
         const items = buttons.filter((b) => !remaining.includes(b)).map((b) => b.item);
         this.__updateOverflow(items);
 
-        // Ensure there is at least one button with tabindex set to 0
-        // so that menu-bar is not skipped when navigating with Tab
-        if (remaining.length && !remaining.some((btn) => btn.getAttribute('tabindex') === '0')) {
+        if (!remaining.length) {
+          // If all buttons except overflow are hidden, set tabindex on it
+          this._overflow.setAttribute('tabindex', '0');
+        } else if (!remaining.some((btn) => btn.getAttribute('tabindex') === '0')) {
+          // Ensure there is at least one button with tabindex set to 0
+          // so that menu-bar is not skipped when navigating with Tab
           this._setTabindex(remaining[remaining.length - 1], true);
         }
       }
@@ -762,7 +764,7 @@ export const MenuBarMixin = (superClass) =>
      */
     _setFocused(focused) {
       if (focused) {
-        const target = this._buttons.find((btn) => isElementFocused(btn));
+        const target = this.__getFocusTarget();
         if (target) {
           this._buttons.forEach((btn) => {
             this._setTabindex(btn, btn === target);
@@ -776,6 +778,24 @@ export const MenuBarMixin = (superClass) =>
       }
     }
 
+    /** @private */
+    __getFocusTarget() {
+      // First, check if focus is moving to a visible button
+      let target = this._buttons.find((btn) => isElementFocused(btn));
+
+      if (!target) {
+        const selector = this.tabNavigation ? '[focused]' : '[tabindex="0"]';
+        // Next, check if there is a button that could be focused but is hidden
+        target = this.querySelector(`vaadin-menu-bar-button${selector}`);
+
+        if (isElementHidden(target)) {
+          target = this._buttons[this._getFocusableIndex()];
+        }
+      }
+
+      return target;
+    }
+
     /**
      * @param {!KeyboardEvent} event
      * @private

packages/menu-bar/test/dom/__snapshots__/menu-bar.test.snap.js

@@ -56,7 +56,7 @@ snapshots["menu-bar basic"] =
     hidden=""
     role="menuitem"
     slot="overflow"
-    tabindex="-1"
+    tabindex="0"
   >
     <div aria-hidden="true">
       ···
@@ -156,7 +156,7 @@ snapshots["menu-bar opened"] =
     hidden=""
     role="menuitem"
     slot="overflow"
-    tabindex="-1"
+    tabindex="0"
   >
     <div aria-hidden="true">
       ···