[WIP] Separate Foundation Palette #9683

natewiebe13 · 2017-01-20T17:45:52Z

Goal: To separate the Foundation palette into having a map that lists possible colors, and a map that assigns a color to a state.
TODO: Update documentation. (This should be done after functionality is finalized)

Fixes: #9671

…sign the colors to states

IamManchanda · 2017-01-20T17:51:00Z

I just pulled it .... i am sorry but docs is not working bro .... did you tested your code before pushing ?

ncoden · 2017-01-20T18:16:03Z

scss/_global.scss

-  success: #3adb76,
-  warning: #ffae00,
-  alert: #cc4b37,
+$foundation-colors: (


I don't know if _global.scss is the best place for this. We should rather put it in util/_color.scss.

Edit: Should it be the same for $foundation-states ?

Also, it makes me think we should separate the utils of Foundation (private variables/mixins/functions) from the utils for the user (real Sass utilities).

I agree with moving the states as well.

Actually, if we were to move this, the variables would need to be declared in the middle of the file. _global.scss may be the best place for now.

the variables would need to be declared in the middle of the file

Why ?

The state color assignments are dependent on get-color which is declared further down the file.

ncoden · 2017-01-20T18:18:37Z

scss/_global.scss

@@ -20,35 +20,50 @@ $global-width: rem-calc(1200) !default;
 /// @type Number
 $global-lineheight: 1.5 !default;

-/// Colors used for buttons, callouts, links, etc. There must always be a color called `primary`.
+/// Colors available by using `get-color()`.


Please precise that unline the previous get-color, it these colors should not be used inside components.

@ncoden, can you elaborate on this?

I mean: Please precise that unlike the previous get-color, these colors (red, blue) should not be used inside components, because component should be customizable and rely on any color (so on semantic colors).

ncoden · 2017-01-20T18:19:39Z

scss/_global.scss

-@if not map-has-key($foundation-palette, primary) {
-  @error 'In $foundation-palette, you must have a color named "primary".';
+@if not map-has-key($foundation-states, primary) {
+  @error 'In $foundation-states, you must have a color named "primary".';


In $foundation-states, you must have a state color named "primary".

ncoden · 2017-01-20T18:22:49Z

scss/settings/_settings.scss

-  success: #3adb76,
-  warning: #ffae00,
-  alert: #cc4b37,
+$foundation-colors: (


scss/settings/_settings.scss is automatically generated. You do not have to update it.

ncoden · 2017-01-20T18:25:28Z

scss/util/_color.scss

+/// @param {key} state key from foundation-states
+///
+/// @returns {Color} color from foundation-states
+@function get-state($key) {


You can use @function map-safe-get() now.

natewiebe13 · 2017-01-20T20:04:42Z

@ncoden, the builds are failing when calling red($color) expecting $color to be a color. If I debug $white during testing, it echos get-color('white') instead of #fefefe. Do you have any ideas why this is happening?

ncoden · 2017-01-20T20:11:03Z

@natewiebe13 In Sass, when a called function is not found, it is considered as text. This is the way Sass prevent to execute CSS functions like rgb()...

I agree, that's dirty.

So it means you called get-color('white') without declaring it.
-> Line 51 in _global.scss.
-> Imported in foundation.scss after util/_util.scss.

Did you built Foundation with gulp ?

ncoden · 2017-01-20T20:19:06Z

scss/util/_color.scss

@@ -4,7 +4,7 @@

 @import 'math';

-/// Colors available by using `get-color()`.
+/// Colors available by using `get-color()`. These should not be used directly within components, variables should be used instead to make components customizable.


get-state() and settings should be used instead to make components customizable.

ncoden · 2017-01-20T20:27:54Z

Could you update docs and add unit tests ?

natewiebe13 · 2017-01-20T20:32:16Z

@ncoden, what parts would you like unit tests for? (What should be tested)

ncoden · 2017-01-20T20:34:07Z

At least for every function you added/modified. So get-color and get-state. Look at tests for map-safe-get in test/util/_value.scss

ncoden · 2017-01-20T20:45:27Z

test/sass/_color.scss

+
+  @include test('Get Color (Blue) [function]') {
+    $test: get-color('blue');
+    $expect: #1779ba;


We do not expect always #1779ba, but only the color for blue from the color map. So please set $foundation-color: (blue: #1779ba);. Same for get-state.

ncoden · 2017-01-20T20:54:16Z

test/sass/_color.scss

    $expect: #1779ba;
+    $foundation-colors: ('blue': $expect);
+    $foundation-states: (primary: get-color('blue'));


Do not rely on other functions within tests. You can simply use $expect instead of get-color('blue') here.

Also, I think you must set !global. Did you run the tests ?

IamManchanda · 2017-01-21T11:03:22Z

Tagging @DaSchTour for his views, reviews and #9646 ......

DaSchTour · 2017-01-23T19:49:50Z

Well. Now I would just add one additional feature, that allows activating states per component without redeclaration of color and it would be nearly exact what I proposed.
Than I could add state colors to inputs and do what would be needed for #9646.

DaSchTour · 2017-01-23T19:54:35Z

scss/util/_color.scss

+/// @param {key} state key from foundation-states
+///
+/// @returns {Color} color from foundation-states
+@function get-state($key) {


I would suggest to make this internal -zf-get-state and wrap generation of states for components. Otherwise I'm sure this will cause a lot of confusion why get-color(green) is equal to get-state(primary) and why both returns a color. Or it should be get-state-color to be sure, that nobody expects something else than a color.

DaSchTour · 2017-01-23T19:56:57Z

scss/_global.scss

-  success: #3adb76,
-  warning: #ffae00,
-  alert: #cc4b37,
+$foundation-states: (


I would suggest that we don't have to use get-color here but to enforce that colors used here need to be declared in $foundation-colorsthis way it is possible to introduce colors "from the side" which may cause confusion, because generated elements suddenly get colors not globally defined.

Can you elaborate here? Do you mean instead of using the get-color function, we would specify the key that would be used for the get-color() function, then return the value using get-color() while calling get-state()?

IamManchanda · 2017-01-24T08:48:12Z

@DaSchTour ...... and dear sir what about Rainbow ( #9608 ) 😈 ?

DaSchTour · 2017-04-21T07:33:20Z

@natewiebe13 well, while talking about it and taking a deeper look on this I get that this is getting in the right direction. Still I would suggest a last change, that we reduce to have only get-color() which will first deliver from $foundation-colors and than from $foundation-states. For internal usage for example while getting colors for states we might avoid circular reference by adding an additional parameter to get-color() to just get colors from color map.

IamManchanda · 2017-04-24T15:50:34Z

Hii Guys,
I have to say i am loving it where its going but i have a suggestion

While creating Building Blocks if there is anything i have noticed is that there is a social button/icon for every second ( if not every single ) component.
Moreover if you look in a broader scope and not just building blocks, every single site that is serious about there site have a social buttons/icons/etc and why not ... best way to get popular is social engagement!

Currently within the building blocks same variables are getting used again and again which is a bloat ...so i would suggest defining these variables within the framework .... See Gist
Yup the way i coded in the gist is not the right way and should be done like this PR .... but you get my idea??

Yup you might consider my suggestion as a bloat .... but believe me.... what's happening currently is a much bigger bloat!

What's your thoughts @kball @natewiebe13 @DaSchTour ??
Also cc / @rafibomb

kball · 2017-04-25T21:33:06Z

I think @IamManchanda raises a potentially interesting question...

The intent of this PR if I'm understanding correctly is to create a distinct abstraction for the color palette and the choice of which states map to which colors. Does it make sense to take that further, and allow the use of multiple palettes, allowing you to e.g. import palettes, or have different palettes for different portions of a site that are logically separated within the scss?

DaSchTour · 2017-04-26T05:40:44Z

@kball I think this could only be achieved in an manageable way if using a map of maps. I think in this case we absolutely should have only one function to retrieve colors. On the other hand maybe there should be only one map with colors, although it might have a lot of them and than a mapping for buttons etc.
I describe a lot of different models in the original issue #9671

IamManchanda · 2017-04-26T09:29:16Z

@DaSchTour => Are you relating with your this comment
=> #9671 (comment) ??

DaSchTour · 2017-04-26T13:03:56Z

@IamManchanda yes and also the following comments. I made several suggestions.

IamManchanda · 2017-04-26T19:30:32Z

Hmmn....
But here are palletes i am particulary interested

Foundation Colors ( Can be replaced but can't be disabled )
Social Media Color Pallete ( Enabled by default )
=> https://designpieces.com/2012/12/social-media-colours-hex-and-rgb/
Flat Design Color Pallete ( Enabled by default )
=> https://flatuicolors.com/
PS: We can mix 1 and 3 also!
Material Design Color Pallete ( Disabled by default )
=> https://www.materialpalette.com/colors
Metro UI Color Pallete ( Disabled by default )
=> https://colorlib.com/etc/metro-colors/

kball · 2017-04-27T23:10:36Z

@joeworkman I think this may lead to a more general form of what you're proposing in emails for foundation/foundation-emails#754

Would be interested in making sure this functionality works for the usecase of having 2 alternate palettes. Joe do you have thoughts on this approach and if something like this would achieve what you're looking for in emails?

(cc @tdhartwick @rafibomb)

joeworkman · 2017-04-28T15:14:14Z

@kball In my Foundation for Sites product, I actually implemented this alt color scheme as well. This is where I got the idea for Email. Its been really popular with my community of designers.

When I update to use F6 later this year, this was on my list of things to submit a pull request for.

IamManchanda · 2017-05-08T11:02:34Z

@DaSchTour Assigning you together with @natewiebe13

As discussed, daniel can you modify the code within this PR ??
I have gone through your comments once again and i think you are going in very right direction specially considering the external pallete!

$button-colors: (primary: blue, secondary: gray, success: green, warning: yellow, alert: red );
$callout-colors: (success: green, warning: yellow, alert: red);
$label-colors: (primary: blue, secondary: medium-gray, success: green, warning: yellow, alert: red);

but what is more required is external color pallete's (enabled/disabled) ....
For shades, we can use sass for scale, darken and lighten .... only one color per color is required!
Social button is one serious need, other flat color like indigo, teal, cyan if can be added then it will be great

For building blocks i add all these colors again and again ....
And i think same goes for end user!

kball · 2017-05-24T23:15:22Z

@natewiebe13 @DaSchTour @IamManchanda
Do we have clarity on what we're doing here? It feels a little muddled to me, but I think this is a pretty cool idea and we should figure out how to move forward with it... so maybe let me see if I can state out what the situation is and where we are from trying to read through the above thread

It seems like we're trying to do a few things here

Separate the concepts of 'state' and 'color'.
Allow the possibility of multiple palettes (most common is probably a single "alternate palette")
Maintain ease of use for the common cases.

Are there any additional constraints I'm missing?

Looking through the conversation, it seems like keeping separate maps makes sense... what I'm not sure is if it makes sense to have the indirection from state to color palettes, especially when we start talking about alternate palettes, some arbitrary number of which may be state based. (e.g. imagine having state palette, alternate palette, social media palette, and possibly some accent colors)

I may be totally off base here, but lets walk down a thought experiment... The states matter because we use them to generate classes... so what if we instead just have an arbitrary number of palettes, and then we specify which ones create classes and which don't?

In pseudocode I'm thinking something like...

$brand-color-palette = (...)
$foundation-state-palette = (...)
$alternate-state-palette = (...)
$random-social-palette = (...)

$palettes-to-generate-classes = ($foundation-state-palette, $alternate-state-palette)

We could take this to get-color both by letting it accept a specific palette (e.g. `get-color('primary', $alternate-state-palette)) but also by having a precedence list for the basic lookup

$color-precedence = ($foundation-state-palette, $brand-color-palette, $random-social-palette)

Such that when get-color is invoked without a palette it looks through the palettes in that order to find a matching color.

Thoughts?

IamManchanda · 2017-07-03T16:50:19Z

Any updates Guys ... I think we should surely get this in for 6.5 ??

Poke @natewiebe13 @DaSchTour
Also tag @ncoden (as he is coming back slowly 😉 )

ncoden · 2017-07-03T21:18:20Z

Separate the concepts of 'state' and 'color'.

Allow the possibility of multiple palettes (most common is probably a single "alternate palette")

Maintain ease of use for the common cases.

I have the feeling that we are forgetting and refinding/repeating a lot if ideas we already discussed in the issue: #9671.

Please take a look at the piece of code we discussed before, they are already following the 3 points @kball given.

DaSchTour · 2017-07-04T05:40:12Z

I think that point 1 and 2 are somehow conflicting. I think we should abandon the concept of state-colors as it always didn't feel right. I think the best would be to have one single point to define colors, a color palette including such things as 'facebook-blue' and than mappings to components. Somethink that tells the button component that it should use facebook-blue for a button with the class facebook.

ncoden · 2017-07-04T10:54:27Z

@DaSchTour

Somethink that tells the button component that it should use facebook-blue for a button with the class facebook.

You miswritten "custom component state". ;)

DaSchTour · 2017-07-04T11:31:32Z

@ncoden well the problem with the term state is, that state is often something that changes and is part of a defined flow which describes the change between different states. I don't see how primary is a state. It's more of a characteristic that is set once and that is constant.

ncoden · 2017-07-04T17:10:16Z

@DaSchTour I agree with you on this point. In a clear CSS architecture, a state is not a modifier. The first is an event, part of a different context, so should have the priority on the second.

Generally in BEM+SMACSS, we use the declaration order to give a modifier (.component--modifier) the priority over the default behavior (.component), and an additional class to give the state (.component.state) the priority over the whole standard context. You already know that.

In our current ~~archaic~~ classic CSS architecture, "modifiers" are implemented with an additional class. So we have two solution for our "states": using two classes (component.is.state), or using one class and the declaration order and pray for it does not cause too much trouble. So we handle modifiers the same way we handle states. The only difference is that we must generate modifier classes before state ones.

Beside that, we also need a set of colors we can use (or not) freely inside components, without generating new dedicated classes.

So finally, we have "modifier", "state" and "color". We need three maps for each component, inherited from three maps at the global level (see my code examples in #9671), and an unique set of mixins the manage all of this easily without code duplication !

Also, I don't see how @kball's 1st and 2nd points are conflicting. We can add palettes globally or locally (from the config, function parameters...) by merging them into three different kind of internal palette in which we search for colors and states.

kball · 2017-09-08T22:26:44Z

bump @ncoden @DaSchTour @natewiebe13 @IamManchanda anyone interested in picking this back up and carrying it across the finish line?

DaSchTour · 2017-09-14T07:04:45Z

@kball I really have no idea which direction this should be carried. There are so many ideas for this topic and all of them somehow feel bad. While working on my current project I also see, that the concept of colors named with color names (blue, red and so on) as used in $foundation-colors is absolutely bad. I guess many pages will need colors named like (brand, secondary, highlight, alert) and maybe different shades of gray from white to black. So maybe the current concept doesn't need that many changes (or even any).

kball · 2019-07-24T18:32:17Z

Closing this as old WIP

Separate the Foundation palette into a map of colors, and a map to as…

9448cb8

…sign the colors to states

natewiebe13 added 2 commits January 20, 2017 13:17

Show missing key in error message

a02118a

Correct color name typo

f7287fe

ncoden reviewed Jan 20, 2017

View reviewed changes

natewiebe13 added 6 commits January 20, 2017 14:00

Update error message wording to be consistent

2f0649a

Revert settings file as this is automatically generated

085311c

Move color map to the color utility file

cfecb79

Use the safe method of getting a map value

610d044

Revert unrelated CSS changesg

2a95a9a

Remove unneeded checks

5132096

Update message about avoiding direct color usage in components

ca7ba75

ncoden reviewed Jan 20, 2017

View reviewed changes

natewiebe13 added 2 commits January 20, 2017 15:19

Import needed files for color unit test

7ffb49e

Update comment for how the color map should be used

84f61b0

Add unit tests for get-color and get-state

2fec363

ncoden reviewed Jan 20, 2017

View reviewed changes

Modify the unit tests to be more flexible

b83f720

ncoden reviewed Jan 20, 2017

View reviewed changes

Change unit test to not rely on other functions

cf22604

DaSchTour reviewed Jan 23, 2017

View reviewed changes

Merge branch 'v6.4' into color-palette-changes

970b67d

DaSchTour mentioned this pull request Apr 24, 2017

Palet colors required with get-color #9833

Closed

tdhartwick mentioned this pull request Apr 27, 2017

Alternate Color Schemes foundation/foundation-emails#754

Closed

IamManchanda assigned natewiebe13 and DaSchTour May 8, 2017

IamManchanda mentioned this pull request Jun 2, 2017

Discussion on naming convention of prototype utilities #10119

Closed

IamManchanda added this to the 6.5 milestone Jul 3, 2017

ghost mentioned this pull request Jul 28, 2017

Enable removing default colors from the $foundation-palette #10479

Merged

IamManchanda mentioned this pull request Jan 28, 2018

Assign a color first and then reassign the variables to semantic color scheme #9671

Closed

kball closed this Jul 24, 2019

[WIP] Separate Foundation Palette #9683

[WIP] Separate Foundation Palette #9683

Conversation

natewiebe13 commented Jan 20, 2017

IamManchanda commented Jan 20, 2017 • edited

ncoden Jan 20, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncoden Jan 20, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natewiebe13 commented Jan 20, 2017

ncoden commented Jan 20, 2017 • edited

Choose a reason for hiding this comment

ncoden commented Jan 20, 2017 • edited

natewiebe13 commented Jan 20, 2017

ncoden commented Jan 20, 2017 • edited

Choose a reason for hiding this comment

ncoden Jan 20, 2017 • edited

Choose a reason for hiding this comment

IamManchanda commented Jan 21, 2017

DaSchTour commented Jan 23, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IamManchanda commented Jan 24, 2017

DaSchTour commented Apr 21, 2017

IamManchanda commented Apr 24, 2017 • edited

kball commented Apr 25, 2017

DaSchTour commented Apr 26, 2017

IamManchanda commented Apr 26, 2017 • edited

DaSchTour commented Apr 26, 2017

IamManchanda commented Apr 26, 2017 • edited

kball commented Apr 27, 2017

joeworkman commented Apr 28, 2017

IamManchanda commented May 8, 2017 • edited

kball commented May 24, 2017

IamManchanda commented Jul 3, 2017

ncoden commented Jul 3, 2017

DaSchTour commented Jul 4, 2017

ncoden commented Jul 4, 2017 • edited

DaSchTour commented Jul 4, 2017

ncoden commented Jul 4, 2017 • edited

kball commented Sep 8, 2017

DaSchTour commented Sep 14, 2017

kball commented Jul 24, 2019

IamManchanda commented Jan 20, 2017 •

edited

ncoden Jan 20, 2017 •

edited

ncoden Jan 20, 2017 •

edited

ncoden commented Jan 20, 2017 •

edited

ncoden commented Jan 20, 2017 •

edited

ncoden commented Jan 20, 2017 •

edited

ncoden Jan 20, 2017 •

edited

DaSchTour commented Jan 23, 2017 •

edited

IamManchanda commented Apr 24, 2017 •

edited

IamManchanda commented Apr 26, 2017 •

edited

IamManchanda commented Apr 26, 2017 •

edited

IamManchanda commented May 8, 2017 •

edited

ncoden commented Jul 4, 2017 •

edited

ncoden commented Jul 4, 2017 •

edited