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: keep action elements focusable when disabled #3040

Merged
merged 18 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ snapshots["sbb-button-link renders a disabled sbb-button-link with slotted icon
aria-disabled="true"
class="sbb-action-base sbb-button-link"
href="https://www.sbb.ch"
tabindex="-1"
>
<slot name="icon">
</slot>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ snapshots["sbb-button renders a sbb-button without icon DOM"] =
negative=""
role="button"
size="m"
tabindex="0"
type="button"
value="value"
>
Expand Down
4 changes: 2 additions & 2 deletions src/elements/button/button/button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { CSSResultGroup } from 'lit';
import { customElement } from 'lit/decorators.js';

import { SbbButtonBaseElement } from '../../core/base-elements.js';
import { SbbDisabledTabIndexActionMixin } from '../../core/mixins.js';
import { SbbFocusableDisabledActionMixin } from '../../core/mixins.js';
import { buttonCommonStyle, buttonPrimaryStyle, SbbButtonCommonElementMixin } from '../common.js';

/**
Expand All @@ -13,7 +13,7 @@ import { buttonCommonStyle, buttonPrimaryStyle, SbbButtonCommonElementMixin } fr
*/
@customElement('sbb-button')
export class SbbButtonElement extends SbbButtonCommonElementMixin(
SbbDisabledTabIndexActionMixin(SbbButtonBaseElement),
SbbFocusableDisabledActionMixin(SbbButtonBaseElement),
) {
public static override styles: CSSResultGroup = [buttonCommonStyle, buttonPrimaryStyle];
}
Expand Down
8 changes: 8 additions & 0 deletions src/elements/button/button/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ sbb-button {

Use the accessibility properties in case of an icon-only button to describe the purpose of the `sbb-button` for screen-reader users.

### Disabled buttons

Generally speaking, `disabled` elements are considered a bad pattern for accessibility. They are invisible to assistive
technology and do not provide the reason for which they are disabled.
To partially address the problem, disabled elements are kept focusable (other interactions are still prevented).
However, it is still the consumers responsibility to provide a reason for the element being disabled.
This can be achieved by adding an `aria-label`, `aria-labelledby` or `aria-describedby` attribute.

<!-- Auto Generated Below -->

## Properties
Expand Down
6 changes: 4 additions & 2 deletions src/elements/button/mini-button/mini-button.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { CSSResultGroup } from 'lit';
import { customElement } from 'lit/decorators.js';

import { SbbDisabledTabIndexActionMixin } from '../../core/mixins.js';
import { SbbFocusableDisabledActionMixin } from '../../core/mixins.js';

import { SbbMiniButtonBaseElement } from './mini-button-base-element.js';
import style from './mini-button.scss?lit&inline';
Expand All @@ -13,7 +13,9 @@ import style from './mini-button.scss?lit&inline';
* @slot icon - Slot used to display the icon, if one is set
*/
@customElement('sbb-mini-button')
export class SbbMiniButtonElement extends SbbDisabledTabIndexActionMixin(SbbMiniButtonBaseElement) {
export class SbbMiniButtonElement extends SbbFocusableDisabledActionMixin(
SbbMiniButtonBaseElement,
) {
public static override styles: CSSResultGroup = style;
}

Expand Down
8 changes: 8 additions & 0 deletions src/elements/button/mini-button/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ sbb-mini-button {

Use the accessibility properties to describe the purpose of the `sbb-mini-button` for screen-reader users.

### Disabled buttons

Generally speaking, `disabled` elements are considered a bad pattern for accessibility. They are invisible to assistive
technology and do not provide the reason for which they are disabled.
To partially address the problem, disabled elements are kept focusable (other interactions are still prevented).
However, it is still the consumers responsibility to provide a reason for the element being disabled.
This can be achieved by adding an `aria-label`, `aria-labelledby` or `aria-describedby` attribute.

<!-- Auto Generated Below -->

## Properties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ snapshots["sbb-secondary-button-link renders a disabled sbb-secondary-button-lin
aria-disabled="true"
class="sbb-action-base sbb-secondary-button-link"
href="https://www.sbb.ch"
tabindex="-1"
>
<slot name="icon">
</slot>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ snapshots["sbb-secondary-button renders a sbb-secondary-button without icon DOM"
negative=""
role="button"
size="m"
tabindex="0"
type="button"
value="value"
>
Expand Down
8 changes: 8 additions & 0 deletions src/elements/button/secondary-button/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ sbb-secondary-button {

Use the accessibility properties in case of an icon-only button to describe the purpose of the `sbb-secondary-button` for screen-reader users.

### Disabled buttons

Generally speaking, `disabled` elements are considered a bad pattern for accessibility. They are invisible to assistive
technology and do not provide the reason for which they are disabled.
To partially address the problem, disabled elements are kept focusable (other interactions are still prevented).
However, it is still the consumers responsibility to provide a reason for the element being disabled.
This can be achieved by adding an `aria-label`, `aria-labelledby` or `aria-describedby` attribute.

<!-- Auto Generated Below -->

## Properties
Expand Down
4 changes: 2 additions & 2 deletions src/elements/button/secondary-button/secondary-button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { CSSResultGroup } from 'lit';
import { customElement } from 'lit/decorators.js';

import { SbbButtonBaseElement } from '../../core/base-elements.js';
import { SbbDisabledTabIndexActionMixin } from '../../core/mixins.js';
import { SbbFocusableDisabledActionMixin } from '../../core/mixins.js';
import { buttonCommonStyle, buttonSecondaryStyle, SbbButtonCommonElementMixin } from '../common.js';

/**
Expand All @@ -13,7 +13,7 @@ import { buttonCommonStyle, buttonSecondaryStyle, SbbButtonCommonElementMixin }
*/
@customElement('sbb-secondary-button')
export class SbbSecondaryButtonElement extends SbbButtonCommonElementMixin(
SbbDisabledTabIndexActionMixin(SbbButtonBaseElement),
SbbFocusableDisabledActionMixin(SbbButtonBaseElement),
) {
public static override styles: CSSResultGroup = [buttonCommonStyle, buttonSecondaryStyle];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ snapshots["sbb-tertiary-button-link renders a disabled sbb-tertiary-button-link
aria-disabled="true"
class="sbb-action-base sbb-tertiary-button-link"
href="https://www.sbb.ch"
tabindex="-1"
>
<slot name="icon">
</slot>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ snapshots["sbb-tertiary-button renders a sbb-tertiary-button without icon DOM"]
negative=""
role="button"
size="m"
tabindex="0"
type="button"
value="value"
>
Expand Down
8 changes: 8 additions & 0 deletions src/elements/button/tertiary-button/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ sbb-tertiary-button {

Use the accessibility properties in case of an icon-only button to describe the purpose of the `sbb-tertiary-button` for screen-reader users.

### Disabled buttons

Generally speaking, `disabled` elements are considered a bad pattern for accessibility. They are invisible to assistive
technology and do not provide the reason for which they are disabled.
To partially address the problem, disabled elements are kept focusable (other interactions are still prevented).
However, it is still the consumers responsibility to provide a reason for the element being disabled.
This can be achieved by adding an `aria-label`, `aria-labelledby` or `aria-describedby` attribute.

<!-- Auto Generated Below -->

## Properties
Expand Down
4 changes: 2 additions & 2 deletions src/elements/button/tertiary-button/tertiary-button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { CSSResultGroup } from 'lit';
import { customElement } from 'lit/decorators.js';

import { SbbButtonBaseElement } from '../../core/base-elements.js';
import { SbbDisabledTabIndexActionMixin } from '../../core/mixins.js';
import { SbbFocusableDisabledActionMixin } from '../../core/mixins.js';
import { buttonCommonStyle, buttonTertiaryStyle, SbbButtonCommonElementMixin } from '../common.js';

/**
Expand All @@ -13,7 +13,7 @@ import { buttonCommonStyle, buttonTertiaryStyle, SbbButtonCommonElementMixin } f
*/
@customElement('sbb-tertiary-button')
export class SbbTertiaryButtonElement extends SbbButtonCommonElementMixin(
SbbDisabledTabIndexActionMixin(SbbButtonBaseElement),
SbbFocusableDisabledActionMixin(SbbButtonBaseElement),
) {
public static override styles: CSSResultGroup = [buttonCommonStyle, buttonTertiaryStyle];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ snapshots["sbb-transparent-button-link renders a disabled sbb-transparent-button
aria-disabled="true"
class="sbb-action-base sbb-transparent-button-link"
href="https://www.sbb.ch"
tabindex="-1"
>
<slot name="icon">
</slot>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ snapshots["sbb-transparent-button renders a sbb-transparent-button without icon
negative=""
role="button"
size="m"
tabindex="0"
type="button"
value="value"
>
Expand Down
8 changes: 8 additions & 0 deletions src/elements/button/transparent-button/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ sbb-transparent-button {

Use the accessibility properties in case of an icon-only button to describe the purpose of the `sbb-transparent-button` for screen-reader users.

### Disabled buttons

Generally speaking, `disabled` elements are considered a bad pattern for accessibility. They are invisible to assistive
technology and do not provide the reason for which they are disabled.
To partially address the problem, disabled elements are kept focusable (other interactions are still prevented).
However, it is still the consumers responsibility to provide a reason for the element being disabled.
This can be achieved by adding an `aria-label`, `aria-labelledby` or `aria-describedby` attribute.

<!-- Auto Generated Below -->

## Properties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { CSSResultGroup } from 'lit';
import { customElement } from 'lit/decorators.js';

import { SbbButtonBaseElement } from '../../core/base-elements.js';
import { SbbDisabledTabIndexActionMixin } from '../../core/mixins.js';
import { SbbFocusableDisabledActionMixin } from '../../core/mixins.js';
import {
buttonCommonStyle,
buttonTransparentStyle,
Expand All @@ -17,7 +17,7 @@ import {
*/
@customElement('sbb-transparent-button')
export class SbbTransparentButtonElement extends SbbButtonCommonElementMixin(
SbbDisabledTabIndexActionMixin(SbbButtonBaseElement),
SbbFocusableDisabledActionMixin(SbbButtonBaseElement),
) {
public static override styles: CSSResultGroup = [buttonCommonStyle, buttonTransparentStyle];
}
Expand Down
1 change: 0 additions & 1 deletion src/elements/core/base-elements/link-base-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ export abstract class SbbLinkBaseElement extends SbbActionBaseElement {
target=${this.target || nothing}
rel=${this._evaluateRelAttribute()}
aria-label=${this.accessibilityLabel || nothing}
tabindex=${this.maybeDisabled ? '-1' : nothing}
aria-disabled=${this.maybeDisabled ? 'true' : nothing}
>
${this.renderTemplate()}
Expand Down
32 changes: 32 additions & 0 deletions src/elements/core/mixins/disabled-mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ export const SbbDisabledMixin = <T extends AbstractConstructor<LitElement>>(
return SbbDisabledElement as unknown as AbstractConstructor<SbbDisabledMixinType> & T;
};

/**
* @deprecated Will be removed with next major version
*/
// eslint-disable-next-line @typescript-eslint/naming-convention
export const SbbDisabledTabIndexActionMixin = <T extends AbstractConstructor<LitElement>>(
superClass: T,
Expand Down Expand Up @@ -69,3 +72,32 @@ export const SbbDisabledTabIndexActionMixin = <T extends AbstractConstructor<Lit
}
return SbbDisabledTabIndexAction as AbstractConstructor<SbbDisabledMixinType> & T;
};

/**
* Extends `SbbDisabledMixin` with the `aria-disabled` attribute.
* For a11y purposes, keeps the element focusable even when disabled.
*/
// eslint-disable-next-line @typescript-eslint/naming-convention
export const SbbFocusableDisabledActionMixin = <T extends AbstractConstructor<LitElement>>(
superClass: T,
): AbstractConstructor<SbbDisabledMixinType> & T => {
abstract class SbbFocusableDisabledAction
extends SbbDisabledMixin(superClass)
implements SbbDisabledMixinType
{
protected override willUpdate(changedProperties: PropertyValues<this>): void {
super.willUpdate(changedProperties);

if (!changedProperties.has('disabled')) {
return;
}

if (this.disabled) {
this.setAttribute('aria-disabled', 'true');
} else {
this.removeAttribute('aria-disabled');
}
}
}
return SbbFocusableDisabledAction as AbstractConstructor<SbbDisabledMixinType> & T;
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,22 @@ snapshots["sbb-datepicker-toggle renders DOM"] =
/* end snapshot sbb-datepicker-toggle renders DOM */

snapshots["sbb-datepicker-toggle renders Shadow DOM"] =
`<sbb-popover-trigger
`<sbb-mini-button
aria-controls="sbb-popover-2"
aria-disabled="true"
aria-expanded="false"
aria-haspopup="dialog"
aria-label="Show calendar"
class="sbb-datepicker-toggle__trigger"
data-action=""
data-button=""
data-icon-small=""
dir="ltr"
disabled=""
icon-name="calendar-small"
role="button"
tabindex="-1"
>
</sbb-popover-trigger>
</sbb-mini-button>
<sbb-popover
data-state="closed"
hide-close-button=""
Expand All @@ -41,20 +42,20 @@ snapshots["sbb-datepicker-toggle in form-field renders DOM"] =
/* end snapshot sbb-datepicker-toggle in form-field renders DOM */

snapshots["sbb-datepicker-toggle in form-field renders Shadow DOM"] =
`<sbb-popover-trigger
`<sbb-mini-button
aria-controls="sbb-popover-4"
aria-expanded="false"
aria-haspopup="dialog"
aria-label="Show calendar"
class="sbb-datepicker-toggle__trigger"
data-action=""
data-button=""
data-icon-small=""
dir="ltr"
icon-name="calendar-small"
role="button"
tabindex="0"
>
</sbb-popover-trigger>
</sbb-mini-button>
<sbb-popover
data-state="closed"
hide-close-button=""
Expand All @@ -73,21 +74,22 @@ snapshots["sbb-datepicker-toggle in form-field renders disabled DOM"] =
/* end snapshot sbb-datepicker-toggle in form-field renders disabled DOM */

snapshots["sbb-datepicker-toggle in form-field renders disabled Shadow DOM"] =
`<sbb-popover-trigger
`<sbb-mini-button
aria-controls="sbb-popover-6"
aria-disabled="true"
aria-expanded="false"
aria-haspopup="dialog"
aria-label="Show calendar"
class="sbb-datepicker-toggle__trigger"
data-action=""
data-button=""
data-icon-small=""
dir="ltr"
disabled=""
icon-name="calendar-small"
role="button"
tabindex="-1"
>
</sbb-popover-trigger>
</sbb-mini-button>
<sbb-popover
data-state="closed"
hide-close-button=""
Expand All @@ -106,20 +108,20 @@ snapshots["sbb-datepicker-toggle in form-field with calendar parameters DOM"] =
/* end snapshot sbb-datepicker-toggle in form-field with calendar parameters DOM */

snapshots["sbb-datepicker-toggle in form-field with calendar parameters Shadow DOM"] =
`<sbb-popover-trigger
`<sbb-mini-button
aria-controls="sbb-popover-8"
aria-expanded="false"
aria-haspopup="dialog"
aria-label="Show calendar"
class="sbb-datepicker-toggle__trigger"
data-action=""
data-button=""
data-icon-small=""
dir="ltr"
icon-name="calendar-small"
role="button"
tabindex="0"
>
</sbb-popover-trigger>
</sbb-mini-button>
<sbb-popover
data-state="closed"
hide-close-button=""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
display: inline-flex;
}

sbb-popover-trigger {
.sbb-datepicker-toggle__trigger {
color: var(--sbb-datepicker-control-color);

// Enables taking full height in order to shift the calendar position to the border bottom of the form field.
Expand Down
Loading
Loading