Skip to content

fix(Form): improve a11y of ui5 web component inputs #5846

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

Merged
merged 3 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions .storybook/preview-head.html
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@
padding: 0;
}

.pseudoInvisibleText {
font-size: 0;
left: 0;
position: absolute;
top: 0;
user-select: none;
}

/* TODO remove this workaround as soon as https://github.com/storybookjs/storybook/issues/20497 is fixed */
.docs-story > div > div[scale] {
min-height: 20px;
Expand Down
64 changes: 39 additions & 25 deletions packages/main/src/components/Form/Form.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,35 @@ import { Form } from './index.js';
import { cypressPassThroughTestsFactory, mountWithCustomTagName } from '@/cypress/support/utils';

const component = (
<Form titleText={'Test form'}>
{false}
{null}
{undefined}
<FormGroup titleText={'Group 1'}>
<FormItem label={'item 1'}>
<Input data-testid="formInput" type={InputType.Text} />
</FormItem>
<FormItem label={'item 2'}>
<Input type={InputType.Number} />
</FormItem>
</FormGroup>
<FormGroup titleText={'Group 2'}>
<FormItem label={'item 3'}>
<Input data-testid="formInput2" type={InputType.Text} />
</FormItem>
<FormItem label={<Label>item 4</Label>}>
<Input type={InputType.Number} id="test-id" />
</FormItem>
</FormGroup>
</Form>
<>
<Form titleText={'Test form'}>
{false}
{null}
{undefined}
<FormGroup titleText={'Group 1'}>
<FormItem label={'item 1'}>
<Input data-testid="formInput" type={InputType.Text} />
</FormItem>
<FormItem label={'item 2'}>
<Input type={InputType.Number} />
</FormItem>
</FormGroup>
<FormGroup titleText={'Group 2'}>
<FormItem label={'item 3'}>
<Input data-testid="formInput2" type={InputType.Text} />
</FormItem>
<FormItem label={<Label>item 4</Label>}>
<Input type={InputType.Number} accessibleNameRef="test-id" />
</FormItem>
<FormItem label={<Label>item 4</Label>}>
<Input type={InputType.Number} accessibleName="custom label" />
</FormItem>
</FormGroup>
</Form>
<span id="test-id" style={{ fontSize: 0, left: 0, position: 'absolute', top: 0, userSelect: 'none' }}>
custom label
</span>
</>
);

const ConditionRenderingExample = () => {
Expand Down Expand Up @@ -119,16 +127,22 @@ describe('Form', () => {
it('a11y labels', () => {
cy.mount(component);
for (let i = 1; i <= 3; i++) {
cy.get('label').contains(`item ${i}`).should('exist').should('not.be.visible');
cy.get('span').contains(`item ${i}`).should('exist').should('not.be.visible');
cy.get('[ui5-label]').contains(`item ${i}`).should('be.visible');
}
// custom `Label`
cy.findAllByText(`item 4`).eq(0).should('be.visible');
cy.findAllByText(`item 4`).eq(1).should('exist').should('not.be.visible');

// custom id child of FormItem
cy.get('#test-id').should('have.length', 1).should('be.visible');
cy.get('[for="test-id"]').should('have.length', 1).should('not.be.visible');
// custom accessibleNameRef of FormItem input
cy.get('#test-id').should('have.length', 1).should('not.be.visible');
cy.get('[accessible-name-ref="test-id"]').should('have.length', 1).should('be.visible');

// custom accessibleName of FormItem input
cy.get('[accessible-name="custom label"]')
.should('have.length', 1)
.should('be.visible')
.should('not.have.attr', 'accessible-name-ref');
});

it('FilterItem: doesnt crash with portal as child', () => {
Expand Down
56 changes: 56 additions & 0 deletions packages/main/src/components/Form/Form.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,62 @@ const FormComponent = (props) => {

</details>

## Custom Screen Reader Announcements

In general we recommend using the default announcements of the Form, but if you need to adjust them anyway, then this example might help avoiding some of the typical pitfalls.

<Canvas of={ComponentStories.CustomLabel} sourceState="none" />

### Code

<details>

<summary>Show Code</summary>

```jsx
function FormComponent() {
const uniqueId = useId();
return (
<Form
titleText="Not announced (because of `aria-label` of the `Form`)"
aria-label="Custom announcement of the form title via aria-label"
>
<FormGroup titleText="Default Group Announcement">
<FormItem label={<Label>Default announcement with custom Label</Label>}>
<Input />
</FormItem>
</FormGroup>
<FormGroup titleText="Not announced (because of `accessibleName` of the `Input`)">
<FormItem label={<Label>Not announced (because of `accessibleName` of the `Input`)</Label>}>
<Input accessibleName="Custom announcement via accessibleName prop" />
</FormItem>
</FormGroup>
<FormGroup titleText="Not announced (because of `accessibleNameRef` of the `Input`)">
<FormItem label={<Label>Not announced (because of `accessibleNameRef` of the `Input`)</Label>}>
<Input accessibleNameRef={`${uniqueId}-input1`} />
<span id={`${uniqueId}-input1`} className="pseudoInvisibleText">
Custom announcement via accessibleNameRef prop
</span>
</FormItem>
</FormGroup>
<FormGroup
titleText="Announced (because of `accessibleNameRef` of the `Input` and linking id)"
id={`${uniqueId}-group`}
>
<FormItem label={<Label>Not announced (because of `accessibleNameRef` of the `Input`)</Label>}>
<Input accessibleNameRef={`${uniqueId}-group ${uniqueId}-input2`} />
<span id={`${uniqueId}-input2`} className="pseudoInvisibleText">
Custom announcement via accessibleNameRef prop
</span>
</FormItem>
</FormGroup>
</Form>
);
}
```

</details>

<Markdown>{SubcomponentsSection}</Markdown>

## Form Group
Expand Down
47 changes: 45 additions & 2 deletions packages/main/src/components/Form/Form.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Meta, StoryObj } from '@storybook/react';
import { useReducer } from 'react';
import { useId, useReducer } from 'react';
import {
Button,
CheckBox,
Expand Down Expand Up @@ -309,7 +309,7 @@ export const DisplayEditMode: Story = {
}
};

export const FormWithOneGroup: Story = {
export const FormItemsWithoutGroup: Story = {
args: {
titleText: 'Address',
columnsM: 2,
Expand Down Expand Up @@ -376,3 +376,46 @@ export const FormWithOneGroup: Story = {
);
}
};

export const CustomLabel: Story = {
name: 'Custom Label (a11y)',
render() {
const uniqueId = useId();
return (
<Form
titleText="Not announced (because of `aria-label` of the `Form`)"
aria-label="Custom announcement of the form title via aria-label"
>
<FormGroup titleText="Default Group Announcement">
<FormItem label={<Label>Default announcement with custom Label</Label>}>
<Input />
</FormItem>
</FormGroup>
<FormGroup titleText="Not announced (because of `accessibleName` of the `Input`)">
<FormItem label={<Label>Not announced (because of `accessibleName` of the `Input`)</Label>}>
<Input accessibleName="Custom announcement via accessibleName prop" />
</FormItem>
</FormGroup>
<FormGroup titleText="Not announced (because of `accessibleNameRef` of the `Input`)">
<FormItem label={<Label>Not announced (because of `accessibleNameRef` of the `Input`)</Label>}>
<Input accessibleNameRef={`${uniqueId}-input1`} />
<span id={`${uniqueId}-input1`} className="pseudoInvisibleText">
Custom announcement via accessibleNameRef prop
</span>
</FormItem>
</FormGroup>
<FormGroup
titleText="Announced (because of `accessibleNameRef` of the `Input` and linking id)"
id={`${uniqueId}-group`}
>
<FormItem label={<Label>Not announced (because of `accessibleNameRef` of the `Input`)</Label>}>
<Input accessibleNameRef={`${uniqueId}-group ${uniqueId}-input2`} />
<span id={`${uniqueId}-input2`} className="pseudoInvisibleText">
Custom announcement via accessibleNameRef prop
</span>
</FormItem>
</FormGroup>
</Form>
);
}
};
10 changes: 9 additions & 1 deletion packages/main/src/components/Form/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,16 @@ export interface FormPropTypes extends CommonProps {

/**
* The `Form` component arranges labels and fields into groups and rows. There are different ways to visualize forms for different screen sizes.
* It is possible to change the alignment of all labels by setting the CSS `align-items` property, per default all labels are centered.
* It is possible to change the alignment of all labels by setting the CSS `align-items` property. By default, labels are centered when `labelSpan` is less than 12 and aligned to the start when `labelSpan` is equal to 12."
*
* __Accessibility features:__
*
* The Form only supports announcing labels and groups by screen readers for UI5 Web Components inputs like `Input (ui5-input)`, `CheckBox (ui5-checkbox)`,`DatePicker (ui5-date-picker)`, etc.
* For other inputs, this behavior must be implemented manually. Also, please note that when passing custom React components to the `FilterItem`, it's mandatory to pass through the `accessibleNameRef` prop, as otherwise the label won't be announced.
*
* __Note:__ The `Form` calculates its width based on the available space of its container. If the container also dynamically adjusts its width to its contents, you must ensure that you specify a fixed width, either for the container or for the `Form` itself. (e.g. when used inside a 'popover').
*
* __Note:__ It's not recommended mixing `FormGroup`s with standalone `FormItem`s, either only use FormItems inside a Form, or wrap all items in one or more groups.
*/
const Form = forwardRef<HTMLFormElement, FormPropTypes>((props, ref) => {
const {
Expand Down Expand Up @@ -417,6 +424,7 @@ const Form = forwardRef<HTMLFormElement, FormPropTypes>((props, ref) => {
'--_ui5wcr_form_columns_l': columnsL,
'--_ui5wcr_form_columns_xl': columnsXL
}}
aria-label={titleText}
{...rest}
>
<div className={formClassNames}>
Expand Down
12 changes: 9 additions & 3 deletions packages/main/src/components/FormGroup/FormGroupTitle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,22 @@ import React, { type ElementType } from 'react';
import { classNames, styleData } from './FormGroupTitle.module.css.js';
import type { FormGroupPropTypes } from './index.js';

export function FormGroupTitle({ as, className, titleText, style, ...rest }: Omit<FormGroupPropTypes, 'children'>) {
export function FormGroupTitle({
as,
className,
titleText,
style,
uniqueId,
...rest
}: Omit<FormGroupPropTypes, 'children'> & { uniqueId: string }) {
useStylesheet(styleData, FormGroupTitle.displayName);
const CustomTag = as as ElementType;

return (
<CustomTag
id={`${uniqueId}-group`}
{...rest}
className={clsx(classNames.title, className)}
title={titleText}
aria-label={titleText}
data-component-name="FormGroupTitle"
style={style}
>
Expand Down
7 changes: 7 additions & 0 deletions packages/main/src/components/FormGroup/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ export interface FormGroupPropTypes extends CommonProps<HTMLHeadingElement> {
* @default "h5"
*/
as?: keyof HTMLElementTagNameMap;
/**
* Defines the [global `id` attribute](https://developer.mozilla.org/docs/Web/HTML/Global_attributes/id).
*
* __Note:__ Only override this prop if you want to implement screen reader announcements for groups manually.
*/
id?: HTMLHeadingElement['id'];
}

/**
Expand Down Expand Up @@ -57,6 +63,7 @@ const FormGroup = (props: FormGroupPropTypes) => {
<FormGroupTitle
{...rest}
titleText={titleText}
uniqueId={uniqueId}
style={{
...style,
display: titleText ? 'unset' : 'none',
Expand Down
8 changes: 8 additions & 0 deletions packages/main/src/components/FormItem/FormItem.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,11 @@
.lastGroupItem {
margin-block-end: 1rem;
}

.pseudoInvisibleText {
font-size: 0;
left: 0;
position: absolute;
top: 0;
user-select: none;
}
21 changes: 16 additions & 5 deletions packages/main/src/components/FormItem/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export interface FormItemPropTypes {
/**
* Content of the FormItem.
*
* __Note:__ Only ui5 web component inputs such as `Input (ui5-input)`, `CheckBox (ui5-checkbox)`,`DatePicker (ui5-date-picker)`, etc. are supporting screen readers. For all other inputs the labels have to be set manually.
*
* __Note:__ Text, numbers and React portals are ignored.
*/
children: FormItemContent;
Expand Down Expand Up @@ -173,14 +175,23 @@ const FormItem = (props: FormItemPropTypes) => {
// @ts-expect-error: type can't be string because of `isValidElement`
if (isValidElement(child) && child.type && child.type.$$typeof !== Symbol.for('react.portal')) {
const content = getContentForHtmlLabel(label);
const childId = child?.props?.id;
let accessibleNameRef: string | undefined;
if (!child?.props.accessibleName) {
accessibleNameRef =
child?.props?.accessibleNameRef ?? `${layoutInfo.groupId}-group ${uniqueId}-${index}-label`;
}

return (
<Fragment key={`${content}-${uniqueId}-${index}`}>
{/*@ts-expect-error: child is ReactElement*/}
{cloneElement(child, { id: childId ?? `${uniqueId}-${index}` })}
<label htmlFor={childId ?? `${uniqueId}-${index}`} style={{ display: 'none' }} aria-hidden={true}>
{accessibleNameRef
? cloneElement(child, {
//@ts-expect-error: child is ReactElement
accessibleNameRef
})
: child}
<span className={classNames.pseudoInvisibleText} id={`${uniqueId}-${index}-label`}>
{content}
</label>
</span>
</Fragment>
);
}
Expand Down
Loading