Configure CSSLint and validate MDN CSS

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: shobson, Unassigned)

Tracking

Details

(Reporter)

Description

4 years ago
Decide on some default configuration values for CSSLint and address warning and errors that CSSLint comes up with when running with those settings.
(Reporter)

Comment 1

4 years ago
adjoining-classes
- We don't support IE6

box-model
- We're seasoned enough devs to avoid the pitfalls of this.

box-sizing
- We don't support IE7

bulletproof-font-face-face-syntax
- Already doing this!

compatible-vendor-prefixes
- We stopped using -o- but they haven't.

display-property-grouping
- According to the spec display: inline doesn't apply margins, but the browsers implemented it and we rely on it and so it would be work to get our stuff to validate against this.

duplicate-background-images
- The fact that we use an icon font mostly does away with the need for this. 
- #todo Probably worth exploring on a slow day.

duplicate-properties
- There's a few places where we use @extends and get duplicate properties, not ideal but changing it to work another way would be a chore and our minimizer handles this so it's a low priority.

empty-rules
- Something else the minimizer handles for us.
- #todo our break points get spat out without rules in compiled files, not sure why.

fallback-colors
- We don't support IE8.
- #todo Without fallback colours this can actually get really unreadable in a way that columns that don't line up can't. I think it's worth addressing this in the future.

floats
- An OOCSS nice to have but not necessary.
- #todo something else worth exploring on a slow day as part of a optimization effort.

font-faces
- Already comply! Woohoo!

font-sizes
- Another OOCSS nice to have. It'd be a lot of work for not much pay off.

gradients
- We don't support the older browsers this targets.

ids
- If we were starting our site from scratch I would turn this on. We're not starting our site from scratch.

import
- Already comply! Let's keep it that way.

important
- Again, if we were starting our site from scratch....
- #todo Good string to pull on if we want to unravel the site CSS and try to knit a more efficient sweater.

known-properties
- Enabling this prevents us from using experimental properties like display:flex. So, not enabling this.

outline-none
- We have 2 valid exceptions to this in place with alternative displays. I'd like to enable it but there's no way to tell csslint that these two instances are okay.

overqualified-elements
- #todo We don't fail this too badly, a half day of work and testing would probably allow us to enable this, but it's low priority.

qualified-headings
- Another OOCSS nice to have that we don't have.

regex-selectors
- Not too worried about selector performance (http://benfrain.com/css-performance-revisited-selectors-bloat-expensive-styles/) and we have pretty heavy reliance on them, changing it would be a pain.

rules-count, selector-max, selector-max-approaching
- Passing, might as well leave them as errors. Probably a bigger issue for testing on our minified files though.

shorthand
- Passing. Might as well leave it in, it's a good habit to enforce.

star-property-hack
- Going to make this a warning. I'm okay with selective use of hacks and if we need an IE7 and under only rule I'd rather this hack than a separate style sheet.

text-indent
- Leaving this in as a warning, especially since we have support for RTL

underscore-property-hack
- See comment on star-property-hack

unique-headings
- Another OOCSS nice to have that would be a lot of work.

universal-selector
- I'd love to turn this on but we rely on it for a lot of last-child stuff

unqualified-attributes
- See comment for regex-selectors

vendor-prefix
- Enabled. CSSlint is correct we should list the standard property with vendor prefixed properties.

zero-units
- The minimizer handles this.

Comment 2

4 years ago
Commits pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/de788118ae67fced7d8e2120e283d3a8c5f3ff6c
Bug 1082853: Configure CSSLint and validate MDN CSS

Made a more decerning vendorize mixin with lookup tables for values.
Made a mixin for creating rgba fallbacks.
Remove table.activity which was presumably added back in during a merge somewhere).
Validate CSS, mostly working with outlines.

https://github.com/mozilla/kuma/commit/0166eee45a7db439b2399075cdab97468f8ac4ca
Merge pull request #2840 from stephaniehobson/BUG-1082853-csslint

Bug 1082853: Configure CSSLint and validate MDN CSS

Comment 3

4 years ago
Commits pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/db11264848390263f19fee5df40749766e41e758
bug 1082853 - Remove stylus vendorize warning

https://github.com/mozilla/kuma/commit/07d0ad3117c6fde33b1e73c0c176e75186a4a7e0
Merge pull request #2862 from darkwing/fix-vendorize

bug 1082853 - Remove stylus vendorize warning
This seems to be done? Or do we want to add CSSLint to the Travis checking?
Flags: needinfo?(dwalsh)
I'd like to let :shobson own CSSLint and the decision-making there, as she configured the rules and will likely be touching CSS the most moving forward.  Let's keep this as a local VM utility until she returns from PTO and we can talk about next steps then.

Awesome work Stephanie!

Marking as resolved:fixed for now as the core task is done.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(dwalsh)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.