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

Prioritize stylesheets to decide which are concatenated and which are excluded #2322

Closed
westonruter opened this issue May 15, 2019 · 1 comment

Comments

Projects
None yet
2 participants
@westonruter
Copy link
Member

commented May 15, 2019

While stylesheets are being concatenated together for style[amp-custom], as soon as the the total reaches 50KB then all subsequent stylesheets are excluded. Each excluded stylesheet raises an excessive_css validation error.

This is wrong, and it leads to a very bad user experiences. Case in point is when the admin bar is being displayed on AMP pages, as the CSS for dashicons and admin bar get added before the CSS for the theme's stylesheet. Since the admin-bar.css is huge, this often results in the theme's own style.css being excluded, leading to completely broken templates. This is a big reason for why this admin-bar toggle exists in the first place:

image

For example, on Twenty Nineteen with Jetpack running and the admin bar shown, the scripts added are:

<link rel='stylesheet' id='twentynineteen-jetpack-css'  href='https://wordpressdev.lndo.site/content/plugins/jetpack/modules/theme-tools/compat/twentynineteen.css?ver=1557447387' type='text/css' media='all' />
<link rel='stylesheet' id='dashicons-css'  href='https://wordpressdev.lndo.site/core-dev/src/wp-includes/css/dashicons.css?ver=5.3-alpha-45282-src' type='text/css' media='all' />
<link rel='stylesheet' id='admin-bar-css'  href='https://wordpressdev.lndo.site/core-dev/src/wp-includes/css/admin-bar.css?ver=5.3-alpha-45282-src' type='text/css' media='all' />
<link rel='stylesheet' id='wp-block-library-css'  href='https://wordpressdev.lndo.site/content/plugins/gutenberg/build/block-library/style.css?ver=1557873353' type='text/css' media='all' />
<link rel='stylesheet' id='wp-block-library-theme-css'  href='https://wordpressdev.lndo.site/content/plugins/gutenberg/build/block-library/theme.css?ver=1557873353' type='text/css' media='all' />
<link rel='stylesheet' id='twentynineteen-style-css'  href='https://wordpressdev.lndo.site/core-dev/src/wp-content/themes/twentynineteen/style.css?ver=1.4' type='text/css' media='all' />
<link rel='stylesheet' id='twentynineteen-print-style-css'  href='https://wordpressdev.lndo.site/core-dev/src/wp-content/themes/twentynineteen/print.css?ver=1.4' type='text/css' media='print' />

On an AMP page, the result is:

