Skip to content

Commit b775965

Browse files
authored
feat: add tabNavigation option for using menu-bar as button group (#7525)
1 parent 351ddd6 commit b775965

File tree

8 files changed

+216
-7
lines changed

8 files changed

+216
-7
lines changed

packages/menu-bar/src/vaadin-menu-bar-mixin.d.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,13 @@ export declare class MenuBarMixinClass {
145145
*/
146146
reverseCollapse: boolean | null | undefined;
147147

148+
/**
149+
* If true, the top-level menu items is traversable by tab
150+
* instead of arrow keys (i.e. disabling roving tabindex)
151+
* @attr {boolean} tab-navigation
152+
*/
153+
tabNavigation: boolean | null | undefined;
154+
148155
/**
149156
* Closes the current submenu.
150157
*/

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

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66
import { DisabledMixin } from '@vaadin/a11y-base/src/disabled-mixin.js';
77
import { FocusMixin } from '@vaadin/a11y-base/src/focus-mixin.js';
8-
import { isElementFocused, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
8+
import { isElementFocused, isElementHidden, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
99
import { KeyboardDirectionMixin } from '@vaadin/a11y-base/src/keyboard-direction-mixin.js';
1010
import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js';
1111
import { ResizeMixin } from '@vaadin/component-base/src/resize-mixin.js';
@@ -144,6 +144,15 @@ export const MenuBarMixin = (superClass) =>
144144
type: Boolean,
145145
},
146146

147+
/**
148+
* If true, the top-level menu items is traversable by tab
149+
* instead of arrow keys (i.e. disabling roving tabindex)
150+
* @attr {boolean} tab-navigation
151+
*/
152+
tabNavigation: {
153+
type: Boolean,
154+
},
155+
147156
/**
148157
* @type {boolean}
149158
* @protected
@@ -173,6 +182,7 @@ export const MenuBarMixin = (superClass) =>
173182
'__i18nChanged(i18n, _overflow)',
174183
'_menuItemsChanged(items, _overflow, _container)',
175184
'_reverseCollapseChanged(reverseCollapse, _overflow, _container)',
185+
'_tabNavigationChanged(tabNavigation, _overflow, _container)',
176186
];
177187
}
178188

@@ -349,6 +359,22 @@ export const MenuBarMixin = (superClass) =>
349359
}
350360
}
351361

362+
/** @private */
363+
_tabNavigationChanged(tabNavigation, overflow, container) {
364+
if (overflow && container) {
365+
const target = this.querySelector('[tabindex="0"]');
366+
this._buttons.forEach((btn) => {
367+
if (target) {
368+
this._setTabindex(btn, btn === target);
369+
} else {
370+
this._setTabindex(btn, false);
371+
}
372+
btn.setAttribute('role', tabNavigation ? 'button' : 'menuitem');
373+
});
374+
}
375+
this.setAttribute('role', tabNavigation ? 'group' : 'menubar');
376+
}
377+
352378
/** @private */
353379
__hasOverflowChanged(hasOverflow, overflow) {
354380
if (overflow) {
@@ -540,7 +566,7 @@ export const MenuBarMixin = (superClass) =>
540566

541567
/** @protected */
542568
_initButtonAttrs(button) {
543-
button.setAttribute('role', 'menuitem');
569+
button.setAttribute('role', this.tabNavigation ? 'button' : 'menuitem');
544570

545571
if (button === this._overflow || (button.item && button.item.children)) {
546572
button.setAttribute('aria-haspopup', 'true');
@@ -667,7 +693,11 @@ export const MenuBarMixin = (superClass) =>
667693

668694
/** @protected */
669695
_setTabindex(button, focused) {
670-
button.setAttribute('tabindex', focused ? '0' : '-1');
696+
if (this.tabNavigation && !button.disabled) {
697+
button.setAttribute('tabindex', '0');
698+
} else {
699+
button.setAttribute('tabindex', focused ? '0' : '-1');
700+
}
671701
}
672702

673703
/**
@@ -715,7 +745,12 @@ export const MenuBarMixin = (superClass) =>
715745
*/
716746
_setFocused(focused) {
717747
if (focused) {
718-
const target = this.querySelector('[tabindex="0"]');
748+
let target = this.querySelector('[tabindex="0"]');
749+
if (this.tabNavigation) {
750+
// Switch submenu on menu button Tab / Shift Tab
751+
target = this.querySelector('[focused]');
752+
this.__switchSubMenu(target);
753+
}
719754
if (target) {
720755
this._buttons.forEach((btn) => {
721756
this._setTabindex(btn, btn === target);
@@ -839,6 +874,25 @@ export const MenuBarMixin = (superClass) =>
839874
// Prevent ArrowLeft from being handled in context-menu
840875
e.stopImmediatePropagation();
841876
this._onKeyDown(e);
877+
} else if (e.keyCode === 9 && this.tabNavigation) {
878+
// Switch opened submenu on submenu item Tab / Shift Tab
879+
const items = this._getItems() || [];
880+
const currentIdx = items.indexOf(this.focused);
881+
const increment = e.shiftKey ? -1 : 1;
882+
let idx = currentIdx + increment;
883+
idx = this._getAvailableIndex(items, idx, increment, (item) => !isElementHidden(item));
884+
this.__switchSubMenu(items[idx]);
885+
}
886+
}
887+
}
888+
889+
/** @private */
890+
__switchSubMenu(target) {
891+
const wasExpanded = this._expandedButton != null && this._expandedButton !== target;
892+
if (wasExpanded) {
893+
this._close();
894+
if (target.item && target.item.children) {
895+
this.__openSubMenu(target, true, { keepFocus: true });
842896
}
843897
}
844898
}

packages/menu-bar/test/a11y.common.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,43 @@ describe('a11y', () => {
3535
});
3636
});
3737

38+
it('should set role attribute on host element in tabNavigation', async () => {
39+
menu.tabNavigation = true;
40+
await nextRender(menu);
41+
expect(menu.getAttribute('role')).to.equal('group');
42+
});
43+
44+
it('should set role attribute on menu bar buttons in tabNavigation', async () => {
45+
menu.tabNavigation = true;
46+
await nextRender(menu);
47+
buttons.forEach((btn) => {
48+
expect(btn.getAttribute('role')).to.equal('button');
49+
});
50+
});
51+
52+
it('should update role attribute on menu bar buttons when changing items', async () => {
53+
menu.items = [...menu.items, { text: 'New item' }];
54+
await nextRender(menu);
55+
menu._buttons.forEach((btn) => {
56+
expect(btn.getAttribute('role')).to.equal('menuitem');
57+
});
58+
});
59+
60+
it('should update role attribute on menu bar buttons when changing items in tabNavigation', async () => {
61+
menu.tabNavigation = true;
62+
await nextRender(menu);
63+
menu.items = [...menu.items, { text: 'New item' }];
64+
await nextRender(menu);
65+
menu._buttons.forEach((btn) => {
66+
expect(btn.getAttribute('role')).to.equal('button');
67+
});
68+
menu.tabNavigation = false;
69+
await nextRender(menu);
70+
menu._buttons.forEach((btn) => {
71+
expect(btn.getAttribute('role')).to.equal('menuitem');
72+
});
73+
});
74+
3875
it('should set aria-haspopup attribute on buttons with nested items', () => {
3976
buttons.forEach((btn) => {
4077
const hasPopup = btn === overflow || btn.item.children ? 'true' : null;

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ snapshots["menu-bar basic"] =
1515
aria-expanded="false"
1616
aria-haspopup="true"
1717
role="menuitem"
18-
tabindex="0"
18+
tabindex="-1"
1919
>
2020
Reports
2121
</vaadin-menu-bar-button>
@@ -31,7 +31,7 @@ snapshots["menu-bar basic"] =
3131
class="help"
3232
last-visible=""
3333
role="menuitem"
34-
tabindex="0"
34+
tabindex="-1"
3535
>
3636
<vaadin-menu-bar-item aria-selected="false">
3737
<strong>
@@ -46,7 +46,7 @@ snapshots["menu-bar basic"] =
4646
hidden=""
4747
role="menuitem"
4848
slot="overflow"
49-
tabindex="0"
49+
tabindex="-1"
5050
>
5151
<div aria-hidden="true">
5252
···

packages/menu-bar/test/menu-bar.common.js

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
nextRender,
1010
nextUpdate,
1111
} from '@vaadin/testing-helpers';
12+
import { sendKeys } from '@web/test-runner-commands';
1213
import sinon from 'sinon';
1314

1415
describe('custom element definition', () => {
@@ -89,6 +90,29 @@ describe('root menu layout', () => {
8990
});
9091
});
9192

93+
it('should set tabindex to 0 when the button is not disabled in tab navigation', async () => {
94+
menu.tabNavigation = true;
95+
await nextUpdate(menu);
96+
buttons.forEach((btn) => {
97+
if (btn.disabled) {
98+
expect(btn.getAttribute('tabindex')).to.equal('-1');
99+
} else {
100+
expect(btn.getAttribute('tabindex')).to.equal('0');
101+
}
102+
});
103+
});
104+
105+
it('should reset tabindex after switching back from tab navigation', async () => {
106+
menu.tabNavigation = true;
107+
await nextUpdate(menu);
108+
menu.tabNavigation = false;
109+
await nextUpdate(menu);
110+
expect(buttons[0].getAttribute('tabindex')).to.equal('0');
111+
buttons.slice(1).forEach((btn) => {
112+
expect(btn.getAttribute('tabindex')).to.equal('-1');
113+
});
114+
});
115+
92116
it('should not throw when changing items before the menu bar is attached', () => {
93117
expect(() => {
94118
const menuBar = document.createElement('vaadin-menu-bar');
@@ -206,6 +230,41 @@ describe('root menu layout', () => {
206230
});
207231
});
208232

233+
describe('tab navigation mode', () => {
234+
beforeEach(() => {
235+
menu.tabNavigation = true;
236+
});
237+
238+
it('should move focus to next button on Tab keydown', async () => {
239+
buttons[0].focus();
240+
await sendKeys({ press: 'Tab' });
241+
expect(buttons[1].hasAttribute('focused')).to.be.true;
242+
});
243+
244+
it('should move focus to prev button on Shift Tab keydown', async () => {
245+
buttons[1].focus();
246+
await sendKeys({ down: 'Shift' });
247+
await sendKeys({ press: 'Tab' });
248+
await sendKeys({ up: 'Shift' });
249+
expect(buttons[0].hasAttribute('focused')).to.be.true;
250+
});
251+
252+
it('should move focus to fourth button if third is disabled on Tab keydown', async () => {
253+
await updateItemsAndButtons();
254+
buttons[1].focus();
255+
await sendKeys({ press: 'Tab' });
256+
expect(buttons[3].hasAttribute('focused')).to.be.true;
257+
});
258+
259+
it('should not move the focus to the next button if tab navigation is disabled', async () => {
260+
menu.tabNavigation = false;
261+
menu.focus();
262+
expect(document.activeElement).to.equal(buttons[0]);
263+
await sendKeys({ press: 'Tab' });
264+
expect(document.activeElement).to.not.equal(buttons[1]);
265+
});
266+
});
267+
209268
describe('updating items', () => {
210269
it('should remove buttons when setting empty array', async () => {
211270
menu.items = [];

packages/menu-bar/test/overflow.common.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,22 @@ describe('overflow', () => {
179179
await onceResized(menu);
180180

181181
expect(buttons[0].getAttribute('tabindex')).to.equal('0');
182+
expect(buttons[1].getAttribute('tabindex')).to.equal('-1');
183+
});
184+
185+
it('should set tabindex -1 on the overflow menu in tab navigation', async () => {
186+
menu.tabNavigation = true;
187+
buttons[0].focus();
188+
arrowRight(buttons[0]);
189+
190+
expect(buttons[0].getAttribute('tabindex')).to.equal('0');
191+
expect(buttons[1].getAttribute('tabindex')).to.equal('0');
192+
193+
menu.style.width = '150px';
194+
await onceResized(menu);
195+
196+
expect(buttons[0].getAttribute('tabindex')).to.equal('0');
197+
expect(buttons[1].getAttribute('tabindex')).to.equal('-1');
182198
});
183199

184200
it('should set the aria-label of the overflow button according to the i18n of the menu bar', async () => {

packages/menu-bar/test/sub-menu.common.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
touchend,
1818
touchstart,
1919
} from '@vaadin/testing-helpers';
20+
import { sendKeys } from '@web/test-runner-commands';
2021
import sinon from 'sinon';
2122
import { setCancelSyntheticClickEvents } from '@polymer/polymer/lib/utils/settings.js';
2223
import { isTouch } from '@vaadin/component-base/src/browser-utils.js';
@@ -261,6 +262,40 @@ describe('sub-menu', () => {
261262
expect(buttons[1].hasAttribute('focus-ring')).to.be.true;
262263
});
263264

265+
it('should switch menubar button with items and open submenu on Tab in tab navigation', async () => {
266+
menu.tabNavigation = true;
267+
menu.items = [...menu.items, { text: 'Menu Item 4', children: [{ text: 'Menu Item 4 1' }] }];
268+
await nextUpdate(menu);
269+
buttons = menu._buttons;
270+
buttons[2].focus();
271+
arrowDown(buttons[2]);
272+
await oneEvent(subMenu, 'opened-changed');
273+
274+
await sendKeys({ press: 'Tab' });
275+
await nextRender(subMenu);
276+
277+
expect(subMenu.opened).to.be.true;
278+
expect(subMenu.listenOn).to.equal(buttons[3]);
279+
});
280+
281+
it('should switch menubar button with items and open submenu on Shift Tab in tab navigation', async () => {
282+
menu.tabNavigation = true;
283+
menu.items = [...menu.items, { text: 'Menu Item 4', children: [{ text: 'Menu Item 4 1' }] }];
284+
await nextUpdate(menu);
285+
buttons = menu._buttons;
286+
buttons[3].focus();
287+
arrowDown(buttons[3]);
288+
await oneEvent(subMenu, 'opened-changed');
289+
290+
await sendKeys({ down: 'Shift' });
291+
await sendKeys({ press: 'Tab' });
292+
await sendKeys({ up: 'Shift' });
293+
await nextRender(subMenu);
294+
295+
expect(subMenu.opened).to.be.true;
296+
expect(subMenu.listenOn).to.equal(buttons[2]);
297+
});
298+
264299
it('should focus first item on arrow down after opened on arrow left', async () => {
265300
arrowDown(buttons[0]);
266301
await oneEvent(subMenu, 'opened-changed');

packages/menu-bar/test/typings/menu-bar.types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ const menu = document.createElement('vaadin-menu-bar');
1616
const assertType = <TExpected>(actual: TExpected) => actual;
1717

1818
assertType<boolean | null | undefined>(menu.openOnHover);
19+
assertType<boolean | null | undefined>(menu.tabNavigation);
1920
assertType<MenuItem[]>(menu.items);
2021
assertType<string>(menu.overlayClass);
2122

0 commit comments

Comments
 (0)