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

Display outline on control buttons when focused #8520

Merged
merged 4 commits into from
Jul 23, 2019

Conversation

ryanhamley
Copy link
Contributor

Launch Checklist

Fixes #8246

  • briefly describe the changes in this PR
    • this creates an outline on the control buttons when they are tab focused, which helps with accessibility issues
    • this avoids using the CSS outline property because outlines do not follow rounded borders. box-shadow does follow rounded borders, leading to a cleaner looking UI while simulating the outline effect (not sure why the GIF is so slow but you get the idea)

buttons

Copy link
Collaborator

@andrewharvey andrewharvey left a comment

Choose a reason for hiding this comment

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

This looks and works great!

@life777
Copy link

life777 commented Jul 20, 2019

@ryanhamley I agree, your solution looks better. Thanks

Copy link
Member

@mourner mourner 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 to me, me pending lint fixes. One thing to note is that the outline will now appear when you click on the buttons with a mouse, but that's probably OK.

@ryanhamley
Copy link
Contributor Author

ryanhamley commented Jul 22, 2019

I fixed the lint issue and added a couple new rules that will only display the outline when the buttons are focused by keyboard input using the :focus-visible selector. This is currently only supported out of the box in Firefox. It's also supported in Chrome behind the enable-experimental-web-platform-features flag. Normally I wouldn't use an experimental feature, but this is part of the W3C spec and seems very likely to be implemented widely in the not-too-distant future; it's harmless for browsers that don't implement it and makes the UI a touch nicer for browsers that do. That being said, I'm not real attached to it so we can remove those two rules if anyone has an objection.

Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

🍏 I didn't know about focus-visible but it totally makes sense to use it, because it adheres to the principles of progressive enhancement — browsers that support it will get enhanced experience while browsers that don't are still fully functional.


.mapboxgl-ctrl-group > button:focus-visible {
box-shadow: 0 0 2px 2px rgba(0, 150, 255, 1);
}
Copy link
Member

Choose a reason for hiding this comment

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

Nit: you can group these two selectors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually can't because I just realized Firefox uses the -moz-focusring selector instead of focus-visible so combining them makes the entire rule invalid in Firefox which breaks all focusing on the buttons. It turns out that Firefox and Safari don't appear to register clicks as focusing the button so they just work without this rule. I'll still leave it though because it's a progressive enhancement in Chrome and presumably newer versions of Edge.

@ryanhamley ryanhamley merged commit abb2dd6 into master Jul 23, 2019
@ryanhamley ryanhamley deleted the controls-outline-rounded-corners branch July 23, 2019 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Focused control button doesn't defer visually from non-focused
4 participants