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 buttons focusable with feature flag #8513

Merged
merged 22 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
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
21 changes: 20 additions & 1 deletion packages/a11y-base/src/tabindex-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ export const TabindexMixin = (superclass) =>
_disabledChanged(disabled, oldDisabled) {
super._disabledChanged(disabled, oldDisabled);

if (this.__shouldAllowFocusWhenDisabled()) {
return;
}

if (disabled) {
if (this.tabindex !== undefined) {
this._lastTabIndex = this.tabindex;
Expand All @@ -70,6 +74,10 @@ export const TabindexMixin = (superclass) =>
* @protected
*/
_tabindexChanged(tabindex) {
if (this.__shouldAllowFocusWhenDisabled()) {
return;
}

if (this.disabled && tabindex !== -1) {
this._lastTabIndex = tabindex;
this.tabindex = -1;
Expand All @@ -86,8 +94,19 @@ export const TabindexMixin = (superclass) =>
* @override
*/
focus() {
if (!this.disabled) {
if (!this.disabled || this.__shouldAllowFocusWhenDisabled()) {
super.focus();
}
}

/**
* Returns whether the component should be focusable when disabled.
* Returns false by default.
*
* @private
* @return {boolean}
*/
__shouldAllowFocusWhenDisabled() {
return false;
}
};
3 changes: 2 additions & 1 deletion packages/button/src/vaadin-button-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ export const buttonStyles = css`
}

:host([disabled]) {
pointer-events: none;
pointer-events: var(--_vaadin-button-disabled-pointer-events, none);
cursor: not-allowed;
}

/* Aligns the button with form fields when placed on the same line.
Expand Down
4 changes: 4 additions & 0 deletions packages/button/src/vaadin-button-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ export const ButtonMixin = (superClass) =>
if (!this.hasAttribute('role')) {
this.setAttribute('role', 'button');
}

if (this.__shouldAllowFocusWhenDisabled()) {
this.style.setProperty('--_vaadin-button-disabled-pointer-events', 'auto');
}
}

/**
Expand Down
20 changes: 19 additions & 1 deletion packages/button/src/vaadin-button.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,25 @@ import { ButtonMixin } from './vaadin-button-mixin.js';
*
* See [Styling Components](https://vaadin.com/docs/latest/styling/styling-components) documentation.
*/
declare class Button extends ButtonMixin(ElementMixin(ThemableMixin(ControllerMixin(HTMLElement)))) {}
declare class Button extends ButtonMixin(ElementMixin(ThemableMixin(ControllerMixin(HTMLElement)))) {
/**
* When set to true, prevents any user interaction with the button such as
* clicking or hovering, and removes the button from the tab order, which
* makes it inaccessible to screen readers.
*
* To improve accessibility, disabled buttons can be made focusable so that
* screen readers can still reach and properly announce them, including any
* attached tooltips and popovers, while still preventing clicks. This is
* currently available as an experimental enhancement that can be enabled
* with the following feature flag:
*
* ```
* // Set before any button is attached to the DOM.
* window.Vaadin.featureFlags.accessibleDisabledButtons = true;
* ```
*/
disabled: boolean;
}

declare global {
interface HTMLElementTagNameMap {
Expand Down
30 changes: 30 additions & 0 deletions packages/button/src/vaadin-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,31 @@ registerStyles('vaadin-button', buttonStyles, { moduleId: 'vaadin-button-styles'
* @mixes ThemableMixin
*/
class Button extends ButtonMixin(ElementMixin(ThemableMixin(ControllerMixin(PolymerElement)))) {
static get properties() {
return {
/**
* When set to true, prevents any user interaction with the button such as
* clicking or hovering, and removes the button from the tab order, which
* makes it inaccessible to screen readers.
*
* To improve accessibility, disabled buttons can be made focusable so that
* screen readers can still reach and properly announce them, including any
* attached tooltips and popovers, while still preventing clicks. This is
* currently available as an experimental enhancement that can be enabled
* with the following feature flag:
*
* ```
* // Set before any button is attached to the DOM.
* window.Vaadin.featureFlags.accessibleDisabledButtons = true;
* ```
*/
disabled: {
sissbruecker marked this conversation as resolved.
Show resolved Hide resolved
type: Boolean,
value: false,
},
};
}

static get is() {
return 'vaadin-button';
}
Expand All @@ -65,6 +90,11 @@ class Button extends ButtonMixin(ElementMixin(ThemableMixin(ControllerMixin(Poly
this._tooltipController = new TooltipController(this);
this.addController(this._tooltipController);
}

/** @override */
__shouldAllowFocusWhenDisabled() {
return window.Vaadin.featureFlags.accessibleDisabledButtons;
}
}

defineCustomElement(Button);
Expand Down
5 changes: 5 additions & 0 deletions packages/button/src/vaadin-lit-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ class Button extends ButtonMixin(ElementMixin(ThemableMixin(PolylitMixin(LitElem
this._tooltipController = new TooltipController(this);
this.addController(this._tooltipController);
}

/** @override */
__shouldAllowFocusWhenDisabled() {
return window.Vaadin.featureFlags.accessibleDisabledButtons;
}
}

defineCustomElement(Button);
Expand Down
46 changes: 46 additions & 0 deletions packages/button/test/button.common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,50 @@ describe('vaadin-button', () => {
});
});
});

describe('disabled and accessible', () => {
let lastGlobalFocusable: HTMLInputElement;

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

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

beforeEach(async () => {
[button, lastGlobalFocusable] = fixtureSync(
`<div>
<vaadin-button disabled>Press me</vaadin-button>
<input id="last-global-focusable" />
</div>`,
).children as unknown as [Button, HTMLInputElement];
await nextRender(button);
});

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

it('should allow programmatic focus when disabled', () => {
button.focus();
expect(document.activeElement).to.equal(button);
});

it('should allow pointer focus when disabled', async () => {
const { x, y } = middleOfNode(button);
await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] });
expect(document.activeElement).to.equal(button);
});

it('should allow keyboard focus when disabled', async () => {
await sendKeys({ press: 'Tab' });
expect(document.activeElement).to.equal(button);

await sendKeys({ press: 'Tab' });
expect(document.activeElement).to.equal(lastGlobalFocusable);
});
});
});
4 changes: 2 additions & 2 deletions packages/button/theme/lumo/vaadin-button-styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const button = css`
/* Hover */

@media (any-hover: hover) {
:host(:hover)::before {
:host(:not([disabled]):hover)::before {
opacity: 0.02;
}
}
Expand Down Expand Up @@ -159,7 +159,7 @@ const button = css`
}

@media (any-hover: hover) {
:host([theme~='primary']:hover)::before {
:host([theme~='primary']:not([disabled]):hover)::before {
opacity: 0.05;
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/button/theme/material/vaadin-button-styles.js
sissbruecker marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const button = css`
vertical-align: middle;
}

:host(:hover)::before,
:host(:hover:not([disabled]))::before,
:host([focus-ring])::before {
opacity: 0.08;
transition-duration: 0.2s;
Expand All @@ -77,7 +77,7 @@ const button = css`
transition: 0s;
}

:host(:hover:not([active]))::after {
:host(:hover:not([active]):not([disabled]))::after {
transform: translate(-50%, -50%) scale(1);
opacity: 0;
}
Expand Down Expand Up @@ -106,7 +106,7 @@ const button = css`
background-color: var(--material-secondary-background-color);
}

:host([theme~='contained']:hover) {
:host([theme~='contained']:not([disabled]):hover) {
box-shadow: var(--material-shadow-elevation-4dp);
}

Expand Down Expand Up @@ -149,7 +149,7 @@ const button = css`
transform: translate(50%, -50%) scale(0.0000001);
}

:host(:hover:not([active])[dir='rtl'])::after {
:host(:hover:not([active]):not([disabled])[dir='rtl'])::after {
transform: translate(50%, -50%) scale(1);
}

Expand Down
6 changes: 3 additions & 3 deletions packages/vaadin-lumo-styles/mixins/input-field-shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,17 @@ const inputField = css`
}

/* Hover */
:host(:hover:not([readonly]):not([focused])) [part='input-field']::after {
:host(:hover:not([readonly]):not([focused]):not([disabled])) [part='input-field']::after {
opacity: var(--vaadin-input-field-hover-highlight-opacity, 0.1);
}

/* Touch device adjustment */
@media (pointer: coarse) {
:host(:hover:not([readonly]):not([focused])) [part='input-field']::after {
:host(:hover:not([readonly]):not([focused]):not([disabled])) [part='input-field']::after {
opacity: 0;
}

:host(:active:not([readonly]):not([focused])) [part='input-field']::after {
:host(:active:not([readonly]):not([focused]):not([disabled])) [part='input-field']::after {
opacity: 0.2;
}
}
Expand Down
63 changes: 56 additions & 7 deletions test/integration/component-tooltip.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { expect } from '@vaadin/chai-plugins';
import { fixtureSync, nextRender, tabKeyDown } from '@vaadin/testing-helpers';
import { fixtureSync, middleOfNode, nextRender, tabKeyDown } from '@vaadin/testing-helpers';
import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands';
import './not-animated-styles.js';
import { Button } from '@vaadin/button';
import { Checkbox } from '@vaadin/checkbox';
import { CheckboxGroup } from '@vaadin/checkbox-group';
Expand All @@ -25,6 +27,12 @@ import { TimePicker } from '@vaadin/time-picker';
import { Tooltip } from '@vaadin/tooltip';
import { mouseenter, mouseleave } from '@vaadin/tooltip/test/helpers.js';

before(() => {
Tooltip.setDefaultFocusDelay(0);
Tooltip.setDefaultHoverDelay(0);
Tooltip.setDefaultHideDelay(0);
});

[
{ tagName: Button.is },
{ tagName: Checkbox.is, ariaTargetSelector: 'input' },
Expand Down Expand Up @@ -85,12 +93,6 @@ import { mouseenter, mouseleave } from '@vaadin/tooltip/test/helpers.js';
describe(`${tagName} with a slotted tooltip`, () => {
let element, tooltip, tooltipOverlay;

before(() => {
Tooltip.setDefaultFocusDelay(0);
Tooltip.setDefaultHoverDelay(0);
Tooltip.setDefaultHideDelay(0);
});

beforeEach(() => {
element = fixtureSync(`
<${tagName}>
Expand Down Expand Up @@ -199,3 +201,50 @@ import { mouseenter, mouseleave } from '@vaadin/tooltip/test/helpers.js';
});
});
});

describe('accessible disabled button', () => {
let button, tooltip;

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

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

beforeEach(() => {
button = fixtureSync(
`<div>
<vaadin-button disabled>
Press me
<vaadin-tooltip slot="tooltip" text="Tooltip text"></vaadin-tooltip>
</vaadin-button>
<input id="last-global-focusable" />
</div>`,
).firstElementChild;
tooltip = button.querySelector('vaadin-tooltip');
});

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

it('should toggle tooltip on hover when button is disabled', async () => {
const { x, y } = middleOfNode(button);
await sendMouse({ type: 'move', position: [Math.floor(x), Math.floor(y)] });
expect(tooltip._overlayElement.opened).to.be.true;

await sendMouse({ type: 'move', position: [0, 0] });
expect(tooltip._overlayElement.opened).to.be.false;
});

it('should toggle tooltip on focus when button is disabled', async () => {
await sendKeys({ press: 'Tab' });
expect(tooltip._overlayElement.opened).to.be.true;

await sendKeys({ press: 'Tab' });
expect(tooltip._overlayElement.opened).to.be.false;
});
});