refactor: make _getAvailableIndex check item visibility by default

packages/a11y-base/src/keyboard-direction-mixin.d.ts

@@ -26,7 +26,7 @@ export declare class KeyboardDirectionMixinClass {
     items: Element[],
     index: number,
     increment: number,
-    condition: (item: Element) => boolean,
+    condition?: (item: Element) => boolean,
   ): number;
 
   /**

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

@@ -34,7 +34,7 @@ export const KeyboardDirectionMixin = (superclass) =>
     focus() {
       const items = this._getItems();
       if (Array.isArray(items)) {
-        const idx = this._getAvailableIndex(items, 0, null, (item) => !isElementHidden(item));
+        const idx = this._getAvailableIndex(items, 0, null);
         if (idx >= 0) {
           this._focus(idx);
         }
@@ -91,7 +91,7 @@ export const KeyboardDirectionMixin = (superclass) =>
         idx = items.length - 1;
       }
 
-      idx = this._getAvailableIndex(items, idx, increment, (item) => !isElementHidden(item));
+      idx = this._getAvailableIndex(items, idx, increment);
 
       if (idx >= 0) {
         event.preventDefault();
@@ -171,22 +171,28 @@ export const KeyboardDirectionMixin = (superclass) =>
 
         const item = items[idx];
 
-        if (!item.hasAttribute('disabled') && this.__isMatchingItem(item, condition)) {
-          return idx;
+        if (!this._isItemFocusable(item)) {
+          continue;
         }
+
+        if (typeof condition === 'function' && !condition(item)) {
+          continue;
+        }
+
+        return idx;
       }
       return -1;
     }
 
     /**
-     * Returns true if the item matches condition.
+     * Returns whether the item is focusable. By default,
+     * returns true if the item is visible and not disabled.
      *
-     * @param {Element} item - item to check
-     * @param {Function} condition - function used to check the item
-     * @return {number}
-     * @private
+     * @param {Element} item
+     * @return {boolean}
+     * @protected
      */
-    __isMatchingItem(item, condition) {
-      return typeof condition === 'function' ? condition(item) : true;
+    _isItemFocusable(item) {
+      return !item.hasAttribute('disabled') && !isElementHidden(item);
     }
   };

packages/a11y-base/src/list-mixin.js

@@ -265,15 +265,6 @@ export const ListMixin = (superClass) =>
       super._onKeyDown(event);
     }
 
-    /**
-     * @param {!Element} item
-     * @return {boolean}
-     * @protected
-     */
-    _isItemHidden(item) {
-      return getComputedStyle(item).display === 'none';
-    }
-
     /**
      * @param {number} idx
      * @protected
@@ -346,6 +337,11 @@ export const ListMixin = (superClass) =>
       }
     }
 
+    /** @override */
+    _isItemFocusable(item) {
+      return !item.hasAttribute('disabled');
+    }
+
     /**
      * Fired when the selection is changed.
      * Not fired when used in `multiple` selection mode.

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, isElementHidden, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
+import { isElementFocused, 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';
@@ -880,7 +880,7 @@ export const MenuBarMixin = (superClass) =>
           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));
+          idx = this._getAvailableIndex(items, idx, increment);
           this.__switchSubMenu(items[idx]);
         }
       }