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(range, select): prefer labels passed by developer #29145

Merged
merged 12 commits into from
Mar 14, 2024
60 changes: 21 additions & 39 deletions .github/COMPONENT-GUIDE.md
brandyscarney marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -439,53 +439,35 @@ render() {

#### 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
const { label, labelId, labelText } = getAriaLabel(el, inputId);
```

where `el` and `inputId` are the following:
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.
liamdebeasi marked this conversation as resolved.
Show resolved Hide resolved

```tsx
export class Checkbox implements ComponentInterface {
private inputId = `ion-cb-${checkboxIds++}`;
import { Prop } from '@stencil/core';
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
<Host
aria-labelledby={label ? labelId : null}
aria-checked={`${checked}`}
aria-hidden={disabled ? 'true' : null}
role="checkbox"
>
```
@Prop() labelText?: string;

In addition to that, the checkbox input should have a label added:
componentWillLoad() {
this.inheritedAttributes = inheritAttributes(this.el, ['aria-label']);
}

```tsx
<Host
aria-labelledby={label ? labelId : null}
aria-checked={`${checked}`}
aria-hidden={disabled ? 'true' : null}
role="checkbox"
>
<label htmlFor={inputId}>
{labelText}
</label>
<input
type="checkbox"
aria-checked={`${checked}`}
disabled={disabled}
id={inputId}
/>
render() {
return (
<Host>
<label>
{this.labelText}
<button role="checkbox" {...this.inheritedAttributes}></button>
</label>
</Host>
)
}
```

#### Hidden Input
Expand Down
4 changes: 2 additions & 2 deletions BREAKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,11 @@ For more information on styling toast buttons, refer to the [Toast Theming docum

<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>

- 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>

Expand Down
51 changes: 10 additions & 41 deletions core/src/components/range/range.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core';
import { Component, Element, Event, Host, Prop, State, Watch, h } from '@stencil/core';
import { findClosestIonContent, disableContentScrollY, resetContentScrollY } from '@utils/content';
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 { isRTL } from '@utils/rtl';
import { createColorClasses, hostContext } from '@utils/theme';
Expand Down Expand Up @@ -624,28 +624,16 @@ export class Range implements ComponentInterface {
min,
max,
step,
el,
handleKeyboard,
pressedKnob,
disabled,
pin,
ratioLower,
ratioUpper,
inheritedAttributes,
rangeId,
pinFormatter,
inheritedAttributes,
} = 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 barEnd = `${100 - ratioUpper * 100}%`;

Expand Down Expand Up @@ -715,11 +703,6 @@ export class Range implements ComponentInterface {
}
}

let labelledBy: string | undefined;
if (this.hasLabel) {
labelledBy = 'range-label';
}

return (
<div
class="range-slider"
Expand Down Expand Up @@ -791,8 +774,7 @@ export class Range implements ComponentInterface {
handleKeyboard,
min,
max,
labelText,
labelledBy,
inheritedAttributes,
})}

{this.dualKnobs &&
Expand All @@ -807,8 +789,7 @@ export class Range implements ComponentInterface {
handleKeyboard,
min,
max,
labelText,
labelledBy,
inheritedAttributes,
})}
</div>
);
Expand Down Expand Up @@ -887,27 +868,13 @@ interface RangeKnob {
pressed: boolean;
pin: boolean;
pinFormatter: PinFormatter;
labelText?: string | null;
labelledBy?: string;
inheritedAttributes: Attributes;
handleKeyboard: (name: KnobName, isIncrease: boolean) => void;
}

const renderKnob = (
rtl: boolean,
{
knob,
value,
ratio,
min,
max,
disabled,
pressed,
pin,
handleKeyboard,
labelText,
labelledBy,
pinFormatter,
}: RangeKnob
{ knob, value, ratio, min, max, disabled, pressed, pin, handleKeyboard, pinFormatter, inheritedAttributes }: RangeKnob
) => {
const start = rtl ? 'right' : 'left';

Expand All @@ -919,6 +886,8 @@ const renderKnob = (
return style;
};

const ariaLabel = inheritedAttributes['aria-label'];

return (
<div
onKeyDown={(ev: KeyboardEvent) => {
Expand Down Expand Up @@ -946,8 +915,8 @@ const renderKnob = (
style={knobStyle()}
role="slider"
tabindex={disabled ? -1 : 0}
aria-label={labelledBy === undefined ? labelText : null}
aria-labelledby={labelledBy !== undefined ? labelledBy : null}
aria-label={ariaLabel !== undefined ? ariaLabel : null}
aria-labelledby={ariaLabel === undefined ? 'range-label' : null}
aria-valuemin={min}
aria-valuemax={max}
aria-disabled={disabled ? 'true' : null}
Expand Down
9 changes: 5 additions & 4 deletions core/src/components/select/select.tsx
brandyscarney marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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 type { NotchController } 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 { actionSheetController, alertController, popoverController } from '@utils/overlays';
import type { OverlaySelect } from '@utils/overlays-interface';
Expand Down Expand Up @@ -874,10 +874,11 @@ export class Select implements ComponentInterface {
}

private get ariaLabel() {
const { placeholder, el, inputId, inheritedAttributes } = this;
const { placeholder, inheritedAttributes } = this;
const displayValue = this.getText();
const { labelText } = getAriaLabel(el, inputId);
const definedLabel = this.labelText ?? inheritedAttributes['aria-label'] ?? labelText;

// Developers should provide either visible text or an aria-label
const definedLabel = this.labelText ?? inheritedAttributes['aria-label'];

/**
* If developer has specified a placeholder
Expand Down
58 changes: 0 additions & 58 deletions core/src/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
* a Shadow DOM. It does not add the input inside of the Shadow root which
Expand Down
98 changes: 0 additions & 98 deletions core/src/utils/test/aria.spec.ts

This file was deleted.

Loading