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

Stepper accessibility fixes #4571

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
title: Accessibility
redirect_from:
- /components/stepper/accessibility/
---

## Accessibility

The Stepper component has been designed with accessibility in mind. It can be used with keyboard navigation and includes properties that enhance the experience for users of assistive technologies.

The following props provide additional information to screen readers, enhancing the accessibility of the respective component. By using them, you can ensure that users who rely on assistive technologies receive the necessary context and information about the component's purpose and functionality.

- The `ariaLabel` prop allows you to specify an `aria-label` attribute of the component.

- The `titleDecrement` prop allows you to specify an `aria-label` attribute for the decrement icon button in the Stepper (StepperStateless) component.

- The `titleIncrement` prop allows you to specify an `aria-label` attribute for the increment icon button in the Stepper (StepperStateless) component.

- The `ariaLabelledBy` prop allows you to specify an `aria-labelledby` attribute for the Stepper component. This attribute references the ID of the element that labels the Stepper, ensuring that screen readers announce the label correctly.

Although these props are optional for the Stepper (StepperStateless) component itself, it is recommended to fill them in to ensure that the component can be perceived by assistive technologies.

### Example

```jsx
<Stepper
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would maybe add more elements to this example, as it probably wouldn't be placed in isolation but with some label? (This is based on the comment I put to the review with the image example, I just wanted to post it also here in case we add all these other props.)

Copy link
Member Author

@sarkaaa sarkaaa Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added an example when using Stepper with a connected title (ariaLabelledBy prop is used).

step={1}
minValue={0}
minValue={10}
ariaLabel="Number of passengers"
titleDecrement="Remove a passenger"
titleIncrement="Add a passenger"
/>
```

The screen reader will announce the input title (`Number of passengers`) and buttons title (`Add a passenger`, `Remove a passenger`) once they are focused by the screen reader.

```jsx
<Stack>
<Stack>
<Text id="passengers">Passengers</Text>
</Stack>
<Stepper
step={1}
minValue={0}
minValue={10}
ariaLabel="Number of passengers"
aria-labelledby="passengers"
titleDecrement="Remove a passenger"
titleIncrement="Add a passenger"
/>
</Stack>
```

This example includes `ariaLabelledby` prop. In this case, `ariaLabelledBy` prop is prioritized over `ariaLabel`, so the screen reader will announce the input title (`Passengers`) and buttons title (`Add a passenger`, `Remove a passenger`) once they are focused by the screen reader.
19 changes: 8 additions & 11 deletions packages/orbit-components/src/Stepper/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,10 @@ Table below contains all types of the props available in Stepper component.
| onChange | `number => void \| Promise` | | Function for handling onClick event. |
| onFocus | `event => void \| Promise` | | Function for handling onFocus event. |
| step | `number` | `1` | Specifies the value of step to increment and decrement. |
| titleDecrement | `string \| (any => string)` | | Specifies `title` property on decrement `Button`. |
| titleIncrement | `string \| (any => string)` | | Specifies `title` property on increment `Button`. |

### size
DSil marked this conversation as resolved.
Show resolved Hide resolved

| size |
| :--------- |
| `"small"` |
| `"normal"` |
| titleDecrement | `string \| (any => string)` | | Specifies `ariaLabel` property on decrement `Button`. |
| titleIncrement | `string \| (any => string)` | | Specifies `ariaLabel` property on increment `Button`. |
| ariaLabel | `string` | | Optional prop for `aria-label` value. |
| ariaLabelledBy | `string` | | Optional prop for `aria-labelledby` value. |

## Functional specs

Expand Down Expand Up @@ -77,9 +72,11 @@ Table below contains all types of the props available in `StepperStateless` comp
| onIncrement | `event => void \| Promise` | | Function for handling increment event. |
| onKeyDown | `event => void \| Promise` | | Function for handling onKeyDown event present on input. |
| step | `number` | `1` | Specifies the value of step to increment and decrement. |
| titleDecrement | `string \| (any => string)` | | Specifies `title` property on decrement `Button`. |
| titleIncrement | `string \| (any => string)` | | Specifies `title` property on increment `Button`. |
| titleDecrement | `string \| (any => string)` | | Specifies `ariaLabel` property on decrement `Button`. |
| titleIncrement | `string \| (any => string)` | | Specifies `ariaLabel` property on increment `Button`. |
| value | `number \| string` | | Specifies the value of the StepperStateless. |
| ariaLabel | `string` | | Optional prop for `aria-label` value. |
| ariaLabelledBy | `string` | | Optional prop for `aria-labelledby` value. |

