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

Fix btn-light's active state color #35293

Closed

Conversation

tpg-gulshan
Copy link

@tpg-gulshan tpg-gulshan commented Oct 28, 2021

Fixes #35258

Fixes #34123

@tpg-gulshan tpg-gulshan requested a review from a team as a code owner October 28, 2021 13:40
@XhmikosR XhmikosR changed the title fix for btn-light's active state color Fix btn-light's active state color Oct 28, 2021
Copy link
Member

@ffoodd ffoodd left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this!
This is not the best way though, since you're making an exception were we try to have a generic behaviour.

I think we need to question the light color itself, since your issue relates more to the color being to close to white (thus lightening it has almost no effect).

@ffoodd
Copy link
Member

ffoodd commented Dec 15, 2021

Back to this, I'd say either we consider darkening our default light color (which would concern everything using it) or we consider using the button-variant() mixin with defined parameters, instead of simply using the color itself.

I'd recommend going for this, since we also have contrast issues with some of our outline buttons, that would benefit from a less generic way of building buttons variants — meaning probably to drop the very simple loop and declare at least some variants indepently. This is used in Boosted at least, even if we don't need that much specificity.

By doing so, we could get closer to what v4 looked like. Not quite sure yet of how the best values to use, but if you want to give it a shot it'd be really appreciated :)

@tpg-gulshan
Copy link
Author

Thanks, @ffoodd for your observations on this.
I'll look into it again and spend some more time fixing it in a different and better way. I will also consider your idea to make it done.

@mdo
Copy link
Member

mdo commented Apr 13, 2022

Is this still something you want to iterate on @tpg-gulshan or should we take a crack at it?

@mdo
Copy link
Member

mdo commented Apr 14, 2022

Thinking about this in another way in #36168.

@mdo mdo closed this in #36168 Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants