Skip to content

Commit

Permalink
fix(picker): correct label application for screen readers
Browse files Browse the repository at this point in the history
  • Loading branch information
Westbrook committed Jun 30, 2023
1 parent 98d0370 commit 8ce0cb0
Show file tree
Hide file tree
Showing 5 changed files with 298 additions and 58 deletions.
46 changes: 26 additions & 20 deletions packages/field-label/src/FieldLabel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ import {
import styles from './field-label.css.js';

type AcceptsFocusVisisble = HTMLElement & { forceFocusVisible?(): void };
type Labelable = Focusable & {
applyFocusElementLabel?: (label?: string) => void;
};

/**
* @element sp-field-label
Expand Down Expand Up @@ -71,7 +74,7 @@ export class FieldLabel extends SizedMixin(SpectrumElement) {
@property({ type: String, reflect: true, attribute: 'side-aligned' })
public sideAligned?: 'start' | 'end';

private target?: HTMLElement;
private target?: Labelable;

private handleClick(event: Event): void {
if (!this.target || this.disabled || event.defaultPrevented) return;
Expand All @@ -89,31 +92,34 @@ export class FieldLabel extends SizedMixin(SpectrumElement) {

private resolvedElement = new ElementResolutionController(this);

private addTarget(target: Focusable): void {
this.target = target.focusElement || target;
const targetParent = this.target.getRootNode() as HTMLElement;
if (targetParent === (this.getRootNode() as HTMLElement)) {
conditionAttributeWithId(this.target, 'aria-labelledby', [this.id]);
} else {
this.target.setAttribute('aria-label', this.labelText);
}
}

private removeTarget(): void {
private applyTargetLabel(target?: Labelable): void {
// Apply new target when provided
this.target = target || this.target;
if (this.target) {
const targetParent = this.target.getRootNode() as HTMLElement;
if (targetParent === (this.getRootNode() as HTMLElement)) {
conditionAttributeWithoutId(this.target, 'aria-labelledby', [
this.id,
]);
// When target is available add or remove label information
// depending on the value of `apply`.
const applyLabel = this.target.applyFocusElementLabel;
const focusable = this.target.focusElement || this.target;
const targetParent = focusable.getRootNode() as HTMLElement;
if (typeof applyLabel !== 'undefined') {
applyLabel(this.labelText);
} else if (targetParent === (this.getRootNode() as HTMLElement)) {
const conditionAttribute = target
? conditionAttributeWithId
: conditionAttributeWithoutId;
conditionAttribute(focusable, 'aria-labelledby', [this.id]);
} else {
this.target.removeAttribute('aria-label');
if (target) {
focusable.setAttribute('aria-label', this.labelText);
} else {
focusable.removeAttribute('aria-label');
}
}
}
}

private async manageTarget(): Promise<void> {
this.removeTarget();
this.applyTargetLabel();
const target = this.resolvedElement.element as Focusable;
if (!target) {
this.target = target;
Expand All @@ -125,7 +131,7 @@ export class FieldLabel extends SizedMixin(SpectrumElement) {
if (typeof target.updateComplete !== 'undefined') {
await target.updateComplete;
}
this.addTarget(target);
this.applyTargetLabel(target);
}

private get labelText(): string {
Expand Down
67 changes: 54 additions & 13 deletions packages/picker/src/Picker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@ import {
SizedMixin,
TemplateResult,
} from '@spectrum-web-components/base';
import { classMap } from '@spectrum-web-components/base/src/directives.js';
import {
classMap,
ifDefined,
} from '@spectrum-web-components/base/src/directives.js';
import {
property,
query,
state,
} from '@spectrum-web-components/base/src/decorators.js';

import pickerStyles from './picker.css.js';
Expand Down Expand Up @@ -77,6 +81,9 @@ export class PickerBase extends SizedMixin(Focusable) {

protected isMobile = new MatchMediaController(this, IS_MOBILE);

@state()
appliedLabel?: string;

@query('#button')
public button!: HTMLButtonElement;

Expand Down Expand Up @@ -373,7 +380,15 @@ export class PickerBase extends SizedMixin(Focusable) {
return content;
}
return html`
<slot name="label">${this.label}</slot>
<slot name="label">
<span
aria-hidden=${ifDefined(
this.appliedLabel ? undefined : 'true'
)}
>
${this.label}
</span>
</slot>
`;
}

Expand All @@ -382,30 +397,56 @@ export class PickerBase extends SizedMixin(Focusable) {
'visually-hidden': this.icons === 'only' && !!this.value,
placeholder: !this.value,
};
const appliedLabel = this.appliedLabel || this.label;
return [
html`
</span>
<span id="icon" ?hidden=${this.icons === 'none'}>
${this.selectedItemContent.icon}
</span>
<span id="label" class=${classMap(labelClasses)}>
${this.renderLabelContent(this.selectedItemContent.content)}
</span>
${this.invalid
? html`
<sp-icon-alert
class="validation-icon"
></sp-icon-alert>
`
: nothing}
${
this.value && this.selectedItem
? html`
<span
aria-hidden="true"
class="visually-hidden"
id="applied-label"
>
${appliedLabel}
<slot name="label"></slot>
</span>
`
: html`
<span hidden id="applied-label">
${appliedLabel}
</span>
`
}
${
this.invalid
? html`
<sp-icon-alert
class="validation-icon"
></sp-icon-alert>
`
: nothing
}
<sp-icon-chevron100
class="picker ${chevronClass[
this.size as DefaultElementSize
]}"
class="picker ${
chevronClass[this.size as DefaultElementSize]
}"
></sp-icon-chevron100>
`,
];
}

applyFocusElementLabel = (value?: string): void => {
this.appliedLabel = value;
};

// a helper to throw focus to the button is needed because Safari
// won't include buttons in the tab order even with tabindex="0"
protected override render(): TemplateResult {
Expand All @@ -418,7 +459,7 @@ export class PickerBase extends SizedMixin(Focusable) {
<button
aria-haspopup="true"
aria-expanded=${this.open ? 'true' : 'false'}
aria-labelledby="button icon label"
aria-labelledby="icon label applied-label"
id="button"
class="button"
@blur=${this.onButtonBlur}
Expand Down
8 changes: 2 additions & 6 deletions packages/picker/src/picker.css
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,6 @@ sp-popover {
white-space: nowrap;
}

:host([dir='ltr']) #label.visually-hidden + .picker {
margin-left: auto;
}

:host([dir='rtl']) #label.visually-hidden + .picker {
margin-right: auto;
#label.visually-hidden ~ .picker {
margin-inline-start: auto;
}
65 changes: 59 additions & 6 deletions packages/picker/stories/picker.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,66 @@ export const Default = (args: StoryArgs): TemplateResult => {
label="Select a Country with a very long label, too long, in fact"
${spreadProps(args)}
>
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-item value="option-1">Deselect</sp-menu-item>
<sp-menu-item value="option-2">Select Inverse</sp-menu-item>
<sp-menu-item value="option-3">Feather...</sp-menu-item>
<sp-menu-item value="option-4">Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
<sp-menu-item value="option-5">Save Selection</sp-menu-item>
<sp-menu-item disabled value="option-6">
Make Work Path
</sp-menu-item>
</sp-picker>
<p>This is some text.</p>
<p>This is some text.</p>
<p>
This is a
<a href="#anchor">link</a>
.
</p>
`;
};

export const noVisibleLabel = (args: StoryArgs): TemplateResult => {
return html`
<sp-picker
@change=${handleChange(args)}
label="Where do you live?"
${spreadProps(args)}
>
<sp-menu-item value="option-1">Deselect</sp-menu-item>
<sp-menu-item value="option-2">Select Inverse</sp-menu-item>
<sp-menu-item value="option-3">Feather...</sp-menu-item>
<sp-menu-item value="option-4">Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item value="option-5">Save Selection</sp-menu-item>
<sp-menu-item disabled value="option-6">
Make Work Path
</sp-menu-item>
</sp-picker>
<p>This is some text.</p>
<p>This is some text.</p>
<p>
This is a
<a href="#anchor">link</a>
.
</p>
`;
};

export const slottedLabel = (args: StoryArgs): TemplateResult => {
return html`
<sp-picker @change=${handleChange(args)} ${spreadProps(args)}>
<span slot="label">Where do you live?</span>
<sp-menu-item value="option-1">Deselect</sp-menu-item>
<sp-menu-item value="option-2">Select Inverse</sp-menu-item>
<sp-menu-item value="option-3">Feather...</sp-menu-item>
<sp-menu-item value="option-4">Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item value="option-5">Save Selection</sp-menu-item>
<sp-menu-item disabled value="option-6">
Make Work Path
</sp-menu-item>
</sp-picker>
<p>This is some text.</p>
<p>This is some text.</p>
Expand Down
Loading

0 comments on commit 8ce0cb0

Please # to comment.