Feature: Implement StyleLint about theme-scaffold HOT 14 CLOSED

Let's also make sure to add this to lint-staged to enforce the rules.

from theme-scaffold.

1. I'd like to use https://github.com/BPScott/stylelint-prettier (as found by @dainemawer)
2. I can see myself having to disable property-no-unknown every now and again. Let's make sure we document how to do that.
3. block-no-empty seems unnecessary. The processing/minification step should eliminate those.
4. I think that we should avoid more opinionated lint rules, like name patterns and disallowed selectors. This is more specific than we usually are with approaches. I feel like stylelint should focus more on catching actual errors than enforcing best practices/opinions.
5. I'd be open to using declaration-no-important, as it's just adding an extra barrier.
6. The spaces/newlines should probably echo those from the WordPress coding standards.

from theme-scaffold.

Let's make sure we all (the FE team) agree on the linting rules before anything get PR'd for this

from theme-scaffold.

@timwright12 I thought of creating a separate package for the rules, something like: @10up/styelint-config at least we can separate implementing the functionality within the scaffold out from writing the rules. I'll ping you for the best way to come to a consensus about the rules!

from theme-scaffold.

@10up/styelint-config sounds good

from theme-scaffold.

I'd like to see a rule around using color variables (I don't know if one exists), essentially not allowing hexcodes in the pre-processed CSS. My top 2 comments in code reviews are "use a color variable here" and "alpha sort these selectors"

from theme-scaffold.

I would like to see using actual color names and not generic ones. I believe this eases cognitive overload when jumping from project to project.

So, this: $color-thistle: #d7bbd4 or --color-thistle: #d7bbd4 instead of this: $color-brand-1: #d7bbd4 or --color-brand-1: #d7bbd4

When I jump from project B to project A and they both have $color-brand-1 I have no memory or context for what brand-1 is, but I could maybe remember something better, like $color-thistle. I've personally found this more scaleable. It also breathes a little personality into the overall project in a way, which I believe is important in all of our day-to-day.

I recommend we add a link in top of theme-scaffold _colors.scss partial in header comment to this site for naming: http://chir.ag/projects/name-that-color/#D7BBD4
It is an ugly site, but quite useful. This is where one can derive color names from hex values when scaffolding a new project.

Ideally, everybody has an extension like Color Highlight (VS Code) or Pigments for (Atom) to also help get an idea of color variables when coding along.

I'm not certain that this can be addressed with Linting, and will likely intertwine with Engineering Best Practices and/or theme-scaffold?

@timwright12 will likely have some insight here please?

from theme-scaffold.

@colorful-tones the undocumented best practice for color variables is to actually use double vars like:

--c-red: #ff0000;
--c-branding: var(--c-red);

If we're just using var(--c-red) directly in the CSS, it really isn't any better than not using the variable at all. Then, for instance, if you wanted to update the branding color to "blue" you'd have to find all instances of var(--c-red) being used as branding (nothing else) and replace it with the new color - a manual process.

Alternatively you'd hit the same issue if you just wanted to change the shade of link colors a bit, if var(--c-blue) is used in more than one context you'd need to parse through all of them to figure out where the updates need to be, rather than just updating the --c-links variable.

As a general recommendation, I would review the variables file while onboarding to a project to avoid any confusion early on.

from theme-scaffold.

@timwright12 💯 about using double vars. This is what I practice too, but left out for sake of time, and was looking as a Phase II suggestion :)

from theme-scaffold.

@dainemawer any updates on this?

from theme-scaffold.

@timwright12 yup! I'll ping you on slack just about setting up a 10up owned repo, I've translated the WordPress CSS Coding Standards into a config as well as added Styelint Order (Alpha Sorting) and Stylelint Use Variables which will force engineers to use a variable on any /color/ property (background-color, border-color, etc) - need to do a bit of work on some of the more "opinionated" rules but this is a good place to start.

from theme-scaffold.

Not sure if this was a given but should also apply rules based on https://10up.github.io/Engineering-Best-Practices/css/.

from theme-scaffold.

#79

from theme-scaffold.

Success, thanks for the hard work everyone!

from theme-scaffold.