-
Notifications
You must be signed in to change notification settings - Fork 56
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
fix(comboBox): refactor components for accessibility #636
base: master
Are you sure you want to change the base?
fix(comboBox): refactor components for accessibility #636
Conversation
I just did a quick look on the PR, but looks like it tries to fix multiple unrelated issues in many component. It would be nice if this PR have a proper description about which issues handled. |
@maxinteger The modifications to ListBoxItem and ListBoxBase are all based on the functionality of Combobox, (add data-focused to the element to implement the focus style), Why? Regarding Popover, there are two changes here, one is to remove aria-hidden from the tippyContainer (to ensure that the focus of the SR can enter the Popover), and the other is to fix the positioning of the Popover. I agree with you, so I'm going to split out the part of Popover's fix positioning, and let me know if you have any other suggestions! |
@Tianhui-Han Thank you for the explanation. You should split up this PR into digestible size ones. I still see changes in the Popover folder are they needed for the combobox changes? Please note there are many change happened around the menus and list items in the last few week. |
@maxinteger Previously, the changes related to option in popover were removed. The remaining changes are to support the accessibility of combobox. See the figure below. Here, the attribute aria-hidden='true' is present in tippy, which will cause the screen reader to be unable to read the content. This is the problem solved by the changes in popover. |
80c070e
to
6cac017
Compare
IMPORTANT
Currently, this project is closed to any external contributions. Any pull request made against this project from external sources will likely be closed. If you would like to make changes to this project, please fork this project.
Guide
This "Help" section can be deleted before submitting this pull request.
Update the name of this pull request to reflect the following shape:
Provide a general summary of the scope of the changes in this pull request.
Description
refactor components for accessibility
In total, the changes involve four components, Combobox, Popover, ListBoxItem and listBoxBase:
The modifications to ListBoxItem and ListBoxBase are all based on the functionality of Combobox, (add data-focused to the element to implement the focus style), Why?
Since the Combobox implementation based on React-aria, when we use the keyboard to select the listItem (using the up and down keys to switch targets), the focus is actually still on the input, so we need to do an additional focus hint here.
Regarding Popover, there are two changes here, one is to remove aria-hidden from the tippyContainer (to ensure that the focus of the SR can enter the Popover), and the other is to fix the positioning of the Popover.
The full description of the changes made in this request.
Links
https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-545812
Links to relevent resources.