liferay / liferay-npm-tools

Collection of tools for using npm in Liferay
Other
18 stars 15 forks source link

Sort SCSS map entries #474

Closed wincent closed 4 years ago

wincent commented 4 years ago

As requested here.

Example of incorrect code:

$navigation-bar-size: (
    border-bottom-width: 0.0625rem,
    active-border-offset-bottom: -0.5625rem,
    active-border-offset-bottom-mobile: -0.5625rem,
);

Example of correct code:

$navigation-bar-size: (
    active-border-offset-bottom: -0.5625rem,
    active-border-offset-bottom-mobile: -0.5625rem,
    border-bottom-width: 0.0625rem,
);

Based on a quick search, I couldn't find any pre-existing rules that would do this for us, so I believe we'll have to cook up a custom rule.

pat270 commented 4 years ago

@wincent I don't think sorting SCSS map entries is the right thing to do right now. We have maps like:

$grid-breakpoints: (
  xs: 0,
  sm: 576px,
  md: 768px,
  lg: 992px,
  xl: 1200px,
) !default;

where the first entry needs to be 0 and values be ascending. Bootstrap's grid mixins depends on the order being correct so the styles cascade properly.

The map would make Sass fail, but if it didn't:

$grid-breakpoints: (
  lg: 992px,
  md: 768px,
  sm: 576px,
  xl: 1200px,
  xs: 0,
) !default;

it would output:

.col-lg-3, .col-md-4, .col-sm-6 {
  width: 100%;
}

.col-lg-3 {
  @media (min-width: 1200px) {
    max-width: 25%;
  }
}

.col-md-4 {
  @media (min-width: 768px) {
    max-width: 33.33333%;
  }
}

.col-sm-6 {
  @media (min-width: 576px) {
    max-width: 50%;
  }
}

The markup <div class="col-sm-6 col-md-4 col-lg 3"> will always be 50% wide when the screen is >= 576px.

pat270 commented 4 years ago

I just thought of an idea.

$navigation-bar-size: (
    border-bottom-width: 0.0625rem,

    active-border-offset-bottom: -0.5625rem,
    active-border-offset-bottom-mobile: -0.5625rem,
);

No sorting needed here!

wincent commented 4 years ago

Well that's a bummer. I thought the maps were unordered collections.

wincent commented 4 years ago

(Closed because I'd rather not implement a more subtle/complicated version of this rule — sort unless separated by blank line — unless Brian asks for this again.)

pat270 commented 4 years ago

We can revisit this when we refactor the Bootstrap stuff, then lets-pho-it-up