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

Integrate admin bar with AMP dev mode #3187

Merged
merged 23 commits into from
Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
3380451
Revert "Suppress admin bar from being shown during validation requests"
westonruter Sep 4, 2019
e3d4f57
Skip sanitizing admin bar (and removing it) in favor of ignoring (in …
westonruter Sep 5, 2019
8640d1a
Prevent tree-shaking under #wpadminbar when admin bar is showing
westonruter Sep 6, 2019
f75586f
Prevent admin bar from even attempting to be displayed on story template
westonruter Sep 6, 2019
a81fb73
Remove redundant document argument from is_document_in_dev_mode()
westonruter Sep 6, 2019
bfad786
Skip converting elements in dev mode; skip processing style attribute…
westonruter Sep 6, 2019
5bb7466
Add filters for whether dev mode is enabled and what elements get it
westonruter Sep 6, 2019
d615d39
Bring back remove_admin_bar_if_css_excluded and admin-bar CSS priorit…
westonruter Sep 7, 2019
277131f
Prevent dev mode when admin bar shown for unauthenticated users
westonruter Sep 7, 2019
4b533d2
Fix placement of dev_mode_enabled variable definition
westonruter Sep 7, 2019
e3025cd
Fix undefined dev_mode_xpaths being used in cache key
westonruter Sep 8, 2019
a726d41
Fix typos in comments and add array casting
westonruter Sep 9, 2019
58f5eb3
Merge branch 'develop' of github.com:ampproject/amp-wp into add/dev-m…
westonruter Sep 10, 2019
d1ac164
Move dev mode mutations into new sanitizer
westonruter Sep 10, 2019
5ba10b4
Remove now-unnecessary admin_bar_showing arg for style sanitizer
westonruter Sep 10, 2019
38fa7ba
Add tests for init_admin_bar
westonruter Sep 10, 2019
71c195c
Add test for amp_is_dev_mode()
westonruter Sep 10, 2019
2471036
Add test for amp_get_content_sanitizers()
westonruter Sep 10, 2019
7c57708
Add tests for AMP_Dev_Mode_Sanitizer
westonruter Sep 10, 2019
e15bbe3
Include dashicons in dev mode but only if exclusively connected to ad…
westonruter Sep 10, 2019
d79be8d
Remove amp_dev_mode_enabled filter in test
westonruter Sep 10, 2019
5de22ff
Remove useless variable
schlessera Sep 11, 2019
290adf7
Add asserts to make sure we are not enqueueing both versions of dashi…
schlessera Sep 11, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -1422,7 +1422,31 @@ protected static function has_dependency( WP_Dependencies $dependencies, $curren
* @return string Tag.
*/
public static function filter_admin_bar_style_loader_tag( $tag, $handle ) {
if ( self::has_dependency( wp_styles(), $handle, 'admin-bar' ) ) {
if ( 'dashicons' === $handle ) {
// Conditionally include Dashicons in dev mode only if was included because it is a dependency of admin-bar.
$needs_dev_mode = true;
foreach ( wp_styles()->queue as $queued_handle ) {
$is_dashicons_without_admin_bar = (
schlessera marked this conversation as resolved.
Show resolved Hide resolved
// If a theme or plugin directly enqueued dashicons, then it is not added via admin-bar dependency and it is not part of dev mode.
'dashicons' === $queued_handle
||
// If a stylesheet has dashicons as a dependency without also having admin-bar as a dependency, then no dev mode.
(
self::has_dependency( wp_styles(), $queued_handle, 'dashicons' )
&&
! self::has_dependency( wp_styles(), $queued_handle, 'admin-bar' )
)
);
if ( $is_dashicons_without_admin_bar ) {
$needs_dev_mode = false;
break;
}
}
} else {
$needs_dev_mode = self::has_dependency( wp_styles(), $handle, 'admin-bar' );
}

if ( $needs_dev_mode ) {
$tag = preg_replace( '/(?<=<link)(?=\s|>)/i', ' ' . AMP_Rule_Spec::DEV_MODE_ATTRIBUTE, $tag );
}
return $tag;
Expand Down
54 changes: 52 additions & 2 deletions tests/php/test-class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -1188,12 +1188,15 @@ function() {
$this->assertEquals( 10, has_action( 'wp_head', $callback ) );

AMP_Theme_Support::init_admin_bar();
$this->assertEquals( 10, has_filter( 'style_loader_tag', [ 'AMP_Theme_Support', 'filter_admin_bar_style_loader_tag' ] ) );
$this->assertEquals( 10, has_filter( 'script_loader_tag', [ 'AMP_Theme_Support', 'filter_admin_bar_script_loader_tag' ] ) );
$this->assertFalse( has_action( 'wp_head', $callback ) );
ob_start();
wp_print_styles();
wp_print_scripts();
$output = ob_get_clean();
$this->assertContains( '<style id=\'admin-bar-inline-css\' type=\'text/css\'>', $output ); // Note: data-ampdevmode attribute will be added by AMP_Dev_Mode_Sanitizer.
$this->assertContains( '<link data-ampdevmode rel=\'stylesheet\' id=\'dashicons-css\'', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
$this->assertContains( '<link data-ampdevmode rel=\'stylesheet\' id=\'admin-bar-css\'', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
$this->assertContains( '<link data-ampdevmode rel=\'stylesheet\' id=\'example-admin-bar-css\'', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
$this->assertContains( 'html { margin-top: 64px !important; }', $output );
Expand All @@ -1203,9 +1206,56 @@ function() {
$body_classes = get_body_class();
$this->assertContains( 'customize-support', $body_classes );
$this->assertNotContains( 'no-customize-support', $body_classes );
}

$this->assertEquals( 10, has_filter( 'style_loader_tag', [ 'AMP_Theme_Support', 'filter_admin_bar_style_loader_tag' ] ) );
$this->assertEquals( 10, has_filter( 'script_loader_tag', [ 'AMP_Theme_Support', 'filter_admin_bar_script_loader_tag' ] ) );
/**
* Test init_admin_bar to ensure dashicons are not added to dev mode when directly enqueued.
*
* @covers \AMP_Theme_Support::init_admin_bar()
* @covers \AMP_Theme_Support::filter_admin_bar_style_loader_tag()
*/
public function test_init_admin_bar_for_directly_enqueued_dashicons() {
require_once ABSPATH . WPINC . '/class-wp-admin-bar.php';

global $wp_admin_bar;
$wp_admin_bar = new WP_Admin_Bar();
$wp_admin_bar->initialize();
AMP_Theme_Support::init_admin_bar();

// Enqueued directly.
wp_enqueue_style( 'dashicons' );

ob_start();
wp_print_styles();
$output = ob_get_clean();

$this->assertContains( '<link rel=\'stylesheet\' id=\'dashicons-css\'', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
$this->assertContains( '<link data-ampdevmode rel=\'stylesheet\' id=\'admin-bar-css\'', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
schlessera marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Test init_admin_bar to ensure dashicons are not added to dev mode when indirectly enqueued.
*
* @covers \AMP_Theme_Support::init_admin_bar()
* @covers \AMP_Theme_Support::filter_admin_bar_style_loader_tag()
*/
public function test_init_admin_bar_for_indirectly_enqueued_dashicons() {
require_once ABSPATH . WPINC . '/class-wp-admin-bar.php';

global $wp_admin_bar;
$wp_admin_bar = new WP_Admin_Bar();
$wp_admin_bar->initialize();
AMP_Theme_Support::init_admin_bar();

// Enqueued indirectly.
wp_enqueue_style( 'my-font-pack', 'https://example.com/fonts', [ 'dashicons' ], '0.1' );

ob_start();
wp_print_styles();
$output = ob_get_clean();

$this->assertContains( '<link rel=\'stylesheet\' id=\'dashicons-css\'', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
schlessera marked this conversation as resolved.
Show resolved Hide resolved
$this->assertContains( '<link data-ampdevmode rel=\'stylesheet\' id=\'admin-bar-css\'', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
}

/**
Expand Down