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: improve accessibility of failure instance card #3502

Merged
merged 18 commits into from
Oct 29, 2020
Merged

fix: improve accessibility of failure instance card #3502

merged 18 commits into from
Oct 29, 2020

Conversation

karanbirsingh
Copy link
Contributor

@karanbirsingh karanbirsingh commented Oct 20, 2020

Description of changes

This PR implements the proposed solution from this comment on 1796. This ends up resolving #3421 as well.

The general idea is to remove tab-focus from the card itself and instead create a hidden button that manages the highlight state.

The visual behavior should be identical for the interactive card in fastpass. The following things are different in the report card:

  • the card is not a tab stop, and receives no visual focus
  • the cursor is no longer a pointer when hovered over the card (because there's nothing to click)
  • the card no longer is styled differently on hover (because there's nothing to click)

Here is a gif of the interactive card behavior in Automated Checks after this PR:

capture

Here is a gif of the report card behavior after this PR (mouse location slightly off, recording error):

capture

@RobGallo reviewed this branch locally with a screen reader to validate the experience.

We ensure that clicking on the card moves keyboard focus to the hidden button, and that focusing on the hidden button applies visual focus on the card. cardFocused state is introduced to allow button onFocus/onBlur handlers to affect focus styling on the card. onClick behavior on the card is added to forward focus/click events to the hidden button. The initial implementation (no card tabindex) suffered from focus-style flickering on card-click because the onBlur handler fired before the click handler that restores button focus. We explored two options to fix this. They have identical keyboard behavior, but differ on-card-click.

  • Setting tabindex to -1 on the card. This keeps the card out of tab-order, but allows card-clicks to trigger :focus state so visual focus doesn't disappear and re-appear. When the card is clicked, NVDA reads extra information about the card before focus is moved to the hidden button. This is the approach in this PR.
  • Deferring the focus updates until we have associated isSelected prop updates to match; deferring the update was awkward / required extra state variables, but ensured visual focus style didn't flicker. In this approach NVDA only reads information about the hidden button.

@iamrafan pointed out that the placement of the hidden button relative to its table affects where NVDA goes upon tab / shift-tab after pressing T to go to a table. I moved the button after the table to align with his feedback so that pressing T when focus is on the WCAG link goes to the first table - then pressing shift-tab returns to the WCAG link (as opposed to returning to the button). When the first card is selected, at least visually it will look like we're focused on the first table; pressing T takes us to the next table.

Pull request checklist

@karanbirsingh karanbirsingh marked this pull request as ready for review October 20, 2020 22:55
@karanbirsingh karanbirsingh requested a review from a team as a code owner October 20, 2020 22:55
@karanbirsingh karanbirsingh added the pr: second review required Author has requested an additional review. label Oct 20, 2020
@karanbirsingh
Copy link
Contributor Author

@ferBonnin @LiLoDavis I hope you can take a look at the GIFs/description as well since this PR changes UI behavior

@LiLoDavis
Copy link

@ferBonnin @LiLoDavis I hope you can take a look at the GIFs/description as well since this PR changes UI behavior

This looks good to me.

@karanbirsingh
Copy link
Contributor Author

Working on two updates:

  • place focus on button when card is clicked
  • ensure button padding/margin is 0px

@karanbirsingh karanbirsingh marked this pull request as draft October 21, 2020 23:02
@karanbirsingh karanbirsingh marked this pull request as ready for review October 26, 2020 23:55
… about NVDA table command / tab expectations
Copy link
Contributor

@dbjorge dbjorge left a comment

Choose a reason for hiding this comment

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

Code and usability both LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: second review required Author has requested an additional review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants