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.

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(#6130): Ignore HiddenSelect when tree walking #6139

Merged

Conversation

majornista
Copy link
Collaborator

@majornista majornista commented Apr 2, 2024

We should skip HiddenSelect element in Picker when tree walking to determine the next or previous focusable element. However, we should not use [data-a11y-ignore], which is used to exclude elements from aXe automated accessibility tests.

Per comments: #6116 (comment)

Closes #6130

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue 6130.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

  1. Open TableView focusable cells example: https://reactspectrum.blob.core.windows.net/reactspectrum/b783832dd2ff7bf0a85ee30f1233ef9ce370b661/storybook/iframe.html?scrolling=false&providerSwitcher-locale=&providerSwitcher-theme=&providerSwitcher-scale=&providerSwitcher-express=false&args=&id=tableview--focusable-cells&viewMode=story
  2. The example app consists of a TableView in which the last row has a Picker in the second cell.
  3. Press Tab to navigate and set focus to the first row in the TableView.
  4. Press ArrowDown until the last row, containing the Picker has focus.
  5. Press ArrowRight to focus the Switch in the first cell.
  6. Press ArrowRight again to focus the Picker in the second cell.
  7. Press ArrowRight to move focus to the next cell, containing the word "Three."
  8. The value of the Picker does not change as the focus moves to the next cell.
  9. Press ArrowLeft to focus the Picker in the second cell again.
  10. Press ArrowLeft again to move focus to the previous cell.
  11. Focus moves to the Switch in the first cell, and the value of the Picker does not change.

Related Issue and PR: #5877, #5878, #6116

🧢 Your Project:

Adobe/Accessibility

@majornista majornista linked an issue Apr 2, 2024 that may be closed by this pull request
We should skip HiddenSelect element in Picker when tree walking to determine the next or previous focusable element. However, we should not use [data-a11y-ignore], which is used to exclude elements from aXe automated accessibility tests.
@majornista majornista force-pushed the 6130-iselementvisible-remove-unnecessary-attribute-check branch from e659367 to b783832 Compare April 2, 2024 18:53
@rspbot
Copy link

rspbot commented Apr 2, 2024

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

I'm happier with this. I also feel like data-hidden might be a more common than 0 chance, so I'm happier with the longer name for it as well. I think we can still change the name down the line as well, but at least now it's separated from tooling

@rspbot
Copy link

rspbot commented Apr 6, 2024

@rspbot
Copy link

rspbot commented Apr 6, 2024

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }

@LFDanLu LFDanLu merged commit 24d931b into main Apr 6, 2024
25 checks passed
@LFDanLu LFDanLu deleted the 6130-iselementvisible-remove-unnecessary-attribute-check branch April 6, 2024 00:11
@ashfaque-pixelotech
Copy link

what is the actual solution of this problem i am facing issue with Input, combobox inside table i am using react-aria-components

@LFDanLu
Copy link
Member

LFDanLu commented Apr 8, 2024

@ashfaque-pixelotech Are you running into issues with keyboard navigation and combobox inside the table?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

isElementVisible: remove unnecessary attribute check
5 participants