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.

Sign up for GitHub

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

fix: introduce disabledInteractive property and revert focusing disabled actions in general #3096

Merged
merged 4 commits into from
Sep 19, 2024
Merged

fix: introduce disabledInteractive property and revert focusing disabled actions in general #3096

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
158ba40
fix: introduce disabledInteractive property and revert focusing disab…
jeripeierSBB Sep 19, 2024
dc4cbfe
docs: fix docs
jeripeierSBB Sep 19, 2024
765dc6e
fix: version 2
jeripeierSBB Sep 19, 2024
0794351
fix: review
jeripeierSBB Sep 19, 2024
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
1 change: 1 addition & 0 deletions src/elements/button/button-link/__snapshots__/button-link.snapshot.spec.snap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ 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
4 changes: 2 additions & 2 deletions src/elements/button/button-link/button-link.ts
View file
Open in desktop
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 { SbbLinkBaseElement } from '../../core/base-elements.js';
import { SbbDisabledMixin } from '../../core/mixins.js';
import { SbbDisabledInteractiveMixin, SbbDisabledMixin } 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-link')
export class SbbButtonLinkElement extends SbbButtonCommonElementMixin(
SbbDisabledMixin(SbbLinkBaseElement),
SbbDisabledMixin(SbbDisabledInteractiveMixin(SbbLinkBaseElement)),
) {
public static override styles: CSSResultGroup = [buttonCommonStyle, buttonPrimaryStyle];
}
Expand Down
23 changes: 12 additions & 11 deletions src/elements/button/button-link/readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,18 @@ Use the accessibility properties in case of an icon-only button to describe the

## Properties

| Name | Attribute | Privacy | Type | Default | Description |
| -------------------- | --------------------- | ------- | --------------------------------------- | ------- | -------------------------------------------------------------------------------------------------------------------------------- |
| `accessibilityLabel` | `accessibility-label` | public | `string \| undefined` | | This will be forwarded as aria-label to the inner anchor element. |
| `disabled` | `disabled` | public | `boolean` | `false` | Whether the component is disabled. |
| `download` | `download` | public | `boolean \| undefined` | | Whether the browser will show the download dialog on click. |
| `href` | `href` | public | `string \| undefined` | | The href value you want to link to. |
| `iconName` | `icon-name` | public | `string \| undefined` | | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch. |
| `negative` | `negative` | public | `boolean` | `false` | Negative coloring variant flag. |
| `rel` | `rel` | public | `string \| undefined` | | The relationship of the linked URL as space-separated link types. |
| `size` | `size` | public | `SbbButtonSize \| undefined` | `'l'` | Size variant, either l or m. |
| `target` | `target` | public | `LinkTargetType \| string \| undefined` | | Where to display the linked URL. |
| Name | Attribute | Privacy | Type | Default | Description |
| --------------------- | ---------------------- | ------- | --------------------------------------- | ------- | -------------------------------------------------------------------------------------------------------------------------------- |
| `accessibilityLabel` | `accessibility-label` | public | `string \| undefined` | | This will be forwarded as aria-label to the inner anchor element. |
| `disabled` | `disabled` | public | `boolean` | `false` | Whether the component is disabled. |
| `disabledInteractive` | `disabled-interactive` | public | `boolean` | `false` | Whether disabled buttons should be interactive. |
| `download` | `download` | public | `boolean \| undefined` | | Whether the browser will show the download dialog on click. |
| `href` | `href` | public | `string \| undefined` | | The href value you want to link to. |
| `iconName` | `icon-name` | public | `string \| undefined` | | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch. |
| `negative` | `negative` | public | `boolean` | `false` | Negative coloring variant flag. |
| `rel` | `rel` | public | `string \| undefined` | | The relationship of the linked URL as space-separated link types. |
| `size` | `size` | public | `SbbButtonSize \| undefined` | `'l'` | Size variant, either l or m. |
| `target` | `target` | public | `LinkTargetType \| string \| undefined` | | Where to display the linked URL. |

## Slots

Expand Down
1 change: 0 additions & 1 deletion src/elements/button/button/__snapshots__/button.snapshot.spec.snap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ 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
View file
Open in desktop
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 { SbbFocusableDisabledActionMixin } from '../../core/mixins.js';
import { SbbDisabledTabIndexActionMixin } 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(
SbbFocusableDisabledActionMixin(SbbButtonBaseElement),
SbbDisabledTabIndexActionMixin(SbbButtonBaseElement),
) {
public static override styles: CSSResultGroup = [buttonCommonStyle, buttonPrimaryStyle];
}
Expand Down
36 changes: 21 additions & 15 deletions src/elements/button/button/readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,28 +62,34 @@ 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
### Interactive disabled button

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.
Native disabled elements cannot receive focus and do not dispatch any events. This can
be problematic in some cases because it can prevent the app from telling the user why the button is
disabled. Consumers can use the `disabledInteractive` property to style the button as disabled but allow for
it to receive focus and dispatch events. The button will have `aria-disabled="true"` for assistive
technology. It is 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.

**Note:** Using the `disabledInteractive` property can result in buttons that previously prevented
actions to no longer do so, for example a submit button in a form. When using this input, you should
guard against such cases in your component.

<!-- Auto Generated Below -->

## Properties

| Name | Attribute | Privacy | Type | Default | Description |
| ---------- | ----------- | ------- | ---------------------------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------- |
| `disabled` | `disabled` | public | `boolean` | `false` | Whether the component is disabled. |
| `form` | `form` | public | `string \| undefined` | | The <form> element to associate the button with. |
| `iconName` | `icon-name` | public | `string \| undefined` | | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch. |
| `name` | `name` | public | `string` | | The name of the button element. |
| `negative` | `negative` | public | `boolean` | `false` | Negative coloring variant flag. |
| `size` | `size` | public | `SbbButtonSize \| undefined` | `'l'` | Size variant, either l or m. |
| `type` | `type` | public | `SbbButtonType` | `'button'` | The type attribute to use for the button. |
| `value` | `value` | public | `string` | | The value of the button element. |
| Name | Attribute | Privacy | Type | Default | Description |
| --------------------- | ---------------------- | ------- | ---------------------------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------- |
| `disabled` | `disabled` | public | `boolean` | `false` | Whether the component is disabled. |
| `disabledInteractive` | `disabled-interactive` | public | `boolean` | `false` | Whether disabled buttons should be interactive. |
| `form` | `form` | public | `string \| undefined` | | The <form> element to associate the button with. |
| `iconName` | `icon-name` | public | `string \| undefined` | | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch. |
| `name` | `name` | public | `string` | | The name of the button element. |
| `negative` | `negative` | public | `boolean` | `false` | Negative coloring variant flag. |
| `size` | `size` | public | `SbbButtonSize \| undefined` | `'l'` | Size variant, either l or m. |
| `type` | `type` | public | `SbbButtonType` | `'button'` | The type attribute to use for the button. |
| `value` | `value` | public | `string` | | The value of the button element. |

## Slots

Expand Down
11 changes: 11 additions & 0 deletions src/elements/button/common/button-common-stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ const disabled: InputType = {
},
};

const disabledInteractive: InputType = {
control: {
type: 'boolean',
},
table: {
category: 'Button',
},
};

const name: InputType = {
control: {
type: 'text',
Expand Down Expand Up @@ -76,6 +85,7 @@ export const buttonDefaultArgTypes: ArgTypes = {
...commonDefaultArgTypes,
type,
disabled,
'disabled-interactive': disabledInteractive,
name,
value,
form,
Expand All @@ -86,6 +96,7 @@ export const buttonDefaultArgs: Args = {
...commonDefaultArgs,
type: type.options![0],
disabled: false,
'disabled-interactive': false,
name: 'Button Name',
value: undefined,
form: undefined,
Expand Down
11 changes: 11 additions & 0 deletions src/elements/button/common/button-link-common-stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@ const disabled: InputType = {
},
};

const disabledInteractive: InputType = {
control: {
type: 'boolean',
},
table: {
category: 'Button',
},
};

const accessibilityLabel: InputType = {
control: {
type: 'text',
Expand All @@ -68,6 +77,7 @@ export const buttonLinkDefaultArgTypes: ArgTypes = {
rel,
download,
disabled,
disabledInteractive: disabledInteractive,
'accessibility-label': accessibilityLabel,
};

Expand All @@ -78,5 +88,6 @@ export const buttonLinkDefaultArgs: Args = {
rel: 'noopener',
download: false,
disabled: false,
'disabled-interactive': false,
'accessibility-label': undefined,
};
6 changes: 2 additions & 4 deletions src/elements/button/mini-button/mini-button.ts
View file
Open in desktop
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 { SbbFocusableDisabledActionMixin } from '../../core/mixins.js';
import { SbbDisabledTabIndexActionMixin } from '../../core/mixins.js';

import { SbbMiniButtonBaseElement } from './mini-button-base-element.js';
import style from './mini-button.scss?lit&inline';
Expand All @@ -13,9 +13,7 @@ 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 SbbFocusableDisabledActionMixin(
SbbMiniButtonBaseElement,
) {
export class SbbMiniButtonElement extends SbbDisabledTabIndexActionMixin(SbbMiniButtonBaseElement) {
public static override styles: CSSResultGroup = style;
}

Expand Down
Loading
Loading