The style[amp-custom] element is populated with:
     0 B: style[amp-custom=]
   106 B (2%): link#twentynineteen-jetpack-css[rel=stylesheet][id=twentynineteen-jetpack-css][href=https://wordpressdev.lndo.site/content/plugins/jetpack/modules/theme-tools/compat/twentynineteen.css?ver=1557447387][type=text/css][media=all]
   780 B (14%): link#dashicons-css[rel=stylesheet][id=dashicons-css][href=https://wordpressdev.lndo.site/core-dev/src/wp-includes/css/dashicons.css?ver=5.3-alpha-45282-src][type=text/css][media=all]
 17336 B (96%): link#admin-bar-css[rel=stylesheet][id=admin-bar-css][href=https://wordpressdev.lndo.site/content/plugins/amp/assets/css/admin-bar.css?ver=1.2-alpha1][type=text/css][media=all]
    99 B (46%): style#admin-bar-inline-css[id=admin-bar-inline-css][type=text/css]
    73 B (57%): link#amp-default-css[rel=stylesheet][id=amp-default-css][href=https://wordpressdev.lndo.site/content/plugins/amp/assets/css/amp-default.css?ver=1.2-alpha1][type=text/css][media=all]
  2168 B (18%): link#wp-block-library-css[rel=stylesheet][id=wp-block-library-css][href=https://wordpressdev.lndo.site/content/plugins/gutenberg/build/block-library/style.css?ver=1557873353][type=text/css][media=all]
   249 B (19%): link#wp-block-library-theme-css[rel=stylesheet][id=wp-block-library-theme-css][href=https://wordpressdev.lndo.site/content/plugins/gutenberg/build/block-library/theme.css?ver=1557873353][type=text/css][media=all]
  1116 B (80%): link#twentynineteen-print-style-css[rel=stylesheet][id=twentynineteen-print-style-css][href=https://wordpressdev.lndo.site/core-dev/src/wp-content/themes/twentynineteen/print.css?ver=1.4][type=text/css][media=print]
   122 B: style[type=text/css]
    39 B: style[type=text/css][media=print]
   225 B: style[type=text/css][media=screen]
    73 B: amp-img.wp-image-53 amp-wp-enforced-sizes amp-wp-b8332d9[class=wp-image-53 amp-wp-enforced-sizes amp-wp-b8332d9][src=https://wordpressdev.lndo.site/content/uploads/2019/02/1200px-American_bison_k5680-1.jpg][alt=][srcset=https://wordpressdev.lndo.site/content/uploads/2019/02/1200px-American_bison_k5680-1.jpg 1200w, https://wordpressdev.lndo.site/content/uploads/2019/02/1200px-American_bison_k5680-1-300x196.jpg 300w, https://wordpressdev.lndo.site/content/uploads/2019/02/1200px-American_bison_k5680-1-768x501.jpg 768w, https://wordpressdev.lndo.site/content/uploads/2019/02/1200px-American_bison_k5680-1-1024x668.jpg 1024w][width=1200][height=783][layout=intrinsic]
    93 B: div.wp-block-media-text has-media-on-the-right is-stacked-on-mobile amp-wp-2d949e1[class=wp-block-media-text has-media-on-the-right is-stacked-on-mobile amp-wp-2d949e1]
   231 B: figure.wp-block-media-text__media amp-wp-6c8fbd0[class=wp-block-media-text__media amp-wp-6c8fbd0]
    93 B: div.wp-block-media-text alignwide amp-wp-2d42cb8[class=wp-block-media-text alignwide amp-wp-2d42cb8]
    74 B: figure#attachment_53.wp-caption alignright amp-wp-d9900c6[id=attachment_53][aria-describedby=caption-attachment-53][class=wp-caption alignright amp-wp-d9900c6]
    74 B: figure#attachment_53.wp-caption aligncenter amp-wp-343bce0[id=attachment_53][aria-describedby=caption-attachment-53][class=wp-caption aligncenter amp-wp-343bce0]
Total included size: 22,951 bytes (54% of 42,385 total after tree shaking)

The following stylesheets are too large to be included in style[amp-custom]:
 29500 B (58%): link#twentynineteen-style-css[rel=stylesheet][id=twentynineteen-style-css][href=https://wordpressdev.lndo.site/core-dev/src/wp-content/themes/twentynineteen/style.css?ver=1.4][type=text/css][media=all]
Total excluded size: 29,500 bytes (58% of 50,020 total after tree shaking)

Total combined size: 52,451 bytes (56% of 92,405 total after tree shaking)

The most important stylesheet of all is the one being excluded!

image

At least the admin bar looks good! 😕

So what is needed is to introduce a sense of priority weights for the stylesheets as they are selected for concatenation.

Given the stylesheets loaded above:

The priority order should be something like:

  1. https://wordpressdev.lndo.site/core-dev/src/wp-content/themes/twentynineteen/style.css
  2. https://wordpressdev.lndo.site/content/plugins/gutenberg/build/block-library/style.css
  3. https://wordpressdev.lndo.site/content/plugins/gutenberg/build/block-library/theme.css
  4. https://wordpressdev.lndo.site/content/plugins/jetpack/modules/theme-tools/compat/twentynineteen.css
  5. https://wordpressdev.lndo.site/core-dev/src/wp-includes/css/dashicons.css
  6. https://wordpressdev.lndo.site/core-dev/src/wp-includes/css/admin-bar.css
  7. https://wordpressdev.lndo.site/core-dev/src/wp-content/themes/twentynineteen/print.css

So the prioritized categories:

  1. Parent theme stylesheets (e.g. style.css) which have all/screen media
  2. Child theme stylesheets (e.g. style.css) which have all/screen media
  3. Core stylesheets used by themes (e.g. wp-block-library and wp-block-library-theme), including the ones included by Gutenberg
  4. Stylesheets from plugins (with all/screen media)
  5. Inline style elements and attributes
  6. Stylesheet for Dashicons
  7. Stylesheet for admin-bar
  8. Stylesheets with print media

This should greatly reduce the frequency of templates appearing completely broken when CSS is being excluded.

The prioritization could be indicated when displaying the manifest of the stylesheets in style[amp-custom] (including on validated URL admin screen: #2169).

Bonus: once we get to the admin bar CSS and we determine it cannot be loaded, then we can automatically remove the admin bar from the page. (Which is an alternative to #1921.)

@westonruter westonruter added this to the v1.2 milestone May 15, 2019

@swissspidy

This comment has been minimized.

Copy link
Collaborator

commented May 15, 2019

I like this, especially since it is a good alternative to #1921!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.