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

Feature Request: Previous Group Threshold

Open thegranddesign opened this issue 8 years ago • 1 comments

This isn't the "How many properties do I need to group anything" option that already exists, this is a more flexible (and I think better) option to have the linter decide when any group needs to be grouped with a previous group.

This is best illustrated with a couple examples:

Let's say that we have a simplified property grouping of:

display
position
top
right
bottom
left

visibility
opacity
z-index

outline
outline-offset
outline-width
outline-style
outline-color

border
border-top
border-right
border-bottom
border-left

margin
margin-top
margin-right
margin-bottom
margin-left

padding
padding-top
padding-right
padding-bottom
padding-left

width
min-width
max-width

height
min-height
max-height

background
background-color
background-image
background-repeat
background-position
background-size

Currently as it stands, the following CSS would be desired by the linter:

.my-class {
  display:                  absolute;

  z-index:                  1000;

  outline:                  1px dashed #ff00ff;

  border:                   10rem solid #ff0000;

  margin:                   1px 2px 3px 4px;

  padding:                  100px;

  width:                    900px;

  height:                   300px;

  background:               url('/foo/bar.jpg');
}

That's a lot of extra spacing and noise in my opinion. I think that in these cases, the groups should be allowed to be collapsed and I think the rule should be:

Look at the group before you. Does it have less than threshold properties and does my group have less than threshold properties? If yes, then allow (but don't require) it to be grouped with the previous group.

With this rule you could have something like this:

.my-class {
  display:                  absolute;
  z-index:                  1000;

  outline:                  1px dashed #ff00ff;
  border:                   10rem solid #ff0000;

  margin:                   1px 2px 3px 4px;
  padding:                  100px;

  width:                    900px;
  height:                   300px;

  background:               url('/foo/bar.jpg');
}

Or some combination of that, however if you wanted to add some more specific widths and positional properties like so:

.my-class {
  display:                  absolute;
  top:                      10px;
  left:                     20px;

  z-index:                  1000;

  outline:                  1px dashed #ff00ff;
  border:                   10rem solid #ff0000;

  margin:                   1px 2px 3px 4px;
  padding:                  100px;

  width:                    900px;
  min-width:                300px;
  max-width:                1200px;

  height:                   300px;

  background:               url('/foo/bar.jpg');
}

Then, if the threshold was 2, the width section there would be required to be broken out as a separate group from padding (since it's group is over the threshold) and the height group would be required to be broken out as well (since it's previous group is over the threshold). And the same rules apply to the position section.

I think that this would allow for much more compact CSS (while still maintaining the proper order) for those times when you mainly use shorthand properties (eg padding) rather than longer form properties (eg border-left-color).

thegranddesign avatar Sep 20 '16 19:09 thegranddesign

Thank you for writing up a detailed proposal for this potential feature, @thegranddesign.

The PropertySort linter is already notoriously complicated. Each new "option" we add to it further complicates things.

With that said, I'm not against this, but I would like any implementation to provide backwards-compatible support with the existing implementation (i.e. there should be a single option that enables this and it should default to be off).

I'm skeptical of the value such a specific check is providing, but that's entirely personal opinion, and it's likely the structure you're aiming for in your SCSS would benefit from automated enforcement of the pattern you've described.

Depending on how complex the change is, I'd perhaps like to see a "show of hands" for how many others would benefit from this. However, if the change isn't too complicated, feel free to submit a pull request (with comprehensive tests!!!) and I'll be happy to merge it. Thanks!

sds avatar Sep 28 '16 02:09 sds