-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Infra UI] Change waffle map node to button for accessibility #31764
Conversation
Pinging @elastic/infrastructure-ui |
💚 Build Succeeded |
@@ -66,13 +66,13 @@ export class Node extends React.PureComponent<Props, State> { | |||
<SquareOuter color={color}> | |||
<SquareInner color={color}> | |||
{valueMode ? ( | |||
<ValueInner> | |||
<ValueInner aria-label={`${node.name}, click to open menu`}> |
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.
Do we need to translate the aria labels?
border: none; | ||
&:focus { | ||
outline: none !important; | ||
border: 3px solid ${params => params.theme.eui.euiFocusRingColor}; |
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.
There is a euiFocusRingSize
variable in the theme. Should we use it?
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.
lgtm
💚 Build Succeeded |
…c#31764) * Fixes elastic#28158 - Change waffle map node to button for accessibility * intl-izing the aria-labels; changing size to eui
💚 Build Succeeded |
Summary
This PR fixes #28158 by changing the
ValueInner
div to a button for theNode
componentChecklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.This was checked for cross-browser compatibility, including a check against IE11Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsUnit or functional tests were updated or added to match the most common scenariosFor maintainers
This was checked for breaking API changes and was labeled appropriatelyThis includes a feature addition or change that requires a release note and was labeled appropriately