fix(store-ui): Fix search-input a11y #1000
Conversation
|
✔️ Deploy Preview for storeui ready! 🔨 Explore the source changes: ce17943 🔍 Inspect the deploy log: https://app.netlify.com/sites/storeui/deploys/617172437c8e670007577153 😎 Browse the preview: https://deploy-preview-1000--storeui.netlify.app |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ce17943:
|
| describe('Accessibility', () => { | ||
| it('should have no violations', async () => { | ||
| const { getByTestId } = render(<Wrapper />) | ||
|
|
||
| expect(await axe(getByTestId('search-input'))).toHaveNoViolations() | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Can you put this test suite inside the Search Input test suite?
| /** | ||
| * Custom aria-label for input and button. | ||
| */ | ||
| ariaLabel?: string |
There was a problem hiding this comment.
IMHO, i prefer to use aria-label as the HTML does.
| ariaLabel?: string | |
| aria-label?: ArriaAttributes['aria-label'] |
There was a problem hiding this comment.
Also, could receive aria-labelledby.
| /** | ||
| * Custom aria-label for input and button. | ||
| */ | ||
| ariaLabel?: string |
There was a problem hiding this comment.
Also, could receive aria-labelledby.
| <Input | ||
| ref={valueRef} | ||
| aria-label={ariaLabel} | ||
| aria-labelledby={ariaLabelledBy} |
There was a problem hiding this comment.
Why is it necessary to explicitly put aria-labelledby here? I understand aria-label because you added a default value to it, but wouldn't aria-labelledby be added through {...props} anyway?
There was a problem hiding this comment.
Thank you for bringing that out, @lariciamota, your point makes total sense to me. I've added the aria-labelledby as Brasileiro suggested before, but I haven't reflect much on it. Any considerations about Laricia's point, @igorbrasileiro?
What's the purpose of this pull request?
This PR intends to solve accessibility (a11y) issues in the SearchInput component, by adding
aria-labelsto either the input and button in it. The lack of these a11y improvements is making this base.store PR fails.How it works?
When screen readers read the content of these
aria-labels, they will be capable of giving the correct feedback on what is being focused at the moment (since neither the input has a label element, nor the button has a value in it describing what they are/do).How to test it?
After importing the StoreInput component you can test it using the following methods:
aria-labelsbase.storeDeploy Previewbase.store deploy preview
References