fix(range, select): prefer labels passed by developer (#29145)

This commit is contained in:
Liam DeBeasi
2024-03-14 11:14:21 -04:00
committed by GitHub
parent b148b3225b
commit 56014cf64c
8 changed files with 122 additions and 281 deletions

View File

@ -439,53 +439,38 @@ render() {
#### Labels #### Labels
A helper function has been created to get the proper `aria-label` for the checkbox. This can be imported as `getAriaLabel` like the following: Labels should be passed directly to the component in the form of either visible text or an `aria-label`. The visible text can be set inside of a `label` element, and the `aria-label` can be set directly on the interactive element.
```tsx In the following example the `aria-label` can be inherited from the Host using the `inheritAttributes` or `inheritAriaAttributes` utilities. This allows developers to set `aria-label` on the host element since they do not have access to inside the shadow root.
const { label, labelId, labelText } = getAriaLabel(el, inputId);
```
where `el` and `inputId` are the following: > [!NOTE]
> Use `inheritAttributes` to specify which attributes should be inherited or `inheritAriaAttributes` to inherit all of the possible `aria` attributes.
```tsx ```tsx
export class Checkbox implements ComponentInterface { import { Prop } from '@stencil/core';
private inputId = `ion-cb-${checkboxIds++}`; import { inheritAttributes } from '@utils/helpers';
import type { Attributes } from '@utils/helpers';
@Element() el!: HTMLElement;
... ...
}
```
This can then be added to the `Host` like the following: private inheritedAttributes: Attributes = {};
```tsx @Prop() labelText?: string;
<Host
aria-labelledby={label ? labelId : null}
aria-checked={`${checked}`}
aria-hidden={disabled ? 'true' : null}
role="checkbox"
>
```
In addition to that, the checkbox input should have a label added: componentWillLoad() {
this.inheritedAttributes = inheritAttributes(this.el, ['aria-label']);
}
```tsx render() {
<Host return (
aria-labelledby={label ? labelId : null} <Host>
aria-checked={`${checked}`} <label>
aria-hidden={disabled ? 'true' : null} {this.labelText}
role="checkbox" <input type="checkbox" {...this.inheritedAttributes} />
>
<label htmlFor={inputId}>
{labelText}
</label> </label>
<input </Host>
type="checkbox" )
aria-checked={`${checked}`} }
disabled={disabled}
id={inputId}
/>
``` ```
#### Hidden Input #### Hidden Input
@ -567,57 +552,40 @@ render() {
#### Labels #### Labels
A helper function has been created to get the proper `aria-label` for the switch. This can be imported as `getAriaLabel` like the following: Labels should be passed directly to the component in the form of either visible text or an `aria-label`. The visible text can be set inside of a `label` element, and the `aria-label` can be set directly on the interactive element.
```tsx In the following example the `aria-label` can be inherited from the Host using the `inheritAttributes` or `inheritAriaAttributes` utilities. This allows developers to set `aria-label` on the host element since they do not have access to inside the shadow root.
const { label, labelId, labelText } = getAriaLabel(el, inputId);
```
where `el` and `inputId` are the following: > [!NOTE]
> Use `inheritAttributes` to specify which attributes should be inherited or `inheritAriaAttributes` to inherit all of the possible `aria` attributes.
```tsx ```tsx
export class Toggle implements ComponentInterface { import { Prop } from '@stencil/core';
private inputId = `ion-tg-${toggleIds++}`; import { inheritAttributes } from '@utils/helpers';
import type { Attributes } from '@utils/helpers';
@Element() el!: HTMLElement;
... ...
}
```
This can then be added to the `Host` like the following: private inheritedAttributes: Attributes = {};
```tsx @Prop() labelText?: string;
<Host
aria-labelledby={label ? labelId : null}
aria-checked={`${checked}`}
aria-hidden={disabled ? 'true' : null}
role="switch"
>
```
In addition to that, the checkbox input should have a label added: componentWillLoad() {
this.inheritedAttributes = inheritAttributes(this.el, ['aria-label']);
}
```tsx render() {
<Host return (
aria-labelledby={label ? labelId : null} <Host>
aria-checked={`${checked}`} <label>
aria-hidden={disabled ? 'true' : null} {this.labelText}
role="switch" <input type="checkbox" role="switch" {...this.inheritedAttributes} />
>
<label htmlFor={inputId}>
{labelText}
</label> </label>
<input </Host>
type="checkbox" )
role="switch" }
aria-checked={`${checked}`}
disabled={disabled}
id={inputId}
/>
``` ```
#### Hidden Input #### Hidden Input
A helper function to render a hidden input has been added, it can be added in the `render`: A helper function to render a hidden input has been added, it can be added in the `render`:

View File

@ -254,11 +254,11 @@ For more information on styling toast buttons, refer to the [Toast Theming docum
<h4 id="version-8x-range">Range</h4> <h4 id="version-8x-range">Range</h4>
- The `legacy` property and support for the legacy syntax, which involved placing an `ion-range` inside of an `ion-item` with an `ion-label`, have been removed. For more information on migrating from the legacy range syntax, refer to the [Range documentation](https://ionicframework.com/docs/api/range#migrating-from-legacy-range-syntax). - The `legacy` property and support for the legacy syntax, which involved placing an `ion-range` inside of an `ion-item` with an `ion-label`, have been removed. Ionic will also no longer attempt to automatically associate form controls with sibling `<label>` elements as these label elements are now used inside the form control. Developers should provide a label (either visible text or `aria-label`) directly to the form control. For more information on migrating from the legacy range syntax, refer to the [Range documentation](https://ionicframework.com/docs/api/range#migrating-from-legacy-range-syntax).
<h4 id="version-8x-select">Select</h4> <h4 id="version-8x-select">Select</h4>
- The `legacy` property and support for the legacy syntax, which involved placing an `ion-select` inside of an `ion-item` with an `ion-label`, have been removed. For more information on migrating from the legacy select syntax, refer to the [Select documentation](https://ionicframework.com/docs/api/select#migrating-from-legacy-select-syntax). - The `legacy` property and support for the legacy syntax, which involved placing an `ion-select` inside of an `ion-item` with an `ion-label`, have been removed. Ionic will also no longer attempt to automatically associate form controls with sibling `<label>` elements as these label elements are now used inside the form control. Developers should provide a label (either visible text or `aria-label`) directly to the form control. For more information on migrating from the legacy select syntax, refer to the [Select documentation](https://ionicframework.com/docs/api/select#migrating-from-legacy-select-syntax).
<h4 id="version-8x-textarea">Textarea</h4> <h4 id="version-8x-textarea">Textarea</h4>

View File

@ -2,7 +2,7 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core';
import { Component, Element, Event, Host, Prop, State, Watch, h } from '@stencil/core'; import { Component, Element, Event, Host, Prop, State, Watch, h } from '@stencil/core';
import { findClosestIonContent, disableContentScrollY, resetContentScrollY } from '@utils/content'; import { findClosestIonContent, disableContentScrollY, resetContentScrollY } from '@utils/content';
import type { Attributes } from '@utils/helpers'; import type { Attributes } from '@utils/helpers';
import { inheritAriaAttributes, clamp, debounceEvent, getAriaLabel, renderHiddenInput } from '@utils/helpers'; import { inheritAriaAttributes, clamp, debounceEvent, renderHiddenInput } from '@utils/helpers';
import { printIonWarning } from '@utils/logging'; import { printIonWarning } from '@utils/logging';
import { isRTL } from '@utils/rtl'; import { isRTL } from '@utils/rtl';
import { createColorClasses, hostContext } from '@utils/theme'; import { createColorClasses, hostContext } from '@utils/theme';
@ -624,28 +624,16 @@ export class Range implements ComponentInterface {
min, min,
max, max,
step, step,
el,
handleKeyboard, handleKeyboard,
pressedKnob, pressedKnob,
disabled, disabled,
pin, pin,
ratioLower, ratioLower,
ratioUpper, ratioUpper,
inheritedAttributes,
rangeId,
pinFormatter, pinFormatter,
inheritedAttributes,
} = this; } = this;
/**
* Look for external label, ion-label, or aria-labelledby.
* If none, see if user placed an aria-label on the host
* and use that instead.
*/
let { labelText } = getAriaLabel(el, rangeId!);
if (labelText === undefined || labelText === null) {
labelText = inheritedAttributes['aria-label'];
}
let barStart = `${ratioLower * 100}%`; let barStart = `${ratioLower * 100}%`;
let barEnd = `${100 - ratioUpper * 100}%`; let barEnd = `${100 - ratioUpper * 100}%`;
@ -715,11 +703,6 @@ export class Range implements ComponentInterface {
} }
} }
let labelledBy: string | undefined;
if (this.hasLabel) {
labelledBy = 'range-label';
}
return ( return (
<div <div
class="range-slider" class="range-slider"
@ -791,8 +774,7 @@ export class Range implements ComponentInterface {
handleKeyboard, handleKeyboard,
min, min,
max, max,
labelText, inheritedAttributes,
labelledBy,
})} })}
{this.dualKnobs && {this.dualKnobs &&
@ -807,8 +789,7 @@ export class Range implements ComponentInterface {
handleKeyboard, handleKeyboard,
min, min,
max, max,
labelText, inheritedAttributes,
labelledBy,
})} })}
</div> </div>
); );
@ -887,27 +868,13 @@ interface RangeKnob {
pressed: boolean; pressed: boolean;
pin: boolean; pin: boolean;
pinFormatter: PinFormatter; pinFormatter: PinFormatter;
labelText?: string | null; inheritedAttributes: Attributes;
labelledBy?: string;
handleKeyboard: (name: KnobName, isIncrease: boolean) => void; handleKeyboard: (name: KnobName, isIncrease: boolean) => void;
} }
const renderKnob = ( const renderKnob = (
rtl: boolean, rtl: boolean,
{ { knob, value, ratio, min, max, disabled, pressed, pin, handleKeyboard, pinFormatter, inheritedAttributes }: RangeKnob
knob,
value,
ratio,
min,
max,
disabled,
pressed,
pin,
handleKeyboard,
labelText,
labelledBy,
pinFormatter,
}: RangeKnob
) => { ) => {
const start = rtl ? 'right' : 'left'; const start = rtl ? 'right' : 'left';
@ -919,6 +886,9 @@ const renderKnob = (
return style; return style;
}; };
// The aria label should be preferred over visible text if both are specified
const ariaLabel = inheritedAttributes['aria-label'];
return ( return (
<div <div
onKeyDown={(ev: KeyboardEvent) => { onKeyDown={(ev: KeyboardEvent) => {
@ -946,8 +916,8 @@ const renderKnob = (
style={knobStyle()} style={knobStyle()}
role="slider" role="slider"
tabindex={disabled ? -1 : 0} tabindex={disabled ? -1 : 0}
aria-label={labelledBy === undefined ? labelText : null} aria-label={ariaLabel !== undefined ? ariaLabel : null}
aria-labelledby={labelledBy !== undefined ? labelledBy : null} aria-labelledby={ariaLabel === undefined ? 'range-label' : null}
aria-valuemin={min} aria-valuemin={min}
aria-valuemax={max} aria-valuemax={max}
aria-disabled={disabled ? 'true' : null} aria-disabled={disabled ? 'true' : null}

View File

@ -20,4 +20,34 @@ describe('range: label', () => {
expect(propEl).not.toBeNull(); expect(propEl).not.toBeNull();
expect(slotEl).toBeNull(); expect(slotEl).toBeNull();
}); });
it('should prefer aria label if both attribute and visible text provided', async () => {
const page = await newSpecPage({
components: [Range],
html: `
<ion-range aria-label="Aria Label Text" label="Label Prop Text"></ion-range>
`,
});
const range = page.body.querySelector('ion-range')!;
const nativeSlider = range.shadowRoot!.querySelector('.range-knob-handle')!;
expect(nativeSlider.getAttribute('aria-label')).toBe('Aria Label Text');
expect(nativeSlider.getAttribute('aria-labelledby')).toBe(null);
});
it('should prefer visible label if only visible text provided', async () => {
const page = await newSpecPage({
components: [Range],
html: `
<ion-range label="Label Prop Text"></ion-range>
`,
});
const range = page.body.querySelector('ion-range')!;
const nativeSlider = range.shadowRoot!.querySelector('.range-knob-handle')!;
expect(nativeSlider.getAttribute('aria-label')).toBe(null);
expect(nativeSlider.getAttribute('aria-labelledby')).toBe('range-label');
});
}); });

View File

@ -2,7 +2,7 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core';
import { Component, Element, Event, Host, Method, Prop, State, Watch, h, forceUpdate } from '@stencil/core'; import { Component, Element, Event, Host, Method, Prop, State, Watch, h, forceUpdate } from '@stencil/core';
import type { NotchController } from '@utils/forms'; import type { NotchController } from '@utils/forms';
import { compareOptions, createNotchController, isOptionSelected } from '@utils/forms'; import { compareOptions, createNotchController, isOptionSelected } from '@utils/forms';
import { focusVisibleElement, getAriaLabel, renderHiddenInput, inheritAttributes } from '@utils/helpers'; import { focusVisibleElement, renderHiddenInput, inheritAttributes } from '@utils/helpers';
import type { Attributes } from '@utils/helpers'; import type { Attributes } from '@utils/helpers';
import { actionSheetController, alertController, popoverController } from '@utils/overlays'; import { actionSheetController, alertController, popoverController } from '@utils/overlays';
import type { OverlaySelect } from '@utils/overlays-interface'; import type { OverlaySelect } from '@utils/overlays-interface';
@ -874,10 +874,11 @@ export class Select implements ComponentInterface {
} }
private get ariaLabel() { private get ariaLabel() {
const { placeholder, el, inputId, inheritedAttributes } = this; const { placeholder, inheritedAttributes } = this;
const displayValue = this.getText(); const displayValue = this.getText();
const { labelText } = getAriaLabel(el, inputId);
const definedLabel = this.labelText ?? inheritedAttributes['aria-label'] ?? labelText; // The aria label should be preferred over visible text if both are specified
const definedLabel = inheritedAttributes['aria-label'] ?? this.labelText;
/** /**
* If developer has specified a placeholder * If developer has specified a placeholder

View File

@ -68,6 +68,34 @@ describe('ion-select', () => {
expect(propEl).not.toBe(null); expect(propEl).not.toBe(null);
expect(slotEl).toBe(null); expect(slotEl).toBe(null);
}); });
it('should prefer aria label if both attribute and visible text provided', async () => {
const page = await newSpecPage({
components: [Select],
html: `
<ion-select aria-label="Aria Label Text" label="Label Prop Text"></ion-select>
`,
});
const select = page.body.querySelector('ion-select')!;
const nativeButton = select.shadowRoot!.querySelector('button')!;
expect(nativeButton.getAttribute('aria-label')).toBe('Aria Label Text');
});
it('should prefer visible label if only visible text provided', async () => {
const page = await newSpecPage({
components: [Select],
html: `
<ion-select label="Label Prop Text"></ion-select>
`,
});
const select = page.body.querySelector('ion-select')!;
const nativeButton = select.shadowRoot!.querySelector('button')!;
expect(nativeButton.getAttribute('aria-label')).toBe('Label Prop Text');
});
}); });
describe('select: slot interactivity', () => { describe('select: slot interactivity', () => {

View File

@ -273,64 +273,6 @@ export const focusVisibleElement = (el: HTMLElement) => {
} }
}; };
/**
* This method is used for Ionic's input components that use Shadow DOM. In
* order to properly label the inputs to work with screen readers, we need
* to get the text content of the label outside of the shadow root and pass
* it to the input inside of the shadow root.
*
* Referencing label elements by id from outside of the component is
* impossible due to the shadow boundary, read more here:
* https://developer.salesforce.com/blogs/2020/01/accessibility-for-web-components.html
*
* @param componentEl The shadow element that needs the aria label
* @param inputId The unique identifier for the input
*/
export const getAriaLabel = (
componentEl: HTMLElement,
inputId: string
): { label: Element | null; labelId: string; labelText: string | null | undefined } => {
let labelText;
// If the user provides their own label via the aria-labelledby attr
// we should use that instead of looking for an ion-label
const labelledBy = componentEl.getAttribute('aria-labelledby');
// Grab the id off of the component in case they are using
// a custom label using the label element
const componentId = componentEl.id;
let labelId = labelledBy !== null && labelledBy.trim() !== '' ? labelledBy : inputId + '-lbl';
let label = labelledBy !== null && labelledBy.trim() !== '' ? document.getElementById(labelledBy) : null;
if (label) {
if (labelledBy === null) {
label.id = labelId;
}
labelText = label.textContent;
label.setAttribute('aria-hidden', 'true');
// if there is no label, check to see if the user has provided
// one by setting an id on the component and using the label element
} else if (componentId.trim() !== '') {
label = document.querySelector(`label[for="${componentId}"]`);
if (label) {
if (label.id !== '') {
labelId = label.id;
} else {
label.id = labelId = `${componentId}-lbl`;
}
labelText = label.textContent;
}
}
return { label, labelId, labelText };
};
/** /**
* This method is used to add a hidden input to a host element that contains * This method is used to add a hidden input to a host element that contains
* a Shadow DOM. It does not add the input inside of the Shadow root which * a Shadow DOM. It does not add the input inside of the Shadow root which

View File

@ -1,98 +0,0 @@
import { newSpecPage } from '@stencil/core/testing';
import { Item } from '../../components/item/item';
import { Label } from '../../components/label/label';
import { Toggle } from '../../components/toggle/toggle';
import { getAriaLabel } from '../helpers';
// TODO FW-5969
describe.skip('getAriaLabel()', () => {
it('should correctly link component to label', async () => {
const page = await newSpecPage({
components: [Item, Label, Toggle],
html: `
<ion-item>
<ion-label>My Label</ion-label>
<ion-toggle></ion-toggle>
</ion-item>
`,
});
const toggle = page.body.querySelector('ion-toggle')!;
const { label, labelId, labelText } = getAriaLabel(toggle, 'ion-tg-0');
expect(labelText).toEqual('My Label');
expect(labelId).toEqual('ion-tg-0-lbl');
expect(label).toEqual(page.body.querySelector('ion-label'));
});
it('should correctly link component when using custom label', async () => {
const page = await newSpecPage({
components: [Toggle],
html: `
<div id="my-label">Hello World</div>
<ion-toggle legacy="true" aria-labelledby="my-label"></ion-toggle>
`,
});
const toggle = page.body.querySelector('ion-toggle')!;
const { label, labelId, labelText } = getAriaLabel(toggle, 'ion-tg-0');
expect(labelText).toEqual('Hello World');
expect(labelId).toEqual('my-label');
expect(label).toEqual(page.body.querySelector('#my-label'));
});
it('should correctly link component when special characters are used', async () => {
const page = await newSpecPage({
components: [Toggle],
html: `
<div id="id.1">Hello World</div>
<ion-toggle legacy="true" aria-labelledby="id.1"></ion-toggle>
`,
});
const toggle = page.body.querySelector('ion-toggle')!;
const { labelId, labelText } = getAriaLabel(toggle, 'ion-tg-0');
expect(labelText).toEqual('Hello World');
expect(labelId).toEqual('id.1');
});
it('should only set the label id if one was not set already', async () => {
const page = await newSpecPage({
components: [Toggle],
html: `
<label id="my-id" for="id.1">Hello World</label>
<ion-toggle legacy="true" id="id.1"></ion-toggle>
`,
});
const toggle = page.body.querySelector('ion-toggle')!;
const { labelId, labelText } = getAriaLabel(toggle, 'ion-tg-0');
expect(labelText).toEqual('Hello World');
expect(labelId).toEqual('my-id');
});
it('should set label id', async () => {
const page = await newSpecPage({
components: [Toggle],
html: `
<label for="id.1">Hello World</label>
<ion-toggle legacy="true" id="id.1"></ion-toggle>
`,
});
const toggle = page.body.querySelector('ion-toggle')!;
const { labelId, labelText } = getAriaLabel(toggle, 'ion-tg-0');
expect(labelText).toEqual('Hello World');
expect(labelId).toEqual('id.1-lbl');
});
});