[SCSS 6.2] possible bug, default $foundation-palette colors overriding old variables even if @include add-foundation-colors; is not included in _settings.scss

[erutan]

I was wondering why something calling $alert-color had suddenly turned salmon color (I hadn't added in $foundation-palette + @include add-foundation-colors) yet in my 6.2 migration.

An easy override, but I'd rather fix the issue, so I copied the $foundation-palette map from the default _settings.scss and edited the values inside. The element was colored as originally intended - however commenting / uncommenting @include add-foundation-colors had no (immediately noticeable) difference.

If I understand correctly, $foundation-palette is supposed to be optional and we can continue to use traditional variables, but at the moment (I assume) it's !default values are overriding the old traditional variables. It'd also be nice to have some explanation of the implications of using the SASS map (I assume for autogenerating classes? hence why we'd want some things inside it and some more tertiary colors outside of it) - maybe that's up somewhere and I just haven't stumbled across it.

[kball]

@JeremyEnglert can you take a look at this?

[kball]

Found this while triaging outstanding v6.3. @andycochran do you have time to take a look?

[andycochran]

@kball Q: Is $foundation-palette still optional? Seems like that doesn't have to be the case since v6.3 is a minor version bump. Can we deprecate the old way of doing color variables now?

[brettsmason]

I'd agree with removing the old method too 👍

[andycochran]

Let's not let the old method hang around until v6.4. 😬
It's now or then.

[kball]

Good point. There's actually a few things that have warnings that they'll be removed in 6.3 as well that we need to get through. Lets include this along with those and any fixes for bugs we find into a second RC mid next week.

[kball]

so question - does getting rid of the old method mean getting rid of the helper variables $primary-color, etc? Or getting rid of them from internal use but leaving the option to add them with @include add-foundation-colors; for your own use? Or what were you thinking?

[andycochran]

Good question, @kball. The more I think about it, the more complicated it gets.

It looks like we're inconsistent in the way we're using color variables, e.g.:

• In _error.scss we use $input-background-invalid: map-get($foundation-palette, alert) !default;
• But in _meter.scss we use $meter-fill-good: $success-color !default;

We should be consistent in our default settings. It'd be nice if $foundation-palette didn't have too many requirements — e.g. $primary-color was the only required key. But that's only possible if you don't include components that rely on multiple colors (e.g. the Forms component has $meter-fill-[good,medium,bad] which requires success, warning, and alert).

So $foundation-palette has different requirements depending on which components you include. Which makes it difficult to, say, include the Forms component but not generate Buttons, Labels, and Badges for success, warning, and alert.

I'd be okay with getting rid of the internal variables used for colors (_global.scss line 105) and just using map-get($foundation-palette, [key]) throughout settings. But we'd need to clearly note the repercussions of altering $foundation-palette. And I'm sure there are a ton of apps making use of the helper variables. I know I use them a lot in my custom modules.

Having written this up, now I think it'd be best if we make $foundation-palette required, but keep the helper variables.

Not sure what this means for the old method. I think it just means that this issue is no longer a bug as of v6.3, and we should clean up the inconsistencies in our default settings?

[brettsmason]

@andycochran could we use the get-color function everywhere to make it a bit easier?

[andycochran]

What if, instead of using every color in $foundation-palette, components that looped through the palette had a list to determine which colors from the palette to use… 

$button-classes: ( secondary success warning alert ) !default;

[kball]

@andycochran I like that idea... similar to how we use $breakpoint-classes yeah?

[andycochran]

@kball Exactly how we use $breakpoint-classes. 😉

[andycochran]

I think we need to clarify what we mean by "requirements."

The Global Styles > Colors docs say:

If you're using the Sass version of Foundation, it's possible to edit the default color palette, by changing the $foundation-palette variable in your settings file. The only required colors are ones named "primary" and "alert". The names used in the palette will be output as CSS classes.

This isn't really accurate. If a color being "required" just means it's used by components, then Primary and Alert aren't any more required than Success or Warning. The only color that isn't used by component settings is Secondary. So is Secondary the only color that isn't required?

Although messy and repetitive, it would be totally valid to delete all the keys in $foundation-palette and set all the component colors explicitly in each component's settings.

So, I'm not sure $foundation-palette actually is required. It's just a shortcut for setting multiple variables at once. What we need to say in the docs is something like:

If you're using the Sass version of Foundation, it's possible to change the color palette, by editing the $foundation-palette map in your settings file. Its keys are used by various components' settings for style and to output alternate class names.

[Warning Callout]: If you remove a default key from $foundation-palette, be sure to edit any component settings that reference that color.

[andycochran]

I just took a pass at addressing some of the stuff brought up here. #9459

I don't think we can simply get rid of the helper variables for color classes. They're very useful. And the idea of get-color(key) everywhere is an ugly breaking change.

I wonder if instead of hardcoding helper classes for each $foundation-palette color in scss/_global.scss, we could refactor the @include add-foundation-colors; line in settings to serve the same purpose.

[andycochran]

I'm going to close this. Through #9459, the docs have been updated to clearly explain how colors work in v6.3.