Skip to content

Conversation

@cderv
Copy link
Collaborator

@cderv cderv commented Jan 9, 2025

fix #11561 and fix #11816

As discussed, the problem here seems to the wrong color for mixing.

When $body-color and $body-bg are not defined, we should use the same values as Bootstrap default variables, which are the ones used for default theme (i.e theme: default)

However, we need to try get back a nice default. Previously $border-color used was bootstrap default


image

One way to fix is the following:

So

$border-color: mix(
if(variable-exists(body-color), $body-color, $gray-900),
if(variable-exists(body-bg), $body-bg, $white),
10%
) !default;

This way we get
image

Which is quite close.

If we want to keep default theme the same, we could just mix() when variable exists.

@if variable-exists(body-color) and variable-exists(body-bg) {
  $border-color: mix($body-color, $body-bg, 10%) !default;
  $table-border-color: $border-color !default;
}

But I don't think we want to make use of those @if because our SCSS -> CSS exposer logic.

This PR aims to fix the default theme. Then we need to fix

Right now this is some examples after this PR

  • Vapor
    image
  • Darkly
    image
  • Cyborg
    image
  • litera
    image
  • lux
    image

So I think we get good mix with this.

cderv added 2 commits January 9, 2025 17:36
Our default theme use default bootstrap variable and is not a real theme like other bootswatch themes. So be sure to use the default value when we do use variables before defaults bootstrap layer applies
Add comment about using own bootstrap defaults
@cscheid
Copy link
Collaborator

cscheid commented Jan 9, 2025

I think we should backport this one to 1.6.

@cderv
Copy link
Collaborator Author

cderv commented Jan 9, 2025

Yes agreed. It seems you are ok with the color mix. I'll backport tomorrow, and think of something regarding testing. I think I can test with playwright at least the default theme.

cderv added a commit that referenced this pull request Jan 10, 2025
Our default theme use default bootstrap variable and is not a real theme like other bootswatch themes. So be sure to use the default value when we do use variables before defaults bootstrap layer applies

This way we are explicit in the color we are mixing.

Related to #11828
cderv added a commit that referenced this pull request Jan 10, 2025
Our default theme use default bootstrap variable and is not a real theme like other bootswatch themes. So be sure to use the default value when we do use variables before defaults bootstrap layer applies

This way we are explicit in the color we are mixing.

Related to #11828
@cderv cderv merged commit fc6cd8d into main Jan 10, 2025
47 checks passed
@cderv cderv deleted the fix/border-color branch January 10, 2025 17:04
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