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

Display byte size before amp-custom sources instead of after #1318

Merged
merged 3 commits into from Aug 4, 2018

Conversation

Projects
None yet
3 participants
@westonruter
Member

westonruter commented Aug 4, 2018

To improve readability of the manifest of style[amp-custom], move the byte size from the end to the beginning of the line. Also include the sum of bytes that are being excluded.

Before

The style[amp-custom] element is populated with:
- style[amp-custom=] (0 bytes)
- link#twentysixteen-jetpack-css[rel=stylesheet][id=twentysixteen-jetpack-css][href=https://src.wordpress-develop.test/wp-content/plugins/jetpack/modules/theme-tools/compat/twentysixteen.css?ver=6.2.1][type=text/css][media=all] (338 bytes)
- link#dashicons-css[rel=stylesheet][id=dashicons-css][href=https://src.wordpress-develop.test/wp-includes/css/dashicons.css?ver=4.9.8-beta2-43516-src][type=text/css][media=all] (466 bytes)
- link#admin-bar-css[rel=stylesheet][id=admin-bar-css][href=https://src.wordpress-develop.test/wp-content/plugins/amp/assets/css/admin-bar.css?ver=1.0-beta1][type=text/css][media=all] (17168 bytes)
- style#admin-bar-inline-css[id=admin-bar-inline-css][type=text/css] (127 bytes)
- link#wp-core-blocks-css[rel=stylesheet][id=wp-core-blocks-css][href=https://src.wordpress-develop.test/wp-content/plugins/gutenberg/build/core-blocks/style.css?ver=1533089052][type=text/css][media=all] (230 bytes)
- link#genericons-css[rel=stylesheet][id=genericons-css][href=https://src.wordpress-develop.test/wp-content/plugins/jetpack/_inc/genericons/genericons/genericons.css?ver=3.1][type=text/css][media=all] (1032 bytes)
- link#twentysixteen-style-css[rel=stylesheet][id=twentysixteen-style-css][href=https://src.wordpress-develop.test/wp-content/themes/twentysixteen/style.css?ver=4.9.8-beta2-43516-src][type=text/css][media=all] (29839 bytes)
- style#twentysixteen-style-inline-css[id=twentysixteen-style-inline-css][type=text/css] (521 bytes)
- link#jetpack-widget-social-icons-styles-css[rel=stylesheet][id=jetpack-widget-social-icons-styles-css][href=https://src.wordpress-develop.test/wp-content/plugins/jetpack/modules/widgets/social-icons/social-icons.css?ver=20170506][type=text/css][media=all] (205 bytes)
- link#amp-default-css[rel=stylesheet][id=amp-default-css][href=https://src.wordpress-develop.test/wp-content/plugins/amp/assets/css/amp-default.css?ver=1.0-beta1][type=text/css][media=all] (64 bytes)
Total size: 49990 bytes

The following stylesheets are too large to be included in style[amp-custom]:
- style[type=text/css] (74 bytes)
- style#custom-background-css[type=text/css][id=custom-background-css] (45 bytes)
- style[type=text/css][media=print] (39 bytes)
- style[type=text/css][media=screen] (129 bytes)
- style (69 bytes)
- style[type=text/css] (1405 bytes)
- link#jetpack-responsive-videos-style-css[rel=stylesheet][id=jetpack-responsive-videos-style-css][href=https://src.wordpress-develop.test/wp-content/plugins/jetpack/modules/theme-tools/responsive-videos/responsive-videos.css?ver=4.9.8-beta2-43516-src][type=text/css][media=all] (162 bytes)
- link#mediaelement-css[rel=stylesheet][id=mediaelement-css][href=https://src.wordpress-develop.test/wp-includes/js/mediaelement/mediaelementplayer-legacy.min.css?ver=4.2.6-78496d1][type=text/css][media=all] (308 bytes)

After

