Skip to content

Commit

Permalink
feat: make disabled menu bar buttons accessible with feature flag (#8518
Browse files Browse the repository at this point in the history
)
  • Loading branch information
vursen authored Jan 21, 2025
1 parent 69e64bf commit 360bba2
Show file tree
Hide file tree
Showing 11 changed files with 534 additions and 248 deletions.
6 changes: 6 additions & 0 deletions packages/a11y-base/src/keyboard-direction-mixin.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,10 @@ export declare class KeyboardDirectionMixinClass {
* Focus the given item. Override this method to add custom logic.
*/
protected _focusItem(item: Element, navigating: boolean): void;

/**
* Returns whether the item is focusable. By default,
* returns true if the item is not disabled.
*/
protected _isItemFocusable(item: Element): boolean;
}
14 changes: 13 additions & 1 deletion packages/a11y-base/src/keyboard-direction-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export const KeyboardDirectionMixin = (superclass) =>

const item = items[idx];

if (!item.hasAttribute('disabled') && this.__isMatchingItem(item, condition)) {
if (this._isItemFocusable(item) && this.__isMatchingItem(item, condition)) {
return idx;
}
}
Expand All @@ -189,4 +189,16 @@ export const KeyboardDirectionMixin = (superclass) =>
__isMatchingItem(item, condition) {
return typeof condition === 'function' ? condition(item) : true;
}

/**
* Returns whether the item is focusable. By default,
* returns true if the item is not disabled.
*
* @param {Element} item
* @return {boolean}
* @protected
*/
_isItemFocusable(item) {
return !item.hasAttribute('disabled');
}
};
12 changes: 11 additions & 1 deletion packages/button/src/vaadin-button-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,18 @@ export const ButtonMixin = (superClass) =>

/** @private */
__onInteractionEvent(event) {
if (this.disabled) {
if (this.__shouldSuppressInteractionEvent(event)) {
event.stopImmediatePropagation();
}
}

/**
* Returns whether to suppress interaction events like `click`, `keydown`, etc.
* By default suppresses all interaction events when the button is disabled.
*
* @private
*/
__shouldSuppressInteractionEvent(_event) {
return this.disabled;
}
};
14 changes: 14 additions & 0 deletions packages/menu-bar/src/vaadin-lit-menu-bar-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@ class MenuBarButton extends Button {
super._onKeyDown(event);
this.__triggeredWithActiveKeys = null;
}

/**
* Override method inherited from `ButtonMixin` to allow keyboard navigation with
* arrow keys in the menu bar when the button is focusable in the disabled state.
*
* @override
*/
__shouldSuppressInteractionEvent(event) {
if (event.type === 'keydown' && ['ArrowLeft', 'ArrowRight'].includes(event.key)) {
return false;
}

return super.__shouldSuppressInteractionEvent(event);
}
}

defineCustomElement(MenuBarButton);
14 changes: 14 additions & 0 deletions packages/menu-bar/src/vaadin-menu-bar-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,20 @@ class MenuBarButton extends Button {
super._onKeyDown(event);
this.__triggeredWithActiveKeys = null;
}

/**
* Override method inherited from `ButtonMixin` to allow keyboard navigation with
* arrow keys in the menu bar when the button is focusable in the disabled state.
*
* @override
*/
__shouldSuppressInteractionEvent(event) {
if (event.type === 'keydown' && ['ArrowLeft', 'ArrowRight'].includes(event.key)) {
return false;
}

return super.__shouldSuppressInteractionEvent(event);
}
}

