:focus styles should be in sync with .focus styles, so it might be better to copy these lines:

to:

(wait a sec before applying any changes, would like to have a second opinion from @mdo or @ysds)

@MartijnCuppens I agree :focus must be in sync with .focus. Very good point, thank you, I missed that. I'm for copy styles to:

One idea come to my mind: If there should be always available .focus styles where is :focus across the whole Bootstrap, if this is some kind of standard, then .focus may should be added to hover mixins. Like e.g. this:

@mixin hover-focus { 
   &:hover, 
   &:focus,
   &.focus
   { 
     @content; 
   } 
} 

etc. The others should be changed accordingly (:active = .active).
But this is may be to much right?

I look forward to your and @mdo / @ysds views.

I’m against adding a .focus across the board. I can’t think of why we even have it on buttons honestly—everything can be done with :focus. As for :active/.active`, it’s an unfortunately named class that means “selected” more than “active”. The pairing of the names is circumstantial.

@mdo
I searched a bit, and I found a reason for the existence of the .focus class on buttons.
Historically, it has been added here #13907
Actual Button plugin still adding .focus class on focus.

So @MartijnCuppens is right - :focus should still remain in sync with .focus class. Until someone revises whether it is still needed do add .focus on focus.

So I'm going to prepare new commit where styles will be copied to :focus, .focus definition.

I'm not really convinced :focus and :hover states should have the same background. I like the current behaviour where I get :hover feedback when I hover a focused button. But I do like the color fix to prevent the behaviour in example 2.

I would go for adding color: color-yiq($background); to the focus state and move the hover styles under the focus block (to keep the color change on hover if needed). This would lead to the same result we have today, but it would fix the red btn in your second example.

What do you think?

1) color

Agreed. Add color: color-yiq($hover-background); to focus state.

2) background

So I tested the case when is default a styled not only with color but also background to look e.g. like:

("text utilities" is focused link with $danger color, $danger outline and $yellow background)

then button :focus state on .btn made from <a> looks like this:

so it follows that we should set some background to buttons focus state to ensure consistency of the buttons. So if you @MartijnCuppens prefer (and I agree so) to keep normal button state background, then we should add @include gradient-bg($background); to

3) border

A case where someone would add a border to link is really marginal (almost crazy? :). For demonstration it behaves as expected:

then button :focus state on .btn made from <a> looks like this:

IMHO we don't have to deal with this crazy rare case.

Summary:

I propose to add color: color-yiq($hover-background); and @include gradient-bg($background); to focus state:

&:focus,
&.focus {
	color: color-yiq($hover-background);
	@include gradient-bg($background);
	...
}

What do you think?

Hello everybody who is in charge here :]. So what? @MartijnCuppens please :]

Just summarize this:

This change is only in one file /scss/mixins/_buttons.scss.
Into this focus state

adding these lines of code:

&:focus,
&.focus {
  color: color-yiq($hover-background);
  @include gradient-bg($hover-background);
  border-color: $hover-border;
  // Avoid using mixin so we can pass custom focus shadow properly

(here is diff of /scss/mixins/_buttons.scss)

to fix issues specified above in my last issuecomment ie color, background and border style.

The intent is override customized user setup for link styles (basic a href) to keep consistency of focus state of buttons made from a hrefs.

IMHO This can not hurt anyone.
What do you think?

@MartijnCuppens: does this need to be backported to v4?

Yeah, this fixes issues with focus styles. Good call @XhmikosR