-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[a11y] Autocomplete and text field a11y docs #968
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
Conversation
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.
Yes! Content is looking good. Made some comments, asked some questions 🙂
|
||
The autocomplete component is based on the [ARIA 1.1 combobox pattern](https://www.w3.org/TR/wai-aria-practices-1.1/#combobox). See the [text field component](https://polaris.shopify.com/components/forms/text-field) for information on implementing the autocomplete component with a text field. | ||
|
||
We recommend implementing the autocomplete list below the text input so that it’s easy to discover and use. However, you can change the position with the `preferredPosition` prop. |
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.
A couple of questions:
- Does line 435 refer to implementing autocomplete with a text field? Or is it relevant to autocomplete on the whole?
- If you don't change the position with the
preferredPosition
prop, then does the autocomplete list show below the text input by default?
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 believe we only support using the autocomplete with a text field but there are similar features in admin that are used with buttons. I'll clarify that it's below the control by default.
src/components/TextField/README.md
Outdated
|
||
- Use the `disabled` prop to add the HTML `disabled` attribute to the text field | ||
- Use the `readOnly` prop to add the HTML `readonly` attribute to the text field | ||
- If you use the `type` prop, then some technologies update the software keyboard provided to the user to be more specific to the task at hand. This helps merchants with mobility, vision, and cognitive issues to enter information more easily |
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.
Because '"provided to the user" might be implied, and "task at hand" is idiomatic and might be difficult for ESL readers or translations, maybe we can rephrase to something like:
If you use the `type` prop, then some technologies update the software keyboard to be more specific to the current task.
or even
If you use the `type` prop, then some assistive technologies adapt the software keyboard to the current task.
Since one of the bullets here has two sentences, we can add periods to all of the bullets in the list
src/components/TextField/README.md
Outdated
|
||
#### Automatically focusing | ||
|
||
In general, avoid focusing on fields automatically. However, you can use the `autoFocus` prop to automatically move focus to the text field. This prop is set to `false` by default and should only be used in cases where it won’t force focus to skip other controls or content of equal or great importance importance. |
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 paragraph flows a little better if we rephrase so that the first sentence is at the end:
Use the `autoFocus` prop to automatically move focus to the text field. This prop is set to `false` by default and should only be used in cases where it won’t force focus to skip other controls or content of equal or greater importance. In general, avoid focusing on fields automatically.
That said, if it's important to make sure that folks don't miss the message about it being best to not automatically focus, then maybe we can say something like:
Although you can use the `autoFocus` prop to automatically move focus to the text field, it’s generally best to avoid focusing on fields automatically. The `autoFocus` prop is set to `false` by default and should only be used in cases where it won’t force focus to skip other controls or content of equal or greater importance.
Co-Authored-By: dpersing <devon.persing@shopify.com>
Co-Authored-By: dpersing <devon.persing@shopify.com>
Co-Authored-By: dpersing <devon.persing@shopify.com>
Co-Authored-By: dpersing <devon.persing@shopify.com>
Co-Authored-By: dpersing <devon.persing@shopify.com>
Co-Authored-By: dpersing <devon.persing@shopify.com>
Co-Authored-By: dpersing <devon.persing@shopify.com>
Co-Authored-By: dpersing <devon.persing@shopify.com>
@sadiesaurus Updated/added all your recs! |
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.
Looks great, aside from the UNRELEASED.md that contains unrelated entries.
* Add a11y content to text field component * Add a11y content for the autocomplete page * Added entry to UNRELEASED.md
Adds accessibility documentation for the (related) text field and autocomplete components.
WHY are these changes introduced?
Provides guidance on using the text field and autocomplete components as part of accessibility documentation updates. These update are available in component docs on
polaris-react
and the style guide site.See the draft Google doc for editing history for these changes and updates for other components and pages.
WHAT is this pull request doing?
UNRELEASED.md
How to 🎩
master
frompolaris-styleguide
to get the changes that support the accessibility section.polaris-react
and run the instructions for testing in a consuming project.polaris-styleguide
tab, rundev up && dev start
.Screenshots
Autocomplete
Text field (web)