defineCustomElement(MenuBarButton);
20 changes: 20 additions & 0 deletions packages/menu-bar/src/vaadin-menu-bar-mixin.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,26 @@ export declare class MenuBarMixinClass {
* {text: 'Help'}
* ];
* ```
*
* #### Disabled buttons
*
* When a root-level item (menu bar button) is disabled, it prevents all user
* interactions with it, such as focusing, clicking, opening a sub-menu, etc.
* The button is also removed from tab order, which makes it unreachable via
* the keyboard navigation.
*
* While the default behavior effectively prevents accidental interactions,
* it has an accessibility drawback: screen readers skip disabled buttons
* entirely, and users can't see tooltips that might explain why the button
* is disabled. To address this, an experimental enhancement allows disabled
* menu bar buttons to receive focus and show tooltips, while still preventing
* other interactions. This feature can be enabled with the following feature
* flag:
*
* ```
* // Set before any menu bar is attached to the DOM.
* window.Vaadin.featureFlags.accessibleDisabledButtons = true;
* ```
*/
items: MenuBarItem[];

Expand Down
42 changes: 41 additions & 1 deletion packages/menu-bar/src/vaadin-menu-bar-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,26 @@ export const MenuBarMixin = (superClass) =>
* ];
* ```
*
* #### Disabled buttons
*
* When a root-level item (menu bar button) is disabled, it prevents all user
* interactions with it, such as focusing, clicking, opening a sub-menu, etc.
* The button is also removed from tab order, which makes it unreachable via
* the keyboard navigation.
*
* While the default behavior effectively prevents accidental interactions,
* it has an accessibility drawback: screen readers skip disabled buttons
* entirely, and users can't see tooltips that might explain why the button
* is disabled. To address this, an experimental enhancement allows disabled
* menu bar buttons to receive focus and show tooltips, while still preventing
* other interactions. This feature can be enabled with the following feature
* flag:
*
* ```
* // Set before any menu bar is attached to the DOM.
* window.Vaadin.featureFlags.accessibleDisabledButtons = true;
* ```
*
* @type {!Array<!MenuBarItem>}
*/
items: {
Expand Down Expand Up @@ -688,7 +708,7 @@ export const MenuBarMixin = (superClass) =>

/** @protected */
_setTabindex(button, focused) {
if (this.tabNavigation && !button.disabled) {
if (this.tabNavigation && this._isItemFocusable(button)) {
button.setAttribute('tabindex', '0');
} else {
button.setAttribute('tabindex', focused ? '0' : '-1');
Expand Down Expand Up @@ -907,6 +927,10 @@ export const MenuBarMixin = (superClass) =>

/** @private */
__openSubMenu(button, keydown, options = {}) {
if (button.disabled) {
return;
}

const subMenu = this._subMenu;
const item = button.item;

Expand Down Expand Up @@ -1030,4 +1054,20 @@ export const MenuBarMixin = (superClass) =>
close() {
this._close();
}

/**
* Override method inherited from `KeyboardDirectionMixin` to allow
* focusing disabled buttons that are configured so.
*
* @param {Element} button
* @protected
* @override
*/
_isItemFocusable(button) {
if (button.disabled && button.__shouldAllowFocusWhenDisabled) {
return button.__shouldAllowFocusWhenDisabled();
}

return super._isItemFocusable(button);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import './not-animated-styles.js';
import '../vaadin-lit-menu-bar.js';
import './accessible-disabled-buttons.common.js';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import './not-animated-styles.js';
import '../vaadin-menu-bar.js';
import './accessible-disabled-buttons.common.js';
84 changes: 84 additions & 0 deletions packages/menu-bar/test/accessible-disabled-buttons.common.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { expect } from '@vaadin/chai-plugins';
import { fixtureSync, middleOfNode, nextRender } from '@vaadin/testing-helpers';
import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands';

describe('accessible disabled buttons', () => {
let menuBar, buttons;

before(() => {
window.Vaadin.featureFlags ??= {};
window.Vaadin.featureFlags.accessibleDisabledButtons = true;
});

after(() => {
window.Vaadin.featureFlags.accessibleDisabledButtons = false;
});

beforeEach(async () => {
menuBar = fixtureSync('<vaadin-menu-bar></vaadin-menu-bar>');
menuBar.items = [
{ text: 'Item 0' },
{ text: 'Item 1', disabled: true, children: [{ text: 'SubItem 0' }] },
{ text: 'Item 2' },
];
await nextRender(menuBar);
buttons = menuBar._buttons;
});

afterEach(async () => {
await resetMouse();
});

it('should not open sub-menu on disabled button click', async () => {
const { x, y } = middleOfNode(buttons[1]);
await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] });
expect(buttons[1].hasAttribute('expanded')).to.be.false;
});

it('should not open sub-menu on disabled button hover', async () => {
menuBar.openOnHover = true;
const { x, y } = middleOfNode(buttons[1]);
await sendMouse({ type: 'move', position: [Math.floor(x), Math.floor(y)] });
expect(buttons[1].hasAttribute('expanded')).to.be.false;
});

it('should include disabled buttons in arrow key navigation', async () => {
await sendKeys({ press: 'Tab' });
expect(document.activeElement).to.equal(buttons[0]);

await sendKeys({ press: 'ArrowRight' });
expect(document.activeElement).to.equal(buttons[1]);

await sendKeys({ press: 'ArrowRight' });
expect(document.activeElement).to.equal(buttons[2]);

await sendKeys({ press: 'ArrowLeft' });
expect(document.activeElement).to.equal(buttons[1]);

await sendKeys({ press: 'ArrowLeft' });
expect(document.activeElement).to.equal(buttons[0]);
});

it('should include disabled buttons in Tab navigation', async () => {
menuBar.tabNavigation = true;

await sendKeys({ press: 'Tab' });
expect(document.activeElement).to.equal(buttons[0]);

await sendKeys({ press: 'Tab' });
expect(document.activeElement).to.equal(buttons[1]);

await sendKeys({ press: 'Tab' });
expect(document.activeElement).to.equal(buttons[2]);

await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });
expect(document.activeElement).to.equal(buttons[1]);

await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });
expect(document.activeElement).to.equal(buttons[0]);
});
});
Loading

0 comments on commit 360bba2

Please sign in to comment.