-
Notifications
You must be signed in to change notification settings - Fork 34
docs(TextInput): audit accessibility #6732
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
Open
lisalupi
wants to merge
1
commit into
main
Choose a base branch
from
docs/a11y-text-input-audit
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+65
−2
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { Meta } from '@storybook/addon-docs/blocks' | ||
|
|
||
| <Meta title="UI/Data Entry/TextInput/A11y" /> | ||
|
|
||
| # TextInput - Accessibility | ||
|
|
||
| ## Description | ||
|
|
||
| A controlled or uncontrolled form input component that extends HTML input functionality. Supports text, password, URL, and email types with features like prefix/suffix elements, clearable value, show/hide password toggle, loading state, success/error validation states, and optional randomization. | ||
|
|
||
| ## Summary | ||
|
|
||
| 1. ✅ **Perceivable**: Labels and descriptions are properly associated; error states use aria-invalid | ||
| 2. ✅ **Operable**: Focus indicator on wrapper via :focus-within (acceptable pattern) | ||
| 3. ❌ **Understandable**: Uses deprecated `aria-label` pattern; missing `accessibleLabel` prop per RFC #6585 | ||
| 4. ⚠️ **Robust**: Password visibility toggle missing aria-pressed state; status messages not using live regions properly | ||
|
|
||
| ## Rules | ||
|
|
||
| Enforced by the component: | ||
|
|
||
| - ✅ [WCAG 3.3.2 - Labels or Instructions (Level A)](https://www.w3.org/WAI/WCAG22/Understanding/labels-or-instructions.html): Label component properly associated via htmlFor/id | ||
| - ✅ [WCAG 3.3.1 - Error Identification (Level A)](https://www.w3.org/WAI/WCAG22/Understanding/error-identification.html): Error state uses aria-invalid="true" and error message linked via aria-describedby | ||
| - ✅ [WCAG 2.4.6 - Headings and Labels (Level AA)](https://www.w3.org/WAI/WCAG22/Understanding/headings-and-labels.html): Labels are descriptive when provided | ||
| - ✅ [WCAG 2.4.7 - Focus Visible (Level AA)](https://www.w3.org/WAI/WCAG22/Understanding/focus-visible.html): Focus indicator applied to wrapper via `:focus-within` (acceptable pattern). Input has `outline: none`. Border color on focus exceeds 3:1 requirement | ||
| - ✅ [WCAG 4.1.2 - Name, Role, Value (Level A)](https://www.w3.org/WAI/WCAG22/Understanding/name-role-value.html): Input has proper accessible name via label or aria-label | ||
| - ⚠️ [WCAG 4.1.3 - Status Messages (Level AA)](https://www.w3.org/WAI/WCAG22/Understanding/status-messages.html): Description component uses `role="status"` even for static helper text (should only use for dynamic error/success). Additionally, both the Stack wrapper (`aria-live="polite"`) AND Description (`role="status"`) are live regions, causing potential **double announcements** to screen reader users | ||
| - ✅ [WCAG 1.3.5 - Identify Input Purpose (Level AA)](https://www.w3.org/WAI/WCAG22/Understanding/identify-input-purpose.html): Component supports `autoComplete` prop (passed to native input). Developers must add appropriate `autoComplete` value when input collects user information (name, email, tel, etc.) | ||
|
|
||
| To apply when using the component: | ||
|
|
||
| - [WCAG 3.3.3 - Error Suggestion (Level AA)](https://www.w3.org/WAI/WCAG22/Understanding/error-suggestion.html): Provide helpful error messages that suggest how to correct input errors | ||
| - [WCAG 1.1.1 - Non-text Content (Level A)](https://www.w3.org/WAI/WCAG22/Understanding/non-text-content.html): When using prefix or suffix props with icons, ensure icons are `aria-hidden="true"` if decorative, or provide text alternatives via `VisuallyHidden` or tooltip if informative | ||
| - [WCAG 3.3.2 - Labels or Instructions (Level A)](https://www.w3.org/WAI/WCAG22/Understanding/labels-or-instructions.html): **Do not use `placeholder` as a substitute for visible `label`**. Placeholder text disappears when user types, causing memory issues for users with cognitive disabilities. Use placeholder for example input format only (e.g., "DD/MM/YYYY"), not as the primary label | ||
| - ⚠️ [RFC #6585 - Accessible Icons and Hidden Labels](https://github.com/scaleway/ultraviolet/discussions/6585): Component uses deprecated `aria-label` pattern. Should migrate to `accessibleLabel` prop (for hidden labels) per new team standards | ||
| - **When using external `Label` or `Notice` components**: If you render `Label` and/or `Notice` (for helper/error/success messages) externally instead of using the built-in props, you **must** manually manage ARIA associations: | ||
| - Ensure `Label` has a `htmlFor` attribute matching the input's `id` | ||
| - Pass the `Notice` component's `id` to the input's `aria-describedby` prop | ||
|
|
||
| ## Accessibility issues | ||
|
|
||
| **High:** | ||
|
|
||
| - Show/hide password button missing `aria-pressed` state to indicate current visibility state. The button toggles between show/hide but screen readers cannot determine the current state. Should use `aria-pressed={isPasswordVisible}` or `aria-expanded={isPasswordVisible}`. | ||
| - Uses deprecated `aria-label` pattern. Per [RFC #6585](https://github.com/scaleway/ultraviolet/discussions/6585), should migrate to `accessibleLabel` prop for hidden labels or `tooltipLabel` for tooltip-based labels. Current `aria-label` prop should be deprecated in favor of the new pattern. | ||
|
|
||
| **Medium:** | ||
|
|
||
| - Password visibility toggle buttons lack accessible labels that indicate the action. "show" and "hide" are ambiguous without context. Should be "Show password" and "Hide password", or use `tooltipLabel` prop. | ||
| - **Duplicate live region announcements**: Both the Stack wrapper (`aria-live="polite"`) AND Description component (`role="status"`) are live regions. This causes screen readers to announce error/success messages twice. Remove `aria-live` from wrapper or conditionally apply `role="status"` only for dynamic error/success messages (not static helper text) | ||
| - Loading state uses a visual Loader component but no aria-busy or aria-live announcement to inform screen reader users that the input is temporarily unavailable. | ||
| - Clear button has `aria-label="clear value"` which is not descriptive enough. Should indicate what is being cleared, e.g., "Clear [field name]", or use `tooltipLabel` prop. | ||
|
|
||
| **Low:** | ||
|
|
||
| - Tooltip component wraps the input wrapper div but tooltips may not be accessible to keyboard-only users or screen reader users depending on implementation. | ||
|
|
||
| ## Dependencies | ||
|
|
||
| - Button component used for clear and password toggle buttons must be keyboard accessible and have proper focus states | ||
| - Tooltip component must be accessible to keyboard and screen reader users | ||
| - Loader component should have appropriate aria attributes for loading states | ||
| - Migration depends on RFC #6585 implementation: `accessibleLabel`, `tooltipLabel`, `tooltipDescription` props and `VisuallyHidden` component / `sr-only` CSS class | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the
tooltipLabelprop should define the label of the field itself and not some subcomponents, right ? Maybe we could have alabelsprop as an object with different labels used in the component. Same for the clear button below