Manually set hover and active backgrounds and borders for dark and light buttons

[mdo]

This isn't elegant at all, and is actually rather annoying for others, but it fixes a problem, so here it goes.

This PR aims to improve the color contrast on hover and active state for light and dark buttons. There's no other way I can find to do this without it being a complete rewrite of the component at this point. Placing two custom conditions in here though—based on the extremes of our color palette—seems okay though.

I'd rather go this route than #35293 because this keeps the customization in the usage of the mixin as opposed to the logic of the mixin. Ideally we could setup custom color functions like #34013, but I can only see us doing that with a rewrite of the mixin as well (which could include an optional parameter or something for specifying tint or shade.

Lastly, I don't see this needing to affect outline buttons. Those have separate issues, yes, but for the moment they have a clear enough hover state at least (though not a clear enough active one).

Definitely need some more thoughts though from @twbs/css-review :).

Closes #35293.
Fixes #34123.

[mdo]

@julien-deramond This was the best solution I could come up with for these button contrast issues. Not sure if you have any other ideas for what we could try :).

[julien-deramond]

@julien-deramond This was the best solution I could come up with for these button contrast issues. Not sure if you have any other ideas for what we could try :).

We've got a similar approach in Boosted without a loop but where we call button-variant with different parameters following the cases in order to apply the right contrasted colors.

For example in this file:

.btn-light,
.btn-secondary {
  @include button-variant($white, $black, $disabled-background: $white, $disabled-border: $gray-500, $disabled-color: $gray-500);
  &.btn-inverse {
    @include button-variant($black, $white, $white, $white, $white, $black, $primary, $primary, $black, transparent, $gray-700, $gray-700, $black);
  }
}

Our palette is reduced, strict and limited in terms of colors so we don't need shade-color nor tint-color but that's specific to Boosted. This system was introduced by @ffoodd (and has just slightly changed in order to handle buttons with dark backgrounds defined by .btn-inverse and some params) so maybe he will have more details to share on what's been experimented at the time (2 years ago apparently so maybe it'll be complicated to remember 😄 )

I do believe there's room from improvement in Boosted but that seems to work pretty well at the moment.

[ffoodd]

In fact in Boosted, it didn't relate to contrasts but to how inconsistent are Orange's buttons styles 😅

I recall modifying default argument values in button-variant() to reduce the need for explicit code, but couldn't do anything else than using specific mixin call for each button style.

The way Mark handles this looks better since it duplicates less code (and might allow some simplifications in Boosted, at some point).

[fitcwill]

Note added to #34123 with an observation on a change to rounded corners when picking up on this change and applying the update to take an advantage of it.

[fitcwill]

@mdo - saw your note saying the design update was intentional and was blogged about. See mention of the button styles changing but not the card styles to match (or decision not to match). I did see the border colour appears to have adjusted. Should I raise as a bug to get the right eyes on this and allow conversation (even if I'm only to be shut down... might be somewhere for others to see that it's been discussed and confirmed as intentional). Perhaps this is all just an optical illusion for me! 😂

[mdo]

@fitcwill You're commenting on unrelated PRs, please try to open a new issue or find the relevant PR, which is #35979 :). The global border-radius values were updated, which both the cards and buttons use for the current and old values. I don't see the need to change anything further given this.

[fitcwill]

Ah, thanks for the tips, cheers. I see the commit you're referring to now and it's helped highlight the problem. Will add a comment to the other PR hoping that's a better place. Might not be worth taking further forward than that though.

[jufemaiz]

Just noting that similar changes impacting the active / hover states the default values for info and warning.

theme color	v4 bg	v5 bg	v4 bg hover	v5 bg hover	v4 contrast	v5 contrast
warning	#ffc107	#ffc107	#e0a800	#ffcd39	1.31	1.09
info	#17a2b8	#17a2b8	#138496	#3dd5f3	1.44	1.74

As can be seen, if the button colour is light enough, then tint is used, compared with the previous approach which darkened the colour.