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

Fix/1561 switch is not correctly marked for screen reader #1633

Merged
5 changes: 5 additions & 0 deletions .changeset/curly-windows-yell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sl-design-system/form': patch
---

Fix of `aria-describedby` in form field when there is a hint and validation message.
5 changes: 5 additions & 0 deletions .changeset/metal-lamps-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sl-design-system/switch': patch
---

Fix NVDA issues, using input native element as a form control, moving aria attributes to the right control to solve accessibility issues.
7 changes: 7 additions & 0 deletions .changeset/twenty-bags-hide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@sl-design-system/text-field': patch
'@sl-design-system/text-area': patch
'@sl-design-system/checkbox': patch
---

Fix NVDA accessibility issues: moving aria attributes to the right control.
12 changes: 12 additions & 0 deletions packages/components/checkbox/src/checkbox-group.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,18 @@ export const WithoutValues: Story = {
}
};

export const NoLabel: Story = {
render: () => {
return html`
<sl-checkbox-group aria-label="Choose at least one option">
<sl-checkbox value="1">One</sl-checkbox>
<sl-checkbox value="2">Two</sl-checkbox>
<sl-checkbox value="3">Three</sl-checkbox>
</sl-checkbox-group>
`;
}
};

export const CustomValidity: Story = {
args: {
hint: 'This story has both builtin validation (required) and custom validation. You need to select the middle option to make the field valid. The custom validation is done by listening to the sl-validate event and setting the custom validity on the checkbox group.',
Expand Down
23 changes: 20 additions & 3 deletions packages/components/checkbox/src/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe('sl-checkbox', () => {

it('should be touched after losing focus', async () => {
el.focus();
input.blur();
el.blur();

await new Promise(resolve => setTimeout(resolve));

Expand All @@ -135,7 +135,7 @@ describe('sl-checkbox', () => {

el.addEventListener('sl-update-state', onUpdateState);
el.focus();
input.blur();
el.blur();

await new Promise(resolve => setTimeout(resolve));

Expand Down Expand Up @@ -231,7 +231,7 @@ describe('sl-checkbox', () => {

el.addEventListener('sl-blur', onBlur);
el.focus();
input.blur();
el.blur();

expect(onBlur).to.have.been.calledOnce;
});
Expand Down Expand Up @@ -295,6 +295,23 @@ describe('sl-checkbox', () => {
});
});

describe('aria attributes', () => {
beforeEach(async () => {
el = await fixture(html`<sl-checkbox aria-label="my checkbox label" aria-disabled="true"></sl-checkbox>`);
input = el.querySelector('input')!;

// Give time to rewrite arias
await new Promise(resolve => setTimeout(resolve, 100));
});

it('should have an input with proper arias', () => {
expect(el).not.to.have.attribute('aria-label', 'my checkbox label');
expect(el).not.to.have.attribute('aria-disabled', 'true');
expect(input).to.have.attribute('aria-label', 'my checkbox label');
expect(input).to.have.attribute('aria-disabled', 'true');
});
});

describe('validation', () => {
beforeEach(async () => {
el = await fixture(html`<sl-checkbox>Hello world</sl-checkbox>`);
Expand Down
26 changes: 26 additions & 0 deletions packages/components/checkbox/src/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ let nextUniqueId = 0;
*/
@localized()
export class Checkbox<T = unknown> extends FormControlMixin(LitElement) {
/** @internal */
static override get observedAttributes(): string[] {
return [...super.observedAttributes, 'aria-disabled', 'aria-label', 'aria-labelledby'];
}

/** @internal */
static override shadowRootOptions: ShadowRootInit = { ...LitElement.shadowRootOptions, delegatesFocus: true };

Expand Down Expand Up @@ -119,6 +124,23 @@ export class Checkbox<T = unknown> extends FormControlMixin(LitElement) {
this.#onLabelSlotChange();
}

override attributeChangedCallback(name: string, oldValue: string | null, newValue: string | null) {
super.attributeChangedCallback(name, oldValue, newValue);

requestAnimationFrame(() => {
const attributes = ['aria-disabled', 'aria-label', 'aria-labelledby'];
if (this.input && attributes.includes(name)) {
attributes.forEach(attr => {
const value = this.getAttribute(attr);
if (value !== null) {
this.input.setAttribute(attr, value);
this.removeAttribute(attr);
}
});
}
});
}

override updated(changes: PropertyValues<this>): void {
super.updated(changes);

Expand Down Expand Up @@ -166,6 +188,10 @@ export class Checkbox<T = unknown> extends FormControlMixin(LitElement) {
this.input.focus();
}

override blur(): void {
this.input.blur();
}

override getLocalizedValidationMessage(): string {
if (!this.validity.customError && this.validity.valueMissing) {
return msg('Please check this box.');
Expand Down
47 changes: 42 additions & 5 deletions packages/components/form/src/form-field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,14 @@ export class FormField extends ScopedElementsMixin(LitElement) {
this.append(this.#error);
}
} else {
const describedby = this.control?.formControlElement.getAttribute('aria-describedby');
if (describedby) {
const ids = describedby.split(' ').filter(id => id !== this.#error!.id);
this.control?.formControlElement.setAttribute('aria-describedby', ids.join(' '));
}

this.#error?.remove();
this.#error = undefined;
this.control?.formControlElement.removeAttribute('aria-describedby');
}
}
}
Expand All @@ -138,9 +143,14 @@ export class FormField extends ScopedElementsMixin(LitElement) {
this.append(this.#hint);
}
} else {
const describedby = this.control?.formControlElement.getAttribute('aria-describedby');
if (describedby) {
const ids = describedby.split(' ').filter(id => id !== this.#hint!.id);
this.control?.formControlElement.setAttribute('aria-describedby', ids.join(' '));
}

this.#hint?.remove();
this.#hint = undefined;
this.control?.formControlElement.removeAttribute('aria-describedby');
}
}

Expand Down Expand Up @@ -185,7 +195,16 @@ export class FormField extends ScopedElementsMixin(LitElement) {
this.#error.id ||= `sl-form-field-error-${nextUniqueId++}`;

if (this.control) {
this.control.formControlElement.setAttribute('aria-describedby', this.#error.id);
const describedby = this.control.formControlElement.getAttribute('aria-describedby');
if (describedby) {
const ids = describedby.split(' ');
if (!ids.includes(this.#error.id)) {
ids.push(this.#error.id);
this.control.formControlElement.setAttribute('aria-describedby', ids.join(' '));
}
} else {
this.control.formControlElement.setAttribute('aria-describedby', this.#error.id);
}
}
} else {
this.#label = undefined;
Expand All @@ -204,7 +223,16 @@ export class FormField extends ScopedElementsMixin(LitElement) {
this.#hint.id ||= `sl-form-field-hint-${nextUniqueId++}`;

if (this.control) {
this.control.formControlElement.setAttribute('aria-describedby', this.#hint.id);
const describedby = this.control.formControlElement.getAttribute('aria-describedby');
if (describedby) {
const ids = describedby.split(' ');
if (!ids.includes(this.#hint.id)) {
ids.push(this.#hint.id);
this.control.formControlElement.setAttribute('aria-describedby', ids.join(' '));
}
} else {
this.control.formControlElement.setAttribute('aria-describedby', this.#hint.id);
}
}
} else {
this.#label = undefined;
Expand Down Expand Up @@ -250,7 +278,16 @@ export class FormField extends ScopedElementsMixin(LitElement) {
}

if (this.#hint) {
this.control.formControlElement.setAttribute('aria-describedby', this.#hint.id);
const describedby = this.control.formControlElement.getAttribute('aria-describedby');
if (describedby) {
const ids = describedby.split(' ');
if (!ids.includes(this.#hint.id)) {
ids.push(this.#hint.id);
this.control.formControlElement.setAttribute('aria-describedby', ids.join(' '));
}
} else {
this.control.formControlElement.setAttribute('aria-describedby', this.#hint.id);
}
}

if (this.#label) {
Expand Down
1 change: 0 additions & 1 deletion packages/components/paginator/src/paginator.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@ export const WithDataSource: Story = {
}

override render(): TemplateResult {
console.log('datasource in example', this.dataSource);
return html`
<div class="pagination">
<sl-paginator-status .dataSource=${this.dataSource}></sl-paginator-status>
Expand Down
27 changes: 23 additions & 4 deletions packages/components/switch/src/switch.scss
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,27 @@ $sizes: sm, md, lg;
order: -1;
}

:host(:focus-within) .track:focus-visible {
:host([no-label]) {
.label {
display: none;
}
}

[part='track']:focus-visible {
outline: var(--_focus-outline);
outline-offset: var(--_focus-outline-offset);
}

::slotted(input) {
block-size: 1px;
inline-size: 1px;
opacity: 0;
outline: 0;
pointer-events: none;
position: absolute;
}

::slotted(label) {
cursor: pointer;
}

slot {
Expand All @@ -98,14 +116,15 @@ slot {
padding: var(--_toggle-container-padding);
}

.track {
[part='track'] {
background-color: var(--_main-background-color);
border-radius: var(--_track-radius);
box-shadow: var(--_box-shadow);
display: grid;
inline-size: var(--_toggle-width);
outline: none;
outline-color: transparent;
outline-offset: var(--_focus-outline-offset);
padding: var(--_toggle-padding);

@media (prefers-reduced-motion: no-preference) {
Expand All @@ -114,7 +133,7 @@ slot {
}
}

.track div {
[part='track'] div {
background-color: var(--_handle-color);
block-size: var(--_handle-size);
border-radius: calc(var(--_handle-size) / 2);
Expand Down
Loading
Loading