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.

Sign up for GitHub

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

Jump to bottom

ImageInput accessibility improvements #1644

Merged
merged 8 commits into from
Aug 9, 2022
Merged

ImageInput accessibility improvements #1644

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5d08d5a
fix ImageInput accessibility issues
amelako Jul 4, 2022
57f2762
update docs
amelako Jul 4, 2022
1903176
update aria-live to polite
amelako Jul 13, 2022
71c28c9
update snapshots
amelako Jul 13, 2022
9acb4c0
update docs
amelako Jul 15, 2022
325a5af
add changeset
amelako Jul 15, 2022
385b50d
update changeset
amelako Jul 18, 2022
e497fa2
address review comments
amelako Jul 19, 2022
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
14 changes: 14 additions & 0 deletions packages/circuit-ui/components/ImageInput/ImageInput.docs.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,17 @@ There are some caveats with this approach:
### Using a div element

<Story id="forms-imageinput--custom-component-div" />

## Accessibility
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm noticing other issues when using the ImageInput with Firefox and VoiceOver. Can you share your findings from SR testing so we can compare what we found and address any problems?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved it to polite after testing in isolation on storybook with Firefox,Chrome and VoiceOver. I will add recordings as reference. Let me know if you have any more findings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Attaching here the recording with Firefox and VoiceOver. For some reason after my Chrome update the screen reader stopped announcing message completely when I tested in isolation. Let me know if you can try on your side and let me know the result.

Screen.Recording.2022-07-15.at.15.36.56.mov

.

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue I saw (and I'm seeing it in your recording as well) is that when selecting a file, focus seems to be brought to the window, and the loading label is not announced (but that might also be because there's no live region there either 😅)

Fixing this would probably require more research though. I don't mind going ahead with this PR in the meantime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can sync on later then on this and the docs.


### Best practices

#### Error message associated with input field

Error messages should be simple and clear to understand, also provide guidance on how to correct mistakes.
The error messages must be associated with their respective controls (e.g. Input field), via labeling or `aria-describedby`.
The screen readers call the error message with a live region by applying an `aria-live` property or using one of the live region roles (e.g alert).

The `aria-describedby` is added to the Input field, and `aria-invalid="true"` when the error is triggered.
The `aria-live="assertive"` indicate that screen readers should immediately announce the error message rather than completing other announcements first.
amelako marked this conversation as resolved.
Show resolved Hide resolved
This increases the possibility that users are aware of the error message before they move focus out of the input.
8 changes: 7 additions & 1 deletion packages/circuit-ui/components/ImageInput/ImageInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ export const ImageInput = ({
onClick={handleClick}
disabled={disabled || isLoading}
aria-invalid={invalid}
aria-describedby="validation_hint"
amelako marked this conversation as resolved.
Show resolved Hide resolved
{...props}
/>
<StyledLabel
Expand Down Expand Up @@ -480,7 +481,12 @@ export const ImageInput = ({
<LoadingLabel>{loadingLabel}</LoadingLabel>
</LoadingIcon>
</InputWrapper>
<ValidationHint validationHint={validationHint} invalid={invalid} />
<ValidationHint
id="validation_hint"
aria-live="assertive"
amelako marked this conversation as resolved.
Show resolved Hide resolved
validationHint={validationHint}
invalid={invalid}
/>
</Fragment>
);
};
7 changes: 7 additions & 0 deletions packages/circuit-ui/components/ImageInput/__snapshots__/ImageInput.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ exports[`ImageInput styles should render a custom component 1`] = `
>
<input
accept="image/*"
aria-describedby="validation_hint"
aria-invalid="false"
class="circuit-1"
id="image-input_5"
Expand Down Expand Up @@ -663,6 +664,7 @@ exports[`ImageInput styles should render with an existing image 1`] = `
>
<input
accept="image/*"
aria-describedby="validation_hint"
aria-invalid="false"
class="circuit-1"
id="image-input_2"
Expand Down Expand Up @@ -1038,6 +1040,7 @@ exports[`ImageInput styles should render with default styles 1`] = `
>
<input
accept="image/*"
aria-describedby="validation_hint"
aria-invalid="false"
class="circuit-1"
id="image-input_1"
Expand Down Expand Up @@ -1438,6 +1441,7 @@ exports[`ImageInput styles should render with invalid styles 1`] = `
>
<input
accept="image/*"
aria-describedby="validation_hint"
aria-invalid="true"
class="circuit-1"
id="image-input_3"
Expand Down Expand Up @@ -1506,7 +1510,9 @@ exports[`ImageInput styles should render with invalid styles 1`] = `
</span>
</div>
<span
aria-live="assertive"
class="circuit-12"
id="validation_hint"
>
<div
class="circuit-13"
Expand Down Expand Up @@ -1837,6 +1843,7 @@ exports[`ImageInput styles should render with smaller button 1`] = `
>
<input
accept="image/*"
aria-describedby="validation_hint"
aria-invalid="false"
class="circuit-1"
id="image-input_4"
Expand Down
1 change: 1 addition & 0 deletions packages/circuit-ui/components/ValidationHint/ValidationHint.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ export const ValidationHint = ({

return (
<Wrapper
{...props}
invalid={props.invalid}
showValid={props.showValid}
hasWarning={props.hasWarning}
Expand Down