bbc / simorgh

The BBC's Open Source Web Application. Contributions welcome! Used on some of our biggest websites, e.g.
https://www.bbc.com/thai
Other
1.4k stars 225 forks source link

Address CSS duplication #550

Closed sareh closed 5 years ago

sareh commented 6 years ago

Is your feature request related to a problem? Please describe. This issue arises from the discussion here https://github.com/bbc/simorgh/pull/511#discussion_r212074048

By separating the Typography CSS out to a different library file and still having a media query in the component for other styles, we get verbose CSS output.

@media (min-width:37.5em) {
  .c0 {
    font-size: 0.875em;
  }
}

@media (min-width:37.5em) {
  .c0 {
    padding: 0.5rem 1rem;
  }
}

Instead of

@media (min-width:37.5em) {
  .c0 {
    font-size: 0.875em;
    padding: 0.5rem 1rem;
  }
}

We should consider optimisation of styles outputted by styled-components.

Note that we should also consider other examples with multiple components using the same typography groups.

<p class="123">some text.</p>
<figure>
  <img ...>
  <figcaption class="123 456">some caption.</figcaption>
</figure>
.123 {
  * typography definition here *
};

.456 {
  * similar media query here *
};

Describe the solution you'd like A decision to be made - to keep the Typography styles split out in the current way or to change it. To explore if there are any optimisations within StyledComponents that could reduce duplication of CSS.

ChrisBAshton commented 5 years ago

Looks like we may be able to solve the first case by specifying that our CSS is 'pure': https://www.styled-components.com/docs/tooling#dead-code-elimination

jamesbhobbs commented 5 years ago

The stated problem is hard to reproduce as the issue is so old. I await anyone finding a similar example where dead code elimination isn't happening.

There is an example such as this... `@media (min-width:25rem) { .c0 { height: 5rem; padding: 0 1rem; } }

@media (min-width:25rem) { .c4 { padding-top: 1.75rem; padding-bottom: 1.5rem; } }`

But these are styles applied to different components, or at least different classes, and therefore them being separate while not the most performant pure CSS should help dead code elimination.

Nevertheless the two PRs above are performance improvements, and seem not to break anything. So thanks to @ChrisBAshton for the suggestion.