-
Notifications
You must be signed in to change notification settings - Fork 7
Libr311 Accessibility - Issue #25 #302
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
base: main
Are you sure you want to change the base?
Conversation
When verifying focus traversal for Map View, it was apparent the Center Map option wasn't focusable Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
| div.style.display = 'flex'; | ||
| div.style.justifyContent = 'center'; | ||
| div.style.alignItems = 'center'; | ||
| const control = L.DomUtil.create('a'); |
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.
I rename the field in case for some reason we make it a button which also focusable.
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.
Since the href of this "a" element is just set to "#" (not navigating to a new URL), it seems like this really should be a "button" element instead of an "a" element.
…ocus wasn't highlighting like the zoom buttons. Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
…ocus wasn't highlighting like the zoom buttons. Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
| control.appendChild(img); | ||
| control.setAttribute('role', 'button'); | ||
| control.setAttribute('title', 'Center Map'); | ||
| control.setAttribute('aria-label', 'Center Map'); |
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.
"Center Map" being the same prevents NDVA and Narrator from repeating.
| } | ||
|
|
||
| control.addEventListener('mouseenter', highlight); | ||
| control.addEventListener('focus', highlight); |
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.
Handle focus like the Zoom In and Zoom out buttons
Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
| control.setAttribute('title', 'Center Map'); | ||
| control.setAttribute('aria-label', 'Center Map'); | ||
| control.href = '#'; | ||
| control.style.backgroundColor = '#fff'; |
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.
I wonder why all this styling was done in JavaScript instead of in a CSS file. It probably should be moved to CSS.
When verifying focus traversal for Map View, it was apparent the Center Map option wasn't focusable