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.

Sign up for GitHub

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

Jump to bottom

[EuiComboBox][KEYBOARD]: Removing tabindex -1 from EuiComboboxPill SVG #5560

Merged
merged 5 commits into from
Jan 27, 2022
Merged

[EuiComboBox][KEYBOARD]: Removing tabindex -1 from EuiComboboxPill SVG #5560

merged 5 commits into from
Jan 27, 2022

Conversation

1Copenut
Copy link
Contributor

@1Copenut 1Copenut commented Jan 25, 2022 •
edited
Loading

Summary

Our EuiComboBox component had a nested SVG with a tabindex -1 passed into ...closeButtonProps that was causing axe to throw a violation. We removed the negative tabindex because the SVG is nested inside a standard button.

Closes #5545

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs and playground toggles
  • Added documentation
  • Checked Code Sandbox works for any docs examples
  • Added or updated jest and cypress tests
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5560/

@1Copenut 1Copenut marked this pull request as ready for review January 25, 2022 20:23
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5560/

@1Copenut 1Copenut requested a review from cee-chen January 27, 2022 15:02
thompsongl
thompsongl approved these changes Jan 27, 2022
View reviewed changes
Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

👍 Resolves the axe warning and the svg is still not focusable

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5560/

cee-chen
cee-chen approved these changes Jan 27, 2022
View reviewed changes
Copy link
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

🎉

cee-chen
cee-chen reviewed Jan 27, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5560/

@1Copenut 1Copenut merged commit 0e0dddb into elastic:main Jan 27, 2022
@1Copenut 1Copenut deleted the feature/tpierce-eui-5545 branch January 27, 2022 18:33
@breehall breehall mentioned this pull request Feb 8, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cee-chen cee-chen cee-chen approved these changes

@thompsongl thompsongl thompsongl approved these changes

Assignees

@1Copenut 1Copenut

@breehall breehall

Labels
accessibility - automated accessibility - keyboard accessibility - WCAG A Level A WCAG Accessibility Criteria
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[EuiComboBox][COGNITION]: Focusable close buttons should not have a nested element with tabindex
5 participants
@1Copenut @kibanamachine @cee-chen @thompsongl @breehall