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

Update bitters to match thoughtbot's style preferences #260

Closed
creuter opened this issue Aug 29, 2016 · 4 comments
Closed

Update bitters to match thoughtbot's style preferences #260

creuter opened this issue Aug 29, 2016 · 4 comments

Comments

@creuter
Copy link

creuter commented Aug 29, 2016

When installing bitters, Hound throws a few comments because it doesn't quite match our style preferences.

$form-box-shadow: inset 0 1px 3px rgba(#000, 0.06);

Color literals like rgba(#000, 0.06) should only be used in variable declarations; they should be referred to via variable everywhere else.

color: #fff;

Color literals like #fff should only be used in variable declarations; they should be referred to via variable everywhere else.

+#{$all-buttons} {

Rule set contains (19/10) properties

@whmii
Copy link
Contributor

whmii commented Aug 29, 2016

  1. I actually disagree with the linter here. #000 is in a variable. open to discussion though.
  2. bourbon 5 introduces a color-switch (or something like that) function. we are biding our time until the full release so that we can just use that instead. in the meantime I can use $base-background-color instead if that is preferable
  3. seems like more of a suggestion than rigid enforcement. If there is one property that would/should have a lot of rules on it, it would be buttons. They have to override a large amount of browser styles. definitely open to changes tho.

@tysongach
Copy link
Contributor

#294 address the second warning (“Color literals like #fff”), per @whmii’s suggestion.

In regards to the third warning, I think we just need to disable that linter. I agree that this can’t really be a rigid enforcement.

@tysongach
Copy link
Contributor

We disable the PropertyCount linter (which is what throws the warning “Rule set contains (19/10) properties”) in the default scss-lint config our guides. I think that’s a fine solution and I’m not sure we can change much here in Bitters to do anything more.

By default, Hound also uses our default scss-lint config on all thoughtbot repos, so we should be set there. However, if you run scss-lint locally without any configuration, their default config has PropertyCount and set to a max of 10 properties.

@tysongach
Copy link
Contributor

I’m going to close this. As of the latest release (1.7.0), we are now down to just one warning when using the scss-lint config from our guides:

base/_forms.scss:1:36 [W] ColorVariable: Color literals like `rgba(#000, 0.06)` should only be used in variable declarations; they should be referred to via variable everywhere else.

I think this is a failure of the linter. It points to this line which is using a variable and the color that I think it doesn’t like is #000 which is pure black, not some color from a scheme.

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

3 participants