search

ampproject / amp.dev

The AMP Project Website.
https://amp.dev
Other
581 stars 694 forks source link

Add Pixi rule for Icon Font Usage #5325

Closed kristoferbaxter closed 3 years ago

kristoferbaxter commented 3 years ago

🚀 Feature Request

Problem

Icon fonts have a negative impact on CLS, causing layout shift as they load in (unless blocked from loading). Document authors are unaware of the impact since it rarely appears on high-speed connections.

Solution

Identify when icon fonts are used in a document, (potentially via screenshot diffing or checking the content of each text node for known patterns). When detected present this as an opportunity to address CLS.

Alternatives

  1. Use screenshot diffing
  2. Detect nodes with :before and known icon-font providers.
  3. Provide confidence of violation when known, and just warn when uncertain.
sebastianbenz commented 3 years ago

Oh - this is a good one! They often negatively impact LCP as well.

I think checking

for common font providers should go a long way.

nainar commented 3 years ago

@sebastianbenz is this something the JvM team can take on soon?

matthiasrohmer commented 3 years ago

Sure, @lluerich let's briefly discuss this as soon as you've made your way through the Grow 1-Highlighting-Jungle.

lluerich commented 3 years ago

I found a long list of popular icon fonts as a starting point on css-tricks.com. Some more research led me to believe that the most commonly used fonts are as follows:

As far as I can see it, these can all be identified by checking for a certain class name: font classname
Font Awesome .fa
Material Design Icons .material-icons
Octicons .octicon
Nerdfonts .nf
Icofont .icofont-

If we implement a check for classnames for different icon fonts, we can always add more to the list.

matthiasrohmer commented 3 years ago

Status from the amp.dev engineering sync: .fa and .nf might render false positives for AMP pages that use class name minifiers. We look for a (non RegEx based, see) solution to verify the results.

Class names can be checked as the AMP linter has access to the DOM structure ($('.fa').length would work).

sebastianbenz commented 3 years ago

See https://github.com/ampproject/amp-toolbox/pull/1145/ for AMP Linter integration.