cloudfour / pwastats

A directory of Progressive Web App case studies.
https://www.pwastats.com
MIT License
101 stars 8 forks source link

Purgecss #160

Closed calebeby closed 6 years ago

calebeby commented 6 years ago

Adds a tool to remove unused css

gerardo-rodriguez commented 6 years ago

@calebeby One other detail I noticed, the horizontal spacing between the page numbers is different.

Left: this branch (greater in-between horizontal spacing) Right: pwastats.com

screen shot 2018-07-11 at 10 54 43 am

calebeby commented 6 years ago

Regarding the pagination spacing, this rule is being removed:

.Pagination > *:nth-last-child(-n+5):first-child,
.Pagination > *:nth-last-child(-n+5):first-child ~ * {
  flex: 0 1 calc(100% / 8);
}

Apparently purgecss has trouble understanding those selectors

So do I

If I understand these selectors correctly, it is selecting:

  1. The last 5 elements, if they are the first child (🤔)
  2. Elements that come after the last 5 elements that is a first child (😕)

Apparently this was to work around a safari bug?

This confuses me greatly, @tylersticka I would appreciate if you could explain this for me.

Edit: Oh, I see, it is selecting if there are 5 or less items.

calebeby commented 6 years ago

@tylersticka I'm unsure exactly what the desired behavior is, but can I replace this:

.Pagination > *:nth-last-child(-n+5):first-child,
.Pagination > *:nth-last-child(-n+5):first-child ~ * {
  flex: 0 1 calc(100% / 8);
}

with this:

.Pagination > * {
  max-width: <insert value here>
}

This will cap the size of the pagination links, so on larger screens they don't continue to stretch.

tylersticka commented 6 years ago

@calebeby I'm not comfortable making that change in the pattern library because there's a lot of history and edge-cases to consider. Also, I'm just not a fan of altering CSS to satisfy a build process.

I'm not sure the visual change is enough to justify blocking this PR, though props to @gerardo-rodriguez for spotting it. I would probably create a new issue to track it but proceed as-is.

calebeby commented 6 years ago

OK I will open an issue there