Skip to content

Conversation

@agriffis
Copy link

Replace a hard-coded #fff with color-yiq for text color.

@andresgalante
Copy link
Collaborator

Hi @agriffis thanks for the contribution. I don't think we need a default value for $color-hover in this case since we are defining for every hover color but it doesn't hurt to remove the hardcoded.

@agriffis
Copy link
Author

Hi @andresgalante, thanks for the reply.

Regarding the default, I think customizers like myself would appreciate having it, otherwise I'll just be calling color-yiq on the way. It seems to make sense for button-outline-variant considering button-variant uses color-yiq for its text color. Do you disagree?

Copy link
Collaborator

@andresgalante andresgalante left a comment

Choose a reason for hiding this comment

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

@agriffis you are right, it's good to have it to extend bootstrap

@XhmikosR XhmikosR requested a review from mdo November 11, 2017 23:07
@mdo
Copy link
Member

mdo commented Jan 17, 2018

Resolved the conflict here and put the $color-hover variable to use in the mixin. Turns out we weren't even using it there; we were overriding that value with an existing color-yiq function.

@mdo mdo mentioned this pull request Jan 17, 2018
@mdo
Copy link
Member

mdo commented Jan 17, 2018

Actually, to track that right, I cherry-picked your commit into #25339.

@mdo mdo closed this Jan 17, 2018
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.

3 participants