scss-lint icon indicating copy to clipboard operation
scss-lint copied to clipboard

Feature request: change VariableForProperty to allow maps

Open VincentSmedinga opened this issue 10 years ago • 6 comments

We’re using maps and functions to manage and call our variables for colors and fonts, e.g. $ui-colors: (…), $ui-color-variations: (…), { color: ui-color(primary, darker); } and { font-family: ui-font-stack(interface); }.

The VariableForProperty rule warns about this and asks me to use variables. But I am using variables, only behind another layer of abstraction. Is allowing function calls for these properties something you could support?

VincentSmedinga avatar Aug 19 '15 10:08 VincentSmedinga

I understand where you are coming from, but you really have to ask yourself if using functions/maps is going with the spirit of what the check is intended to enforce.

From the documentation:

By using variables, you can describe the semantics of the property value rather than just using the literal value (improving readability) and also make it easier to perform site-wide changes as you only need to change the value in one place, rather than several.

If you're defining your colors as ui-color(primary, darker) in more than one place, you're not using a "semantic" color like $body-text or $border-shadow. Thus when the time comes to change your scheme (and that time always comes sooner or later, as we all know), you'll be struggling to figure out which occurrences of ui-color(primary, darker) are referring to a border and which are referring to some other aspect of your color scheme.

Using semantic variable names makes it trivial to change the scheme: you change the value in one place and that is immediately reflected everywhere as intended. Maps kind of have this property, but I would argue that they are less semantic when compared to plain variables.

What are your thoughts? Does this explain the intent?

sds avatar Aug 19 '15 16:08 sds

Thanks for thinking along.

One of the reasons I still find this a logical suggestion, is that the combination of maps and functions in itself is just a further abstraction of using variables. Obviously, the specific maps I mention contain exactly that: plain variables for colors and their variations.

Changing the scheme is possible in more than one way: either changing a color value or a variation function or argument. So this setup provides a easy to manage, consistent model for the entire palette. If a certain color variation is used for both a border-shadow and a panel-background, we explicitly chose them to be the same tint and we want them to change equally when modifying an underlying aspect of that color variation. The entire set of variations is related, as each basic color is derived into lighter or darker hues, or more transparent variations, in exactly the same way. Tweaking a specific border-shadow defined as a standalone variable would probably be more random than that.

Just to be complete, and this shouldn’t add to or remove from my argument, we use these only as intermediate values. For example, $ui-modal-border-color: ui-color(neutral, light) or $ui-text-color: ui-color(text). So there’s still information on how the color variations are actually used. We would never find and replace one ui-color call with another all over the place.

With the font stacks, it’s even more obvious. Why would $ui-font-stack(interface), which this check rejects, be any worse than $ui-font-stack--interface, which it accepts? Using maps here just makes it easier to add another variant or inject a different font into an existing variant, with no real setback in the way we invoke them.

So why would a Sass linter allow variables for properties, but not functions returning calculations on variables contained by a map? They are principally the same. Also, in neither case can the linter verify whether the names used are any kind of ‘semantic’, so no difference there as well. I’m not really seeing why one would be going with the spirit of the check and another against it.

VincentSmedinga avatar Aug 31 '15 21:08 VincentSmedinga

I concur that function or map returns should not be flagged by VariableForProperty. Similarly to @VincentSmedinga, our project has a core palette, which we store and access via maps or functions.

If we use a particular color for a border and a link, we are specifically choosing for them to be connected. When one changes, all are supposed to change.

Creating vars like $border-color and $link-color becomes extra and unnecessary work, because for us, usage distinction isn't important. What is important is that when we use a blue, the same blue is used throughout the project.

karmasalad avatar Sep 01 '15 22:09 karmasalad

If we use a particular color for a border and a link, we are specifically choosing for them to be connected. When one changes, all are supposed to change.

... extra and unnecessary work, because for us, usage distinction isn't important.

@karmasalad -- I would say the exact opposite can easily be true. And, in most of my projects it is. I've indeed had to change a color in all cases... e.g. our brand identity is no longer blue, so now we are red.

However, it's just as often that a design changes where, for example, we still want borders to be blue, but only the links should now be red. Or, in a project I'm working on now, there's a high-contrast a11y "mode". In that mode, some usages need to be switched from blue to yellow. But, wholesale switching every blue to yellow would actually make some areas less accessible, so we need to make distinctions in how and where the colors are used.

chris-dura avatar Sep 02 '15 00:09 chris-dura

I see your point, @VincentSmedinga (though to be honest I don't feel strongly either way).

Happy to accept a pull request allowing VariableForProperty to allow maps. Thanks for your input!

sds avatar Sep 04 '15 05:09 sds

We are using a very similar system as @VincentSmedinga . This would be an awesome feature, has anyone added this in yet or has gotten it to work?

artemartemov avatar Jun 23 '16 17:06 artemartemov