The style[amp-custom] element is populated with:
     0B: style[amp-custom=]
   338B: link#twentysixteen-jetpack-css[rel=stylesheet][id=twentysixteen-jetpack-css][href=https://src.wordpress-develop.test/wp-content/plugins/jetpack/modules/theme-tools/compat/twentysixteen.css?ver=6.2.1][type=text/css][media=all]
   466B: link#dashicons-css[rel=stylesheet][id=dashicons-css][href=https://src.wordpress-develop.test/wp-includes/css/dashicons.css?ver=4.9.8-beta2-43516-src][type=text/css][media=all]
 17168B: link#admin-bar-css[rel=stylesheet][id=admin-bar-css][href=https://src.wordpress-develop.test/wp-content/plugins/amp/assets/css/admin-bar.css?ver=1.0-beta1][type=text/css][media=all]
   127B: style#admin-bar-inline-css[id=admin-bar-inline-css][type=text/css]
   230B: link#wp-core-blocks-css[rel=stylesheet][id=wp-core-blocks-css][href=https://src.wordpress-develop.test/wp-content/plugins/gutenberg/build/core-blocks/style.css?ver=1533089052][type=text/css][media=all]
  1032B: link#genericons-css[rel=stylesheet][id=genericons-css][href=https://src.wordpress-develop.test/wp-content/plugins/jetpack/_inc/genericons/genericons/genericons.css?ver=3.1][type=text/css][media=all]
 29839B: link#twentysixteen-style-css[rel=stylesheet][id=twentysixteen-style-css][href=https://src.wordpress-develop.test/wp-content/themes/twentysixteen/style.css?ver=4.9.8-beta2-43516-src][type=text/css][media=all]
   521B: style#twentysixteen-style-inline-css[id=twentysixteen-style-inline-css][type=text/css]
   205B: link#jetpack-widget-social-icons-styles-css[rel=stylesheet][id=jetpack-widget-social-icons-styles-css][href=https://src.wordpress-develop.test/wp-content/plugins/jetpack/modules/widgets/social-icons/social-icons.css?ver=20170506][type=text/css][media=all]
    64B: link#amp-default-css[rel=stylesheet][id=amp-default-css][href=https://src.wordpress-develop.test/wp-content/plugins/amp/assets/css/amp-default.css?ver=1.0-beta1][type=text/css][media=all]
Total included size: 49990 bytes

The following stylesheets are too large to be included in style[amp-custom]:
    74B: style[type=text/css]
    45B: style#custom-background-css[type=text/css][id=custom-background-css]
    39B: style[type=text/css][media=print]
   129B: style[type=text/css][media=screen]
    69B: style
  1405B: style[type=text/css]
   162B: link#jetpack-responsive-videos-style-css[rel=stylesheet][id=jetpack-responsive-videos-style-css][href=https://src.wordpress-develop.test/wp-content/plugins/jetpack/modules/theme-tools/responsive-videos/responsive-videos.css?ver=4.9.8-beta2-43516-src][type=text/css][media=all]
   308B: link#mediaelement-css[rel=stylesheet][id=mediaelement-css][href=https://src.wordpress-develop.test/wp-includes/js/mediaelement/mediaelementplayer-legacy.min.css?ver=4.2.6-78496d1][type=text/css][media=all]
Total excluded size: 768 bytes

westonruter added some commits Aug 4, 2018

@westonruter westonruter added this to the v1.0 milestone Aug 4, 2018

@westonruter westonruter requested a review from amedina Aug 4, 2018

@kienstra kienstra self-assigned this Aug 4, 2018

@kienstra

kienstra approved these changes Aug 4, 2018 edited

Approved

Hi @westonruter,
These changes look good.

Like you mentioned, I also see the “Total excluded size” when there is CSS above the 50 KB limit:
total-excluded-size

And the "Total included size" appears at the beginning of the line.
total-size-beginning-line

@kienstra kienstra removed their assignment Aug 4, 2018

@amedina

amedina approved these changes Aug 4, 2018

This is makes it much easier for developers to get the state of their CSS at a glance. Suggested adding a space between the number and the B.

$included_sources = array();
foreach ( $stylesheet_sets['custom']['pending_stylesheets'] as $i => $pending_stylesheet ) {
if ( ! ( $pending_stylesheet['node'] instanceof DOMElement ) ) {
continue;
}
$message = $pending_stylesheet['node']->nodeName;
$message = sprintf( '% 6dB: ', $pending_stylesheet['size'] );

This comment has been minimized.

@amedina

amedina Aug 4, 2018

Member

Could a space between the number and the B be added to make it easier to read?

@westonruter westonruter merged commit 3485b27 into develop Aug 4, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@westonruter westonruter deleted the fix/amp-custom-bytes-formatting branch Aug 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment