Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make disabled menu bar buttons accessible with feature flag #8518

Merged
merged 41 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
6fa1936
feat: make disabled buttons focusable behind feature flag
vursen Jan 15, 2025
9aeaf0c
skip failing crud test
vursen Jan 15, 2025
82509ff
combine CSS selectors
vursen Jan 16, 2025
35f06ee
update Lumo and Material to not show hover effect when disabled
vursen Jan 16, 2025
7088f60
set pointer-events via CSS variable
vursen Jan 16, 2025
2bfe45c
add more tests
vursen Jan 16, 2025
010a38e
set cursor not-allowed when disabled
vursen Jan 16, 2025
51648a2
explain how to enable feature flag
vursen Jan 16, 2025
1610766
polish JSDoc
vursen Jan 17, 2025
6bc5144
remove Vaadin 25 from JSDoc
vursen Jan 17, 2025
48f3d25
rename feature flag, update JSDoc
vursen Jan 17, 2025
4ae6962
polish JSDoc
vursen Jan 17, 2025
4ddfd49
polish JSDoc
vursen Jan 20, 2025
3036120
revert accidental change
vursen Jan 20, 2025
1c648b8
add protected methods to d.ts
vursen Jan 20, 2025
e06edaf
update the list of interaction events
vursen Jan 20, 2025
27be0de
fix: disable hover effect for contained theme
vursen Jan 20, 2025
3fbaae9
Merge remote-tracking branch 'origin/main' into feat/focusable-disabl…
vursen Jan 20, 2025
ea9d79d
reduce visibility of methods
vursen Jan 20, 2025
8139351
remove irrelevant changes
vursen Jan 20, 2025
67f123e
Merge remote-tracking branch 'origin/main' into feat/focusable-disabl…
vursen Jan 21, 2025
f32efdd
improve JavaDoc to better highlight tooltip case
vursen Jan 21, 2025
754735e
feat: make disabled menu bar buttons focusable with feature flag
vursen Jan 16, 2025
daf0591
polish JSDoc
vursen Jan 16, 2025
e92344b
minimize diff
vursen Jan 16, 2025
295b304
add some tests
vursen Jan 16, 2025
4250ff5
prevent sub menu from opening on hover
vursen Jan 16, 2025
f6e8e8b
revert dev pages
vursen Jan 16, 2025
bad7981
add integration test with tooltip
vursen Jan 16, 2025
8620d25
simplify import path
vursen Jan 17, 2025
3c2d798
explain how to make disabled buttons focusable
vursen Jan 17, 2025
55b053a
polish JSDoc
vursen Jan 17, 2025
8ea6437
remove Vaadin 25 from JSDoc
vursen Jan 17, 2025
871c822
rename feature flag, update JSDoc
vursen Jan 17, 2025
52b8185
polish JSDoc
vursen Jan 20, 2025
eaa6ca4
add _isItemFocusable to d.ts
vursen Jan 20, 2025
bbb77ed
do not suppress ArrowLeft, ArrowRight to allow keyboard navigation
vursen Jan 20, 2025
83b1731
improve JSDoc to better highlight tooltip case
vursen Jan 21, 2025
dc5681f
refactor: move not-animated-styles before menu-bar import
vursen Jan 21, 2025
c0d0aea
Merge remote-tracking branch 'origin/main' into feat/focusable-disabl…
vursen Jan 21, 2025
ca8c76f
use accessible term instead of focusable
vursen Jan 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
}
};
3 changes: 3 additions & 0 deletions packages/menu-bar/test/focusable-disabled-buttons-lit.test.js
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 './focusable-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 './focusable-disabled-buttons.common.js';
84 changes: 84 additions & 0 deletions packages/menu-bar/test/focusable-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('focusable 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
Loading