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

Fix Colorname Selectors SASS Warning ("You probably don't mean to use the color value black in interpolation here.") #12333

Closed
thomasfrobieter opened this issue Nov 15, 2021 · 8 comments

Comments

@thomasfrobieter
Copy link

thomasfrobieter commented Nov 15, 2021

Compiling foundation-sites with a current version of SASS (1.43.4), throws various of those issues:

Warning: You probably don't mean to use the color value white in interpolation here.
It may end up represented as white, which will likely produce invalid CSS.
Always quote color names when using them as strings or map keys (for example, "white").
If you really want to use the color value here, use '"" + $name'.

377 │ &.#{$name} {
│ ^^^^^

node_modules/foundation-sites/scss/components/_button.scss 377:15 foundation-button()

Needs to be fixed by writing the selector like so:

&#{"." + $name} {

or

&.#{"" + $name} {

.. but the first one doesn't look that ..hacky.

@joeworkman
Copy link
Member

Would you like to submit a PR?

@joeworkman
Copy link
Member

I tried to generate these same warnings but could not. What are you running to see these?

@thomasfrobieter
Copy link
Author

thomasfrobieter commented Nov 26, 2021

I got these by running our custom gulp sass build task (stripped it down to the relevant part):

function buildSass() {
  return src(config.sassRegex, {
      since: lastRun(buildSass)
    })
    //find files that depend on the files that have changed
    .pipe(sassInheritance({
      dir: 'scss/'
    }))
    .pipe(plumber()) // Hint: Plumber always need to be piped first!
    .pipe(sass.sync().on('error', sass.logError))
    .pipe(rename({
      suffix: '.min'
    }))
    .pipe(dest('./' + config.folderCss));
};

Relevant Versions:

  • sass: 1.43.5
  • node: 14.17.6

Similar issues:
ConciseCSS/concise.css#52
https://stackoverflow.com/questions/57455245/how-to-i-solve-this-sass-interpolation-issue

@joeworkman
Copy link
Member

Thanks. I will work on replicating this. Any PRs would be helpful if you have the time.

@glen-84
Copy link
Contributor

glen-84 commented Dec 15, 2021

I think a better fix is to just quote your map keys, as suggested in the Sass documentation:

Most of the time, it’s a good idea to use quoted strings rather than unquoted strings for map keys. This is because some values, such as color names, may look like unquoted strings but actually be other types. To avoid confusing problems down the line, just use quotes!

So change something like:

$foundation-palette: (
    // ...
    black: #000
);

To:

$foundation-palette: (
    // ...
    "black": #000
);

Perhaps the Foundation Sass should do this as well, just to set an example.

@thomasfrobieter
Copy link
Author

@glen-84 great simple fix, I've tested this right now, works perfectly fine!

So we just need to fix these files:

thomasfrobieter pushed a commit to thomasfrobieter/foundation-sites that referenced this issue Jan 19, 2022
@thomasfrobieter
Copy link
Author

thomasfrobieter commented Feb 9, 2022

Investigated an issue with the button-style() mixin when using this fix:

@include button-style("secondary");

results in:

.selector{
  background-color: secondary;
  color: #fff;
}
.selector:hover{
  background-color: #ff7915;
  color: #fff;
}

Map:

$button-palette: (
  "primary": #0080ff,
  "secondary": $primary-color
}

While this works well:

@include button-style($primary-color);

joeworkman added a commit that referenced this issue Jul 11, 2022
…lation-warning-12333

FIX SASS color interpolation warning #12333
@ameotoko
Copy link

ameotoko commented Jul 1, 2023

It seems that _settings.scss should not be edited directly, because it is (re)generated by a gulp task.

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

No branches or pull requests

4 participants