### Usage:

Expand Down
19 changes: 14 additions & 5 deletions packages/orbit-components/src/Stepper/Stepper.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,26 @@ export const Default: Story = {
disable: true,
},
},

args: {
ariaLabel: "Number of passengers",
titleIncrement: "Add a passenger",
titleDecrement: "Remove a passenger",
},
};

export const Playground: Story = {
args: {
...Default.args,
id: "stepper-ID",
name: "Passengers number",
name: "Number of passengers",
step: 1,
minValue: 0,
maxValue: 20,
defaultValue: 0,
active: false,
disabled: false,
maxWidth: 120,
titleIncrement: "Add a passenger",
titleDecrement: "Remove a passenger",
onChange: action("onChange"),
onFocus: action("onFocus"),
onBlur: action("onBlur"),
Expand Down Expand Up @@ -83,9 +88,9 @@ export const Stateless: Story & StoryObj<typeof StatelessStepper> = {
};

export const Rtl: Story = {
render: () => (
render: args => (
<RenderInRtl>
<Stepper />
<Stepper {...args} />
</RenderInRtl>
),

Expand All @@ -95,4 +100,8 @@ export const Rtl: Story = {
disable: true,
},
},

args: {
...Default.args,
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ const StepperStateless = ({
titleDecrement,
disabledIncrement,
disabledDecrement,
ariaLabel,
ariaLabelledBy,
}: Props) => {
const theme = useTheme();

Expand Down Expand Up @@ -77,8 +79,8 @@ const StepperStateless = ({
onDecrement(ev);
}
}}
title={titleDecrement}
icons={iconStyles}
title={titleDecrement}
/>
<input
className={cx(
Expand All @@ -103,6 +105,8 @@ const StepperStateless = ({
}}
onBlur={onBlur}
onFocus={onFocus}
aria-label={ariaLabel}
aria-labelledby={ariaLabelledBy}
readOnly
/>
<ButtonPrimitive
Expand All @@ -115,8 +119,8 @@ const StepperStateless = ({
onIncrement(ev);
}
}}
title={titleIncrement}
icons={iconStyles}
title={titleIncrement}
Copy link
Member Author

@sarkaaa sarkaaa Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of changing this prop name from title to ariaLabel in ButtonPrimitive and making it more specific, however, this would be (probably) BC. I searched where this prop is used and I've found it's used here.

I'm not sure if it makes sense to change this prop name, if it could be included within this PR or in a separate one.

Copy link
Contributor

@domihustinova domihustinova Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So your suggestion is to keep prop names in Stepper as titleIncrement and titleDecrement and change the prop on the ButtonPrimitive from title to ariaLabel so the usage would be ariaLabel={titleIncrement}?

But anyway, I think it's out of scope and this should be tackled when making Button/ButtonPrimitive accessible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that wouldn't make sense. :D My suggestion was to rename aria attribute in ButtonPrimitive (probably in a different PR) and also change titleIncrement (titleDecrement). I kept titleDecrement (titleIncrement) because of attribute naming in ButtonPrimitive.

As you said, I also think it's out of the scope of this PR, that's the reason why I didn't touch ButtonPrimitive at all.

/>
</div>
);
Expand Down
4 changes: 4 additions & 0 deletions packages/orbit-components/src/Stepper/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ const Stepper = ({ onChange, defaultValue = 0, maxWidth = 108, ...props }: Props
titleIncrement,
titleDecrement,
active,
ariaLabel,
ariaLabelledBy,
} = props;
return (
<StepperStateless
Expand All @@ -63,8 +65,10 @@ const Stepper = ({ onChange, defaultValue = 0, maxWidth = 108, ...props }: Props
id={id}
value={value}
name={name}
ariaLabel={ariaLabel}
titleIncrement={titleIncrement}
titleDecrement={titleDecrement}
ariaLabelledBy={ariaLabelledBy}
/>
);
};
Expand Down
2 changes: 2 additions & 0 deletions packages/orbit-components/src/Stepper/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export interface SharedProps extends Common.Globals {
readonly maxWidth?: string | number;
readonly maxValue?: number;
readonly minValue?: number;
readonly ariaLabel?: string;
readonly ariaLabelledBy?: string;
// Deviation from other stepper properties
readonly titleIncrement?: string;
readonly titleDecrement?: string;
Expand Down
Loading