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

Unable to use overriden $border-color variable in bootstrap overriden application styles #37225

Closed
3 tasks done
gauravshah27 opened this issue Sep 28, 2022 · 9 comments · Fixed by #37239
Closed
3 tasks done
Labels

Comments

@gauravshah27
Copy link

gauravshah27 commented Sep 28, 2022

Prerequisites

Describe the issue

I have a custom light and dark implementation in my application and have $border-color defined within 2 separate set of variables in light and dark. Here is the general setup of what I have in my app.scss file in the application:

html.light {
  // import my bootstrap light variable overrides
  // import bootstrap.scss
  //  import my bootstrap class overrides
}

html.dark {
  // import my bootstrap dark variable overrides
  // import bootstrap.scss
  //  import my bootstrap class overrides
}

This setup was working fine with bootstrap 5.1. However this setup does not work for $border-color in 5.2.
I did some digging and found that the $border-color which is a global scope variable for bootstrap is getting redefined in .table class. Here is the link to the line in question:
https://github.com/twbs/bootstrap/blob/v5.2.1/scss/mixins/_table-variants.scss#L8

This did not exist in the 5.1.3 version of bootstrap:
https://github.com/twbs/bootstrap/blob/v5.1.3/scss/mixins/_table-variants.scss

Because of that redefinition, any overrides that follow the bootstrap import in the setup above are only using the redefined value for $border-color rather than using the overriden value which was the case earlier.

I also confirmed that by changing the variable name from $border-color to something like $foobar and then using that in the subsequent styles for .table class in that file, the problem is resolved.

Reduced test cases

Define the --bs-border-color CSS variable at html.light and html.dark scope level before importing bootstrap.scss which defines it on the :root pseudo and then updating all my class override based styles to use var(--bs-border-color) instead of $border-color. However this problem can be resolved by simply updating the locally scoped variable that is used within .table class from $border-color to something else.

Here is my setup that solves this issue:

html.light {
  // import my bootstrap light variable overrides
  // import mixin that defines --bs-border-color using the overriden $border-color variable
  // import bootstrap.scss
  //  import my bootstrap class overrides
}

html.dark {
  // import my bootstrap dark variable overrides
  // import mixin that defines --bs-border-color using the overriden $border-color variable
  // import bootstrap.scss
  //  import my bootstrap class overrides
}

What operating system(s) are you seeing the problem on?

Windows

What browser(s) are you seeing the problem on?

Chrome, Firefox

What version of Bootstrap are you using?

5.2.1

@gauravshah27
Copy link
Author

@julien-deramond it would be great if you can tell us a tentative timeline on when 5.2.2 will be released with a fix?

@julien-deramond
Copy link
Member

julien-deramond commented Sep 29, 2022

We are all working on our spare time so unfortunately, it is very difficult for us to announce any timelines for the releases. The best indicator is in our GitHub Projects (direct link to v5.2.2). We try to evaluate in there what could be in the next releases and we define the release date when we're close to the end.
For the v5.2.2, I'd in a few weeks but you'll understand I can't guarantee anything :)

For this issue specifically, I've put it in the v5.2.2 project in case of it would need to be fixed as a mandatory requirement. But I haven't had the time to look at it yet.

@gauravshah27
Copy link
Author

@julien-deramond really appreciate your swift response. I do appreciate all the work that you guys put in to make this one of the best styling libraries out there and I understand that you can't say with certainty when the release would be cut.

I was trying to look at the release history in the past to see if there was any cadence that is being followed that would give me a good estimate on when the next release could be and it looks like for every minor release line the patches are cut roughly every 1-2 months. This last release was cut 22 days ago and so I was trying to understand if it is close to a week or is it another month before the release is cut.

I have a custom styling library that is built on top of bootstrap and so was trying to decide whether I should be publishing a release on my end based on 5.2.1 or should cut a release once 5.2.2 is out (which hopefully has the fix for this issue).

Thanks a lot for the acknowledgement and will wait for 5.2.2 to be cut (hopefully soon 😃)

@louismaximepiton
Copy link
Member

Hi there,

I think that this renaming should be done, at least for consistency between mixins, but I can't see a reproducible reduced test case here. I tried this inside the bootstrap.scss file:

$border-color: #f00; // stylelint-disable-line scss/dollar-variable-default

// Bootstrap imports
...
//

.table-success {
  border-color: $border-color;
}

and this didn't led me to any unexpected behavior:

image

Could you please light me up on what I'm missing here?

@gauravshah27
Copy link
Author

Hi there,

I think that this renaming should be done, at least for consistency between mixins, but I can't see a reproducible reduced test case here. I tried this inside the bootstrap.scss file:

$border-color: #f00; // stylelint-disable-line scss/dollar-variable-default

// Bootstrap imports
...
//

.table-success {
  border-color: $border-color;
}

and this didn't led me to any unexpected behavior:

image

Could you please light me up on what I'm missing here?

Here is a sample flow that I have which will reproduce the issue:

$border-color: #f00   // this is my override to bootstrap's $border-color default variable. This will allow me to use f00 for my 
                                    // borders throughout the app

...
...

@import '~bootstrap/scss/bootstrap.scss' // this will import the table variant mixin that redefines border-color 
                                                                // (https://github.com/twbs/bootstrap/blob/v5.2.1/scss/mixins/_table-variants.scss#L8)


.side-menu {
  border-color: $border-color // this will now use the redefined $border-color value in class instead of the #f00 that I want it to 
 // use because that was a global variable that was overriden in .table scope rather than just creating a differently named local // variable and then using it in the mixin.
}

@louismaximepiton
Copy link
Member

Here's what I get using Vite 😕

@gauravshah27
Copy link
Author

@louismaximepiton I have forked your stackblitz here and updated it to reproduce the issue that exactly matches what I have in my application and I see the same thing that I am seeing in my application where the $border-color gets redefined by the table-variant mixin and that is what is used in subsequent classes that follow the import of bootstrap.

Inspect the border on the .side-menu and you will see that it is using #373b3e as the color for the border instead of using the #313131 which is defined as $border-color in html.dark

@julien-deramond
Copy link
Member

@gauravshah27 Could you confirm that #37239 can fix your issue and that it corresponds well to the analysis you've done (thanks a lot for that btw).

@XhmikosR XhmikosR changed the title Unable to use overriden $border-color variable in bootstrap overriden application styles. Unable to use overriden $border-color variable in bootstrap overriden application styles Oct 2, 2022
@mdo mdo closed this as completed in #37239 Oct 2, 2022
@gauravshah27
Copy link
Author

@julien-deramond Thanks a lot for putting the fix out and I tested that out and it is working as expected. Really appreciate the quick turnaround on this as this will unblock us from getting on the latest version of bootstrap. Really appreciate all the good work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants