Skip to content

Conversation

@jacbn
Copy link
Contributor

@jacbn jacbn commented Oct 21, 2025

Previously, we were importing parts of Bootstrap multiple times. In short, this aims to fix that.

https://getbootstrap.com/docs/5.3/customize/sass/#importing details the basic structure, but is significantly less useful than looking at the .scss files Bootstrap comes with.

The general idea is that Bootstrap can be imported in one of two ways; option 1 is to just import the entirety of Bootstrap as is, which you can do via @import "~bootstrap/scss/bootstrap";. This would be fine, but there is a problem with customisation.

We have many places where we want to customise Bootstrap, e.g. by adjusting defaults, extending maps further than the default, etc. To allow you to do this, all the Bootstrap variables are defined with !default, meaning if a definition for that variable already exists, that definition will always override the default. Therefore, by defining e.g. $border-color: {val} before importing Bootstrap, any BS variables that inherit from $border-color will use that value instead.

However. Bootstrap also defines its own functions, and there are times we need to use these functions inside the override. Also, we may e.g. want to extend a Bootstrap map, for which a self-referential declaration like

$theme-colors: map-merge($theme-colors, $theme-colour-overrides);

which can make use of Bootstrap's default is much simpler than having to rewrite it.

In both of these situations, we need to import these bits from Bootstrap first, then apply our overrides, and would then need to import Bootstrap again to make use of them. This is what we were doing before (badly) and is why we have duplicate imports.

To fix this, option 2 for importing Bootstrap is piecewise. There are 5 "required" Bootstrap imports:

@import "~bootstrap/scss/functions"; // map funcs, colour funcs, etc
@import "~bootstrap/scss/variables"; // most of the !default values
@import "~bootstrap/scss/maps"; // defines collections of themed variables
@import "~bootstrap/scss/mixins"; // all kinds of helpful @mixins
@import "~bootstrap/scss/utilities"; // constructs all the BS helper classes (fw-bold, p-md-5, etc) using mixins and variables

There are also a bunch of other optional imports, though since we were previously importing the entirety of Bootstrap, we have made use of most of these optionals somewhere in the code.

For interest, importing variables twice is the cause of the duplicated :root colour definitions; importing utilities twice is the cause of duplicated helper classes, etc etc.

We can add overrides in between these imports; they should be after variables and before utilities, since most of our customisation is changing the raw values of these BS defaults, which we want utilities to process. This idea defines the new structure of the isaac.scss files.

To import the rest of Bootstrap, rather than @import "~bootstrap/scss/bootstrap"; as before (which duplicates the above imports), we need to import all the optional libraries individually. Unfortunately, there is no easy way to do this; I was hoping BS might come with an import for all the optionals, since there are various other files they offer you can import that cover common cases, but no luck. As such, I have created scss/common/bootstrap-contents.scss, which is a dump of the "~bootstrap/scss/bootstrap" file without the 5 imports above. This is imported after the utilities import.

I have restructured most of the Bootstrap-relevant imports of both isaac.scss files to be more logical and to group related bits together.

@codecov
Copy link

codecov bot commented Oct 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.65%. Comparing base (5ff7bd0) to head (8a8c63b).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #1792    +/-   ##
========================================
  Coverage   41.65%   41.65%            
========================================
  Files         541      541            
  Lines       23685    23685            
  Branches     7835     6983   -852     
========================================
  Hits         9867     9867            
- Misses      13178    13777   +599     
+ Partials      640       41   -599     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jacbn jacbn marked this pull request as ready for review October 21, 2025 15:31
Copy link
Contributor

@sjd210 sjd210 left a comment

Choose a reason for hiding this comment

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

Finally, thank you!! This has been very annoying, and this seems to be a very neat way to fix it. I'm not too concerned about needing bootstrap-contents - just something that we'll need to consider if/when new optional libraries are added.

@sjd210 sjd210 merged commit 082ace0 into main Oct 23, 2025
10 checks passed
@sjd210 sjd210 deleted the improvement/bootstrap-import-order branch October 23, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants