Restore admin bar on AMP pages and improve AMP menu items #1219
Conversation
* Add forked version of WP 4.9.6's admin-bar.css with fixes for AMP. * Fix positioning of admin bar with sticky nav in Twenty Seventeen.
There was a problem hiding this comment.
Approved
Hi @westonruter,
It's great to see the admin bar on AMP URLs! This is a big step in usability.
|
|
||
| $i = array_search( $ruleset, $css_list->getContents(), true ); | ||
| if ( false !== $i ) { | ||
| $css_list->splice( $i + 1, 0, array( $important_ruleset ) ); |
There was a problem hiding this comment.
There was originally an error from this line, but running composer install took care of it:
( ! ) Error: Call to undefined method Sabberworm\CSS\CSSList\Document::splice() in /srv/www/loc/public_html/wp-content/plugins/amp/includes/sanitizers/class-amp-style-sanitizer.php on line 1622
There was a problem hiding this comment.
Yes, that is expected 👍
| </label> | ||
| </p> | ||
| <p class="description"> | ||
| <?php esc_html_e( 'An additional stylesheet is required to properly render the admin bar. If the additional stylesheet causes the total CSS to surpass 50KB then the admin bar should be disabled to prevent a validation error or an unstyled admin bar in AMP responses .', 'amp' ); ?> |
There was a problem hiding this comment.
This isn't a blocker. But it looks like there's an extra space after 'responses':
in AMP responses .', 'amp' );
|
|
||
| <p> | ||
| <label for="disable_admin_bar"> | ||
| <input id="disable_admin_bar" type="checkbox" name="<?php echo esc_attr( AMP_Options_Manager::OPTION_NAME . '[disable_admin_bar]' ); ?>" <?php checked( AMP_Options_Manager::get_option( 'disable_admin_bar' ) ); ?>> |
There was a problem hiding this comment.
It's great to have the admin bar enabled by default, with this checkbox to disable it.
Admins might be willing to ignore the AMP validation error of CSS over 50KB in order to have the admin bar. Of course, that error wouldn't even appear for non logged-in users
* Add setting for turning off admin bar * Link between AMP and non-AMP versions when on non-native. * Show actual validation errors when viewing admin bar AMP. * Show warning emoji when there are sanitized unaccepted validation errors.
|
This is great, @westonruter! Further brings parity to the AMP experience: My preferred way to edit a post is, on the front end as a logged-in user, to find the page and use the |
admin-bar.cssfrom core to utilize:focus-withininstead of relying on jQuery to togglehoverclasses.mediaattribute onstyleelements.!importanttransformations by inserting higher-specificity rules adjacent to their original rule instead of appending to the very end of the stylesheet.New admin setting:
Link to AMP version from non-AMP page in paired mode:
Admin bar item that appears after having been redirected from AMP to non-AMP version due to non-sanitized validation errors:
Admin bar item that appears on AMP page when there is an unaccepted validation error that has been forcibly sanitized:
Admin bar item in native mode when there are unaccepted validation errors:
Fixes #1205