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

Bug: color keyword errors on variables named as literals #126

Closed
minamarkham opened this issue Sep 6, 2015 · 4 comments
Closed

Bug: color keyword errors on variables named as literals #126

minamarkham opened this issue Sep 6, 2015 · 4 comments

Comments

@minamarkham
Copy link

Using 1.1.0, when color-keywords is linted, the following is disallowed:

$blue: #0050a0;
$brand-primary: $blue;
@DanPurdy
Copy link
Member

DanPurdy commented Sep 6, 2015

Yeah so this was a hangover actually, basically it was to prevent people doing the following:

$colors: (
  red: #F00,
  blue: #00F
)

As in use the same name as colour literals as map identifiers, I've seen this in a few places before, I think I just got a little too sensitive to variable names in the end.

I'm still in my own work trying to avoid using variable names such as $blue because I think you end up with the same problem this is trying to avoid.

In certain projects I've looked at you'll see the following:

$blue: #0050a0

.button {
  color: $blue
}

.text {
  color: $blue;
}

If you have that all over the place and you have to make the colour green later on you end up with the same problem of searching the codebase to rename all the variables as you would of had using literals. You can't just assign the hex for green to $blue. In the case you have above it's completely manageable and legitimate but it's really difficult to write a linter rule to know the context (though I'm up for trying!).

Abstracted variable names therefore that describe context rather than colour are the best possible way to avoid this problem altogether.

I've got a fix ready to prevent this flagging up again but i'd like to open up some discussion about whether we should add the hotfix or even make a new rule that prevents the use of colour literal names in variables too or perhaps a flag of severity on this rule?

- color-keywords:
- 1
-
  prevent-variable-names : true

I hope my rambling makes sense 😄

@DanPurdy DanPurdy self-assigned this Sep 6, 2015
@DanPurdy DanPurdy mentioned this issue Sep 6, 2015
5 tasks
@Snugug Snugug added this to the 1.2.0 milestone Sep 7, 2015
@Snugug
Copy link
Member

Snugug commented Sep 7, 2015

I think color keywords should only check to see if color keywords are used where colors are used, but not care about variables or keys in maps. Those should have separate rules I think

@joshuacc
Copy link
Contributor

joshuacc commented Sep 7, 2015

Just to throw my two cents in... I think that color keywords should only be prohibited on CSS properties. Otherwise, the rule prohibits a (IMO good) practice of defining human-readable names for flowing through a color scheme like so:

$blue: rgb(10, 10, 255);
$red: rgb(255, 10, 10);

$action-accent: $blue;
$action-text-accent: darken($blue, 10%);
$danger-accent: $red;
$danger-text-accent: $red;

If that is to be prohibited, it should be as part of a separate rule.

@DanPurdy
Copy link
Member

DanPurdy commented Sep 7, 2015

I agree, thanks for the input, I'll roll the fix out to stop this flagging up and then possibly roll out a new rule to prevent their use in variable names.

As I mentioned I think the way you guys are highlighting there is completely acceptable it's when you see$blue used directly on a property that you end up with the same problems maintaining the code.

donabrams pushed a commit to donabrams/sass-lint that referenced this issue Nov 19, 2015
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

4 participants