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 override default colors (e.g. $blue2) with node-sass or dart-sass in v4 #5297

Open
dlech opened this issue May 11, 2022 · 19 comments

Comments

@dlech
Copy link
Contributor

dlech commented May 11, 2022

After upgrade to v4 I am unable to override default colors (e.g. $blue2), even using node-sass. Has anything changed from v3 which would impact this?

Originally posted by @dylangrandmont in #4759 (comment)

@dlech
Copy link
Contributor Author

dlech commented May 11, 2022

I'm seeing the same issue as well. In my case, it seems related to the dart-sass migration.

@dlech
Copy link
Contributor Author

dlech commented May 11, 2022

I think we need to change to the @use syntax described in https://sass-lang.com/documentation/variables#configuring-modules.

@dlech
Copy link
Contributor Author

dlech commented May 11, 2022

Has anything changed from v3 which would impact this?

The answer to this question is #4032 (comment)

@dlech
Copy link
Contributor Author

dlech commented May 11, 2022

I think we need to change to the @use syntax described in https://sass-lang.com/documentation/variables#configuring-modules.

I was able to use this with partial success:

@use '@blueprintjs/core/src/blueprint' with (
    $pt-font-size: 16px,
    $pt-font-size-large: 18px,
    $pt-font-size-small: 14px,
    $pt-app-background-color: #e8e8e8,
    $pt-intent-primary: #0088ce
);

It successfully changes the font sizes, but not the colors.

@jayarjo
Copy link

jayarjo commented May 27, 2022

Any progress on this? Even potential maybe?

@adidahiya
Copy link
Contributor

@jayarjo progress on what? What is your issue exactly?

@dlech one idea to change the colors... maybe try this:

@use '@blueprintjs/colors/src/colors' with (
    // overrides
);
@use '@blueprintjs/core/src/blueprint' with (
    // overrides
);

or this:

@use '@blueprintjs/colors/lib/scss/colors' with (
    // overrides
);
@use '@blueprintjs/core/src/blueprint' with (
    // overrides
);

@Sascha-Gschwind
Copy link

@adidahiya I have the same problem, that my intent-overrides don't work anymore and I've tried making the @use syntax work for multiple hours now without success. It seems to completely ignore those overrides and start the app with the default colors. I've read all the other issues and I know about the dart-sass breaking change, switched the compiling of the scss files to dart-sass as well with no success.

For color-theming, all you basically need is the ability to override the different intent and text colors, so basically the content of the _content-aliases.scss file. This would probably fit the needs for most people.

We really like blueprint as a foundation but since color overriding is broken for us now in version 4 and probably going forward we will have to switch away from it.

Going through the issues showed that there's definetely a need for themeing. With a couple more intents (like for example bootstrap has) it should in theory be really easy to make themeing something that everybody can use without a huge hassle in terms of maintenance.

Regarding ideas how to make this available, why not give us a wrapper like <Theme /> which we can wrap around our whole application where you can set certain colors and stuff via properties.

@adidahiya adidahiya changed the title After upgrade to v4 I am unable to override default colors (e.g. $blue2), even using node-sass. Has anything changed from v3 which would impact this? Unable to override default colors (e.g. $blue2) with node-sass or dart-sass in v4 Aug 5, 2022
@adidahiya adidahiya added the P1 label Aug 5, 2022
@adidahiya
Copy link
Contributor

@Sascha-Gschwind thanks for the feedback. I definitely want to support theming in some way, and it's unfortunate that this Sass API broke after our migration to dart-sass. I just don't have the bandwidth to tackle this problem in a first-class way right now. I'm open to ideas and implementations from the community on how to make this work in Blueprint v4. For the time being, I have my hands full with all the Popover2-related and v5 migrations.

@lifeiscontent
Copy link

@adidahiya any thoughts on maybe using css variables instead of sass variables?

@adidahiya
Copy link
Contributor

any thoughts on maybe using css variables instead of sass variables?

That would be a pretty large change to our Sass code and tooling. I'm generally open to exploring various technical options to accomplish an API goal like theming in Blueprint, but I'd need to see a proposal about how this would work, what code would need to change in Blueprint, how much of a breaking change it is, how we would migrate Blueprint user code, etc.

@adidahiya
Copy link
Contributor

Also, replying to my earlier comment #5297 (comment) - we are generally moving away from @import statements in Sass and planning to migrate to @use. Sass is deprecating @import and dropping support for it in a future version. So, customizing Blueprint variables will only happen with @use.

@dlech friendly ping again -- have you been able to try @use to customize Blueprint variables? Are you still relying on theming support like this?

Here are some docs on the differences between @import and @use.

@justinbhopper
Copy link
Contributor

While css variables would be easier on end-users to override, I imagine it would be challenging to migrate to given the amount of sass math/operations that are happening with sass variables currently in blueprintjs. You cannot call sass methods using css variables, so the team would have to decide what variables are safe to expose at the surface level versus what variables remain internal (and thus usable in sass calculation operations).

@adidahiya
Copy link
Contributor

@justinbhopper yeah, that's what I feared with a proposal to use CSS variables. If there's no good interoperability story to use those variables in Sass code, it's kind of a non-starter for us.

@dlech
Copy link
Contributor Author

dlech commented Oct 6, 2022

have you been able to try @use to customize Blueprint variables? Are you still relying on theming support like this?

I haven't looked into this any farther. We are just overriding styles for individual components now.

@pbower
Copy link

pbower commented Jul 27, 2023

Hi, wondering if there is an update on this issue? Is there a workaround in V5 to apply colour overrides and have it flow through ?
Thanks heaps

@Samrose-Ahmed
Copy link

Samrose-Ahmed commented Aug 19, 2023

I get the following error attempting to override the colors using scss:

Module build failed (from ./node_modules/react-scripts/node_modules/sass-loader/dist/cjs.js):
SassError: (path: (fill: #5f6b7c)) isn't a valid CSS value.
   ╷
39 │       background: svg-icon("16px/chevron-right.svg", (path: (fill: $pt-icon-color)));

@pbower
Copy link

pbower commented Aug 19, 2023 via email

@justinbhopper
Copy link
Contributor

justinbhopper commented Aug 19, 2023

I get the following error attempting to override the colors using scss:

Module build failed (from ./node_modules/react-scripts/node_modules/sass-loader/dist/cjs.js):
SassError: (path: (fill: #5f6b7c)) isn't a valid CSS value.
   ╷
39 │       background: svg-icon("16px/chevron-right.svg", (path: (fill: $pt-icon-color)));

In order to compile Blueprint's scss, you must copy blueprint's resource/icons folder to a path local to your repo, and setup your bundler with the svg-icon function. There are instructions here if you use webpack: https://blueprintjs.com/docs/#core/classes.namespacing

If you use Vite as your bundler, here is an example:

export default defineConfig(config => {
  return {
    ...
    css: {
      preprocessorOptions: {
        scss: {
          functions: {
            "svg-icon($path, $selectors: null)": inliner(
              join(__dirname, "path/to/resources/icons"),
              { optimize: true, encodingFormat: "uri" }
            )
          }
        }
      },
    },
  };
});

@pinqiW

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants