Skip to content

Commit

Permalink
fix: focus first non-disabled item if all items have tabindex -1 (#7230
Browse files Browse the repository at this point in the history
…) (#7239)

Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
  • Loading branch information
vaadin-bot and web-padawan committed Mar 19, 2024
1 parent 2a919ed commit f5f5413
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 3 deletions.
2 changes: 1 addition & 1 deletion packages/a11y-base/src/keyboard-direction-mixin.js
Expand Up @@ -36,7 +36,7 @@ export const KeyboardDirectionMixin = (superclass) =>
if (Array.isArray(items)) {
const idx = this._getAvailableIndex(items, 0, null, (item) => !isElementHidden(item));
if (idx >= 0) {
items[idx].focus();
this._focus(idx);
}
}
}
Expand Down
12 changes: 10 additions & 2 deletions packages/a11y-base/src/list-mixin.js
Expand Up @@ -8,6 +8,7 @@ import { Debouncer } from '@vaadin/component-base/src/debounce.js';
import { getNormalizedScrollLeft, setNormalizedScrollLeft } from '@vaadin/component-base/src/dir-utils.js';
import { getFlattenedElements } from '@vaadin/component-base/src/dom-utils.js';
import { SlotObserver } from '@vaadin/component-base/src/slot-observer.js';
import { isElementHidden } from './focus-utils.js';
import { KeyboardDirectionMixin } from './keyboard-direction-mixin.js';

/**
Expand Down Expand Up @@ -117,8 +118,15 @@ export const ListMixin = (superClass) =>
if (this._observer) {
this._observer.flush();
}
const firstItem = this.querySelector('[tabindex="0"]') || (this.items ? this.items[0] : null);
this._focusItem(firstItem);

const items = Array.isArray(this.items) ? this.items : [];
const idx = this._getAvailableIndex(items, 0, null, (item) => item.tabIndex === 0 && !isElementHidden(item));
if (idx >= 0) {
this._focus(idx);
} else {
// Call `KeyboardDirectionMixin` logic to focus first non-disabled item.
super.focus();
}
}

/** @protected */
Expand Down
34 changes: 34 additions & 0 deletions packages/a11y-base/test/list-mixin.test.js
Expand Up @@ -288,6 +288,40 @@ const runTests = (defineHelper, baseMixin) => {
list.focus();
expect(spy.calledOnce).to.be.true;
});

it('should call focus() on the first non-disabled item if all items have tabindex -1', () => {
list.items.forEach((item) => {
item.tabIndex = -1;
});
const spy = sinon.spy(list.items[2], 'focus');
list.focus();
expect(spy.calledOnce).to.be.true;
});

it('should call focus() on the first non-disabled visible item if all items have tabindex -1', () => {
list.items.forEach((item) => {
item.tabIndex = -1;
});
list.items[2].setAttribute('hidden', '');
const spy = sinon.spy(list.items[3], 'focus');
list.focus();
expect(spy.calledOnce).to.be.true;
});

it('should change tabindex from -1 to 0 on first non-disabled item when focusing it', () => {
list.items.forEach((item) => {
item.tabIndex = -1;
});
list.focus();
[-1, -1, 0, -1].forEach((val, idx) => expect(list.items[idx].tabIndex).to.equal(val));
});

it('should change tabindex from 0 to -1 on items with tabindex 0 other than focused one', () => {
list.items[2].tabIndex = 0;
list.items[3].tabIndex = 0;
list.focus();
[-1, -1, 0, -1].forEach((val, idx) => expect(list.items[idx].tabIndex).to.equal(val));
});
});

describe('tabIndex when all the items are disabled', () => {
Expand Down
28 changes: 28 additions & 0 deletions packages/context-menu/test/items.common.js
Expand Up @@ -414,6 +414,34 @@ describe('items', () => {
expect(items[0].hasAttribute('focus-ring')).to.be.true;
});

it('should focus first non-disabled item after re-opening when using components', async () => {
subMenu.close();
rootOverlay.focus();

rootMenu.items[3].children[0].disabled = true;

const rootItem = getMenuItems(rootMenu)[3];

// Open the sub-menu with item components
await openMenu(rootItem);
const subMenu2 = getSubMenu(rootMenu);
const items = getMenuItems(subMenu2);

// Arrow Down to focus next item
items[1].focus();
arrowDownKeyDown(items[1]);
expect(items[2].hasAttribute('focus-ring')).to.be.true;

// Arrow Left to close the sub-menu
arrowLeftKeyDown(items[2]);
await nextFrame();
expect(rootItem.hasAttribute('focus-ring')).to.be.true;

// Re-open sub-menu and check focus
await openMenu(rootItem);
expect(items[1].hasAttribute('focus-ring')).to.be.true;
});

it('should have modeless sub menus', () => {
const rootItemRect = getMenuItems(rootMenu)[0].getBoundingClientRect();
const element = document.elementFromPoint(rootItemRect.left, rootItemRect.top);
Expand Down
20 changes: 20 additions & 0 deletions packages/menu-bar/test/sub-menu.test.js
Expand Up @@ -58,6 +58,9 @@ describe('sub-menu', () => {
{
component: createComponent('Menu Item 3 2'),
},
{
component: createComponent('Menu Item 3 3'),
},
],
},
];
Expand Down Expand Up @@ -181,6 +184,23 @@ describe('sub-menu', () => {
expect(items[0].hasAttribute('focus-ring')).to.be.true;
});

it('should focus first non-disabled item after re-opening when using components', async () => {
menu.items[2].children[0].disabled = true;

arrowDown(buttons[2]);
await nextRender(subMenu);

const items = subMenuOverlay.querySelectorAll('vaadin-menu-bar-item');
expect(items[1].hasAttribute('focus-ring')).to.be.true;

// Close and re-open
esc(items[1]);
arrowDown(buttons[2]);
await nextRender(subMenu);

expect(items[1].hasAttribute('focus-ring')).to.be.true;
});

it('should close sub-menu on first item arrow up', async () => {
arrowDown(buttons[0]);
await oneEvent(subMenu, 'opened-changed');
Expand Down

0 comments on commit f5f5413

Please sign in to comment.