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

Suppress focus outline for buttons when it shouldn't be visible in Chromium #32689

Merged
merged 3 commits into from
Jan 10, 2021

Conversation

patrickhlauke
Copy link
Member

@patrickhlauke patrickhlauke commented Jan 6, 2021

Follow-up to #32631

To see this in action easily, using Chrome, check one of the "Copy" buttons in the documentation. Currently, they end up with the two-tone outline (even though they shouldn't), but with this additional "hack" (more of a restating of what the browser should be doing), the outline only shows if you're setting focus with the keyboard, not mouse or touch tap

Preview: https://deploy-preview-32689--twbs-bootstrap.netlify.app/

@ffoodd
Copy link
Member

ffoodd commented Jan 6, 2021

I'm definetely in, that's a first step at using :focus-visible appropriately.

However regarding your comment on #32631, I think it means a few older WebKit browsers will still show focus outlines (based on Can I Use).

I personnaly don't care that much, but like to at least mention this.

@XhmikosR
Copy link
Member

XhmikosR commented Jan 7, 2021

Removed from backporting. @mdo your call

@patrickhlauke
Copy link
Member Author

@mdo thoughts on backporting?

@mdo
Copy link
Member

mdo commented Jan 9, 2021

@patrickhlauke I'd say backport this and #32631 for sure.

@XhmikosR
Copy link
Member

The only reason I wasn't sure about backporting is due to @ffoodd's comment above which v4 supports, so this will be inconsistent?

@patrickhlauke
Copy link
Member Author

The only reason I wasn't sure about backporting is due to @ffoodd's comment above which v4 supports, so this will be inconsistent?

this is more of a nice-to-have progressive enhancement, so personally think it's ok to use even if it's not supported by all browsers that are targeted by v4

@patrickhlauke patrickhlauke merged commit a2ae2c3 into main Jan 10, 2021
@patrickhlauke patrickhlauke deleted the patrickhlauke-button-focus-fix branch January 10, 2021 10:42
@XhmikosR
Copy link
Member

@patrickhlauke feel free to push any backports to my v4-dev-xmr branch which is #32748. Should be the last batch of patches before 4.6.0, hopefully :)

@patrickhlauke
Copy link
Member Author

patrickhlauke commented Jan 10, 2021

yup, on it today hopefully.

[edit: done]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants