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

Make filter pills keyboard accessible #13331

Merged
merged 3 commits into from
Aug 8, 2017
Merged

Conversation

Bargs
Copy link
Contributor

@Bargs Bargs commented Aug 3, 2017

Fixes #12639

It's now possible to tab to each filter's actions and interact with them via the keyboard.

In order to get the actions to show/hide on both mouse hover and action focus I had to create a new filter-pill component that could manage a bit of state to track whether the user was interacting with a given pill or not.

Copy link
Contributor

@aphelionz aphelionz left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

The actual focusing works fine. I just noticed, that we don't have any issue yet, that none of that buttons has any text/label/name, meaning you can now focus them, but a visual impaired user would still not be able to use them at all.

Should we fix that issue directly with this PR and add aria-labels to all of them?

@Bargs
Copy link
Contributor Author

Bargs commented Aug 4, 2017

Great point @timroes, we might as well add those in this PR. Is aria-label the only thing we need to add or are there other attributes? These icons confuse regular users as well, I could add a title attribute, but I've read that's often discouraged.

@Bargs
Copy link
Contributor Author

Bargs commented Aug 4, 2017

jenkins, test this

1 similar comment
@Bargs
Copy link
Contributor Author

Bargs commented Aug 4, 2017

jenkins, test this

@timroes
Copy link
Contributor

timroes commented Aug 7, 2017

I think to make the meaning more clear to seeing users, our current approach would be to add a hover tooltip to it (same as e.g. in the collapsed main navigation). You could give each of that tooltips a unique id attribute and put an aria-labelledby to the button, that references that id, so the content of the tooltip will automatically be used as a label for the button. I would also add aria-hidden="true" to the tooltip in that case, so you would prevent screen readers from also navigating over the tooltip, which might cause (depending on DOM order) strange behavior like: Negate filter, button -> next element -> Negate filter -> next element -> ...

@Bargs
Copy link
Contributor Author

Bargs commented Aug 7, 2017

That sounds awesome, but maybe a bit more than I want to bite off in this PR 😅

I'll just add the aria-labels for now, and create a new issue for the tooltip enhancement.

@Bargs
Copy link
Contributor Author

Bargs commented Aug 8, 2017

@timroes I added the aria-labels

Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

LGTM

@Bargs Bargs merged commit 542d553 into elastic:master Aug 8, 2017
Bargs added a commit to Bargs/kibana that referenced this pull request Aug 8, 2017
Fixes elastic#12639

It's now possible to tab to each filter's actions and interact with them via the keyboard.

In order to get the actions to show/hide on both mouse hover and action focus I had to create a new filter-pill component that could manage a bit of state to track whether the user was interacting with a given pill or not.
Bargs added a commit to Bargs/kibana that referenced this pull request Aug 8, 2017
Fixes elastic#12639

It's now possible to tab to each filter's actions and interact with them via the keyboard.

In order to get the actions to show/hide on both mouse hover and action focus I had to create a new filter-pill component that could manage a bit of state to track whether the user was interacting with a given pill or not.
Bargs added a commit that referenced this pull request Aug 8, 2017
Fixes #12639

It's now possible to tab to each filter's actions and interact with them via the keyboard.

In order to get the actions to show/hide on both mouse hover and action focus I had to create a new filter-pill component that could manage a bit of state to track whether the user was interacting with a given pill or not.
Bargs added a commit that referenced this pull request Aug 8, 2017
Fixes #12639

It's now possible to tab to each filter's actions and interact with them via the keyboard.

In order to get the actions to show/hide on both mouse hover and action focus I had to create a new filter-pill component that could manage a bit of state to track whether the user was interacting with a given pill or not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants