Skip to content

TableView overlay auto tab stop Firefox fix #3520

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

Merged
merged 1 commit into from
Sep 19, 2022
Merged

Conversation

ktabors
Copy link
Member

@ktabors ktabors commented Sep 15, 2022

Closes

Found during testing, but pre-existing issue. In Firefox, an element with overflow auto or scroll is a tab stop. This caused an empty table to be tabbed too twice.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • 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:

In FireFox, go to any TableView story with no content (renderEmptyState, isLoading, etc.) and tab into the TableView. It should only stop once on the table body during keyboard tab navigation.

🧢 Your Project:

RSP

@adobe-bot
Copy link

@ktabors ktabors added the small review Easy to review PR label Sep 15, 2022
Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

Seem to work well in FF, Chrome, and Safari, TalkBack, and VO. Since this tabindex=-1 is on the ScrollView it shouldn't interfere with the "restore focus back to the collection" code that useVirtualizer/useSelectableCollection has.

One interesting note is that this adds an additional "focus" target that TalkBack can access now because of the new tabindex but I think this is a minor point IMO

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.

Seems fine, didn't notice any issues with the focus changes we recently made to collections. Cases such as deleting everything in ActionGroup stories.
BTW, we should link this in the wiki https://bugzilla.mozilla.org/show_bug.cgi?id=616594

@ktabors ktabors merged commit b81cc19 into main Sep 19, 2022
@ktabors ktabors deleted the firefox_table_tabstop branch September 19, 2022 20:43
@ktabors ktabors added needs testing and removed small review Easy to review PR labels Sep 19, 2022
@dannify dannify changed the title TableView overlay auto tab stop Firefox fix TableView overflow auto tab stop Firefox fix Oct 3, 2022
@dannify dannify changed the title TableView overflow auto tab stop Firefox fix TableView overlay auto tab stop Firefox fix Oct 3, 2022
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.

5 participants