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

Show notice on frontend when there is excessive CSS #1801

Closed
westonruter opened this issue Jan 8, 2019 · 7 comments · Fixed by #4026
Closed

Show notice on frontend when there is excessive CSS #1801

westonruter opened this issue Jan 8, 2019 · 7 comments · Fixed by #4026
Labels
CSS QA passed Has passed QA and is done
Milestone

Comments

@westonruter
Copy link
Member

westonruter commented Jan 8, 2019

In addition to automatically disabling the admin bar when there is excessive CSS (#1800), there should be an additional message displayed to the logged-in site administrator on the frontend when there is still excessive CSS. At the moment the ways for a user to know that there is excessive CSS are:

  • Notice broken styling.
  • See an AMP validation error when saving a post.
  • Look at the page source for the amp-custom style manifest to see the excessive CSS message.

In addition to these, the plugin could display a message about the excessive CSS along with some suggestions for how to fix. For developers, fixing may involve rewriting CSS. For implementors, fixing may involving switching to a different theme or using alternative plugins. For other users, the fix may be to switch back to Classic/Legacy mode until the ecosystem better supports AMP.

The admin bar would be a good location to show this message:

image

@westonruter
Copy link
Member Author

westonruter commented Sep 21, 2019

It would also be useful to show a <meter> element indicating the percentage of how much of the CSS budget they've used on the current page. Clicking the progress bar can then take the user to view the CSS manifest in the Validated URL screen (#2169).

@pierlon pierlon self-assigned this Oct 15, 2019
@pierlon
Copy link
Contributor

pierlon commented Oct 16, 2019

Would the progress meter be a top-level menu item in the admin bar?

@schlessera
Copy link
Collaborator

In my opinion, it should be a submenu under the main AMP top-level entry. Otherwise, we'll take up too much space to show a metric that will often be irrelevant.

@westonruter
Copy link
Member Author

Agreed. It should be a submenu item of the AMP admin bar.

I think this issue should be put on hold for now because it is closely related to #2169, and I think some more pieces need to be put in place before we can move forward with it.

@westonruter westonruter assigned westonruter and unassigned pierlon Oct 16, 2019
@swissspidy swissspidy modified the milestones: v1.3.1, v1.4 Oct 17, 2019
@westonruter westonruter removed this from the v1.4 milestone Oct 17, 2019
@westonruter westonruter removed their assignment Oct 24, 2019
@westonruter westonruter self-assigned this Jan 3, 2020
@westonruter westonruter added this to the v1.5 milestone Jan 3, 2020
@westonruter
Copy link
Member Author

It would also be useful to show a <meter> element indicating the percentage of how much of the CSS budget they've used on the current page. Clicking the progress bar can then take the user to view the CSS manifest in the Validated URL screen (#2169).

I think optimum="50" low="75" high="99" attributes work well for meter works:

<meter min="0" optimum="50" low="75" high="100" max="101" value="50">50%</meter>
<meter min="0" optimum="50" low="75" high="100" max="101" value="80">80%</meter>
<meter min="0" optimum="50" low="75" high="100" max="101" value="101">100%</meter>

image

  • If the CSS on the page is using 75% or less of the 50KB budget used, then the color is green.
  • If more than 75% is used, then the color is yellow.
  • If more than 100% is used, then the color is red.

@westonruter
Copy link
Member Author

While I like the idea of the meter element:

image

In practice I think just plain text will be more clear:

image

When using 80% or more:

image

vs.

image

And when using more than 100%:

image

vs.

image

There's probably some way to combine the meter with the text label, but then that would seem to just be getting verbose.

Code for meter if we decide to revisit:

$meter_element = $this->dom->createElement( 'meter' );
$meter_element->setAttribute( 'min', '0' );
$meter_element->setAttribute( 'optimum', $this->style_custom_cdata_spec['max_bytes'] * 0.5 );
$meter_element->setAttribute( 'low', $this->style_custom_cdata_spec['max_bytes'] * 0.75 );
$meter_element->setAttribute( 'high', $this->style_custom_cdata_spec['max_bytes'] );
$meter_element->setAttribute( 'max', $this->style_custom_cdata_spec['max_bytes'] + 1 );
$meter_element->setAttribute( 'value', $total_size );
$meter_element->setAttribute( 'style', 'height:1em; width:100%;' );

@csossi
Copy link

csossi commented Jan 31, 2020

Verified in QA

@csossi csossi added the QA passed Has passed QA and is done label Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS QA passed Has passed QA and is done
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants