Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tree-shake CSS selectors for HTML elements that target non-active languages #1221

Merged
merged 1 commit into from
Jun 22, 2018

Conversation

westonruter
Copy link
Member

In Twenty Seventeen there is about 5KB+ of rules with selectors to add styles that are specific to various languages:

https://github.com/WordPress/wordpress-develop/blob/a830dbcab3147e67b3076d3d15c89975df675fbb/src/wp-content/themes/twentyseventeen/style.css#L582-L913

Based on the site's locale, we should only include the rules that are relevant. In other words, this is attribute-based tree-shaking for the root element. It doesn't really make sense for all of these to be always served in the CSS. Note that amp-bind'ed [lang] is not allowed, so there is not the concern that they will become relevant later: the language won't be changed at runtime.

The changes in this PR reduce the resulting CSS size from 33,129 bytes down to 27,741 bytes on an example page in English, a 16% reduction.

@westonruter westonruter added this to the v1.0 milestone Jun 22, 2018
@westonruter westonruter requested a review from kienstra June 22, 2018 01:06
Copy link
Contributor

@kienstra kienstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Hi @westonruter,
Good idea to remove all of these html[lang] rules. I didn't know there were so many.

This worked as expected locally. There are no extra language rules with this PR when using Native AMP with Twenty Seventeen, where there were several before:

lang-selectors

@westonruter westonruter merged commit c74a97a into develop Jun 22, 2018
@westonruter westonruter deleted the add/html-lang-tree-shaking branch June 22, 2018 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants