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

test(a11y): add focus to action story #1423

Merged
merged 10 commits into from
Oct 26, 2021
Merged

Conversation

rshen91
Copy link
Contributor

@rshen91 rshen91 commented Oct 13, 2021

Summary

Adding the ref change in the legend actions story to show focus being passed to the button once the popover is closed.

Oct-13-2021.15-24-55.mp4

Details

I opened elastic/kibana#115066 to have these changes in kibana, but this PR is just focused on the changes for this actions story.

Issues

Checklist

  • All related issues have been linked (i.e. closes #123, fixes #123)
  • The proper documentation and/or storybook story has been added or updated
  • Visual changes have been tested with all available themes including dark, light, eui-dark & eui-light

@rshen91 rshen91 marked this pull request as ready for review October 15, 2021 14:44
@rshen91 rshen91 requested a review from nickofthyme October 15, 2021 14:44
@nickofthyme
Copy link
Collaborator

nickofthyme commented Oct 26, 2021

Alright, so after taking a look I'm not sure what the best option is. Here are the options as I see it.

  • Your approach - User refs anchor button on their own and handles focus
  • Option A (123f9dc) - User calls onClose on the action component, we locate the popover button from eui classes and call focus if found. Not idea but this approach would be the lease amount of work.
  • Option B (cd60d80) - User calls onClose on the action component and sets ref on component. Essentially this is the same as your approach but we create and pass the ref down rather than having the user do it. Though even doing this for them this approach could be the most code needed.

The difficulty is that the since we don't know when the popover is close (as we do with the color picker) we must have the user call onClose on the action component.

Maybe the best approach is a helper hook in kibana to facilitate this process and wrap all actions in a way that is easy to replicate.

Note: I just committed the options for example they are not meant to be merged.

@nickofthyme nickofthyme added :accessibility Accessibility related issue :legend Legend related issue :all Applies to all chart types labels Oct 26, 2021
@rshen91 rshen91 merged commit 05cfa38 into elastic:master Oct 26, 2021
@nickofthyme
Copy link
Collaborator

🎉 This PR is included in version 38.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nickofthyme nickofthyme added the released Issue released publicly label Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:accessibility Accessibility related issue :all Applies to all chart types :legend Legend related issue released Issue released publicly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants