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

text-emphasis-variant mixin should change the anchor's "focused" state as well #16047

Closed
wants to merge 1 commit into from
Closed

Conversation

monoblaine
Copy link
Contributor

The ".text-emphasis-variant" mixin does not change the anchor's color when the anchor is in the focused state, causing the link to have the default a:focus color (defined in https://github.com/twbs/bootstrap/blob/master/less/scaffolding.less#L53).

@cvrebert cvrebert added the css label Mar 11, 2015
@cvrebert
Copy link
Collaborator

(Note to team: If approved, this will require rebasing prior to merging so that the commit message can be changed to something more descriptive.)

@monoblaine monoblaine changed the title Update text-emphasis.less text-emphasis-variant mixin should change the anchor's "focused" state as well Mar 12, 2015
@@ -2,7 +2,7 @@

.text-emphasis-variant(@color) {
color: @color;
a&:hover {
a&:hover, a&:focus {
Copy link
Member

Choose a reason for hiding this comment

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

If we did this, this new selector would need to appear on the next line like so:

a&:hover,
a&:focus {

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm happy to fix that when rebasing this. Do you approve, in principle, of adding the selector?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, seems fine to me. Let's do it.

@cvrebert cvrebert modified the milestone: v3.3.5 Mar 15, 2015
@cvrebert cvrebert closed this in ba2d556 Mar 26, 2015
@cvrebert cvrebert mentioned this pull request Mar 26, 2015
@patrickhlauke
Copy link
Member

👍

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

Successfully merging this pull request may close these issues.

4 participants