-
Notifications
You must be signed in to change notification settings - Fork 844
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
Deprecating EuiKeyboardAccessible #4135
Deprecating EuiKeyboardAccessible #4135
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_4135/ |
c0c515f
to
f7cdaf4
Compare
f7cdaf4
to
eed5d59
Compare
Preview documentation changes for this PR: https://eui.elastic.co/pr_4135/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_4135/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_4135/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_4135/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple small things
+1 to finding a way around the li
click event pass-through
One thing real quick is that CI is throwing this error because you probably forgot to change a token key when copy/pasting. 18:26:55 Token euiStepStrings.warning has two differing defaults:
18:26:55 Step {number} has warnings
18:26:55 Step {number}: {title} has warnings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The EuiStep rendering issues all seem to be fixed and display as they did before. 👍 CI is having some issues though, so be sure to fix those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to merge after these changes
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Greg Thompson <thompson.glowe@gmail.com>
Preview documentation changes for this PR: https://eui.elastic.co/pr_4135/ |
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
Preview documentation changes for this PR: https://eui.elastic.co/pr_4135/ |
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Greg Thompson <thompson.glowe@gmail.com>
Summary
Closes #3838. After this PR is merged, should update the deprecation schedule to include removing EuiKeyboardAccessible and create another issue for addressing EuiBasicTable's a11y issues which are outside the scope of this deprecation.
Adds docs about deprecation
Fixes EuiCodeEditor
Though EuiCodeEditor didn't use EuiKeyboardAccessible, because it mentioned it in the comments, it came up in this work. It used EuiKeyboardAccessible as a template for how to fix a problem that was better fixed other ways.
Fixes EuiStepsHorizontal
While removing the use of EuiKeyboardAccessible, also took the opportunity to fix up a few other a11y issues with the component, simplify its semantics, and align its string usage across all instances.
Ignores EuiBasicTable
EuiBasicTable uses EuiKeyboardAccessible but unwinding that decision would be a whole can of worms. To deprecate EuiKeyboardAccessible as quickly as possible, pushing off any EuiBasicTable work till a decision on how to deal with it can be reached.
Checklist