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

Add outline for control buttons while navigating with keyboard #8457

Closed
wants to merge 1 commit into from

Conversation

life777
Copy link

@life777 life777 commented Jul 10, 2019

This PR solves issue #8246.

If you have some controls on the map (like zoom controls) and you navigate to them with Tab button you couldn't distinguish what button is focused now. This PR adds back an outline for buttons to show what control is focused.

Launch Checklist

  • briefly describe the changes in this PR
  • post benchmark scores
  • manually test the debug page

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 good, thank you.

@andrewharvey
Copy link
Collaborator

Screenshot from 2019-07-11 11-59-12

I did notice this, but it's only a minor issue, so I don't mind merging even with this issue.

@andrewharvey
Copy link
Collaborator

I also tested this with other controls, MapboxGeocoder, MapboxDirections and this change didn't seem to have any negative effect.

@ryanhamley
Copy link
Contributor

Hey @life777 thanks for bringing up this issue and taking a stab at it. I created a PR #8520 that simulates the outline with box-shadow in order to avoid the issue @andrewharvey showed where the outline does not follow the rounded corners of the buttons. I'm inclined to close this in favor of the other approach but thanks a lot!

@ryanhamley ryanhamley closed this Jul 19, 2019
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.

3 participants