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

Immediately add/remove DevTools-related AMP admin menu items when setting updated #6878

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
74 changes: 72 additions & 2 deletions assets/src/settings-page/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
USER_FIELD_REVIEW_PANEL_DISMISSED_FOR_TEMPLATE_MODE,
USERS_RESOURCE_REST_PATH,
VALIDATE_NONCE,
VALIDATED_URLS_LINK,
ERROR_INDEX_LINK,
} from 'amp-settings';

/**
Expand Down Expand Up @@ -141,6 +143,66 @@ function scrollFocusedSectionIntoView( focusedSectionId ) {
}
}

/**
* Get the element for an AMP admin submenu if it exists.
*
* @param {string} href Link href.
* @return {Element|null} LI element or null if not found.
*/
function getAmpAdminMenuItem( href ) {
const parsedUrl = new URL( href );
const searchParams = new Map( new URLSearchParams( parsedUrl.search ).entries() );

links:
for ( const a of document.querySelectorAll( '#toplevel_page_amp-options ul > li > a' ) ) {
if ( ! a.pathname.endsWith( parsedUrl.pathname ) ) {
continue;
}

const linkSearchParams = new URLSearchParams( parsedUrl.search );
for ( const [ key, value ] of searchParams.entries() ) {
if ( linkSearchParams.get( key ) !== value ) {
continue links;
}
}

return a.parentNode;
}
return null;
}

/**
* Add an item to the AMP admin submenu.
*
* @param {string} title Link text.
* @param {string} href Link href.
*/
function addAmpAdminMenuItem( title, href ) {
const ul = document.querySelector( '#toplevel_page_amp-options ul' );
if ( ! ul || getAmpAdminMenuItem( href ) ) {
return;
}

const li = document.createElement( 'li' );
const a = document.createElement( 'a' );
a.innerText = title;
a.href = href;
li.appendChild( a );
ul.appendChild( li );
}

/**
* Remove an item from the AMP admin submenu.
*
* @param {string} href Link href.
*/
function removeAmpAdminMenuItem( href ) {
const li = getAmpAdminMenuItem( href );
if ( li ) {
li.remove();
}
}

/**
* Settings page application root.
*
Expand All @@ -151,7 +213,7 @@ function Root( { appRoot } ) {
const [ focusedSection, setFocusedSection ] = useState( global.location.hash.replace( /^#/, '' ) );

const { hasOptionsChanges, fetchingOptions, saveOptions } = useContext( Options );
const { hasDeveloperToolsOptionChange, saveDeveloperToolsOption } = useContext( User );
const { hasDeveloperToolsOptionChange, saveDeveloperToolsOption, developerToolsOption } = useContext( User );
const { templateModeWasOverridden } = useContext( ReaderThemes );
const { isSkipped } = useContext( SiteScanContext );

Expand All @@ -164,10 +226,18 @@ function Root( { appRoot } ) {
if ( hasOptionsChanges ) {
saveOptions();
}

if ( hasDeveloperToolsOptionChange ) {
saveDeveloperToolsOption();
if ( developerToolsOption ) {
addAmpAdminMenuItem( __( 'Validated URLs', 'amp' ), VALIDATED_URLS_LINK );
addAmpAdminMenuItem( __( 'Error Index', 'amp' ), ERROR_INDEX_LINK );
} else {
removeAmpAdminMenuItem( VALIDATED_URLS_LINK );
removeAmpAdminMenuItem( ERROR_INDEX_LINK );
}
}
}, [ hasDeveloperToolsOptionChange, hasOptionsChanges, saveDeveloperToolsOption, saveOptions ] );
}, [ hasDeveloperToolsOptionChange, hasOptionsChanges, saveDeveloperToolsOption, saveOptions, developerToolsOption ] );

/**
* Scroll to the focused element on load or when it changes.
Expand Down
12 changes: 12 additions & 0 deletions src/Admin/OptionsMenu.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use AMP_Options_Manager;
use AMP_Validated_URL_Post_Type;
use AMP_Validation_Error_Taxonomy;
use AMP_Validation_Manager;
use AmpProject\AmpWP\DependencySupport;
use AmpProject\AmpWP\DevTools\UserAccess;
Expand Down Expand Up @@ -236,6 +237,16 @@ public function enqueue_assets( $hook_suffix ) {
)
);

$amp_error_index_link = admin_url(
add_query_arg(
[
'taxonomy' => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
'post_type' => AMP_Validated_URL_Post_Type::POST_TYPE_SLUG,
],
'edit-tags.php'
)
);

$js_data = [
'AMP_COMPATIBLE_THEMES_URL' => current_user_can( 'install_themes' ) ? admin_url( '/theme-install.php?browse=amp-compatible' ) : 'https://amp-wp.org/ecosystem/themes/',
'AMP_COMPATIBLE_PLUGINS_URL' => current_user_can( 'install_plugins' ) ? admin_url( '/plugin-install.php?tab=amp-compatible' ) : 'https://amp-wp.org/ecosystem/plugins/',
Expand All @@ -260,6 +271,7 @@ public function enqueue_assets( $hook_suffix ) {
'USERS_RESOURCE_REST_PATH' => '/wp/v2/users',
'VALIDATE_NONCE' => AMP_Validation_Manager::has_cap() ? AMP_Validation_Manager::get_amp_validate_nonce() : '',
'VALIDATED_URLS_LINK' => $amp_validated_urls_link,
'ERROR_INDEX_LINK' => $amp_error_index_link,
'HAS_PAGE_CACHING' => ( is_array( $page_cache_detail ) && 'good' === $page_cache_detail['status'] ),
];

Expand Down
45 changes: 45 additions & 0 deletions tests/e2e/specs/admin/anchor-linking.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { loginUser } from '@wordpress/e2e-test-utils';
* Internal dependencies
*/
import { visitAdminPageWithHash } from '../../utils/visit-admin-page-with-hash';
import { cleanUpSettings, scrollToElement } from '../../utils/onboarding-wizard-utils';
import { saveSettings } from '../../utils/amp-settings-utils';

describe( 'AMP settings page anchor linking', () => {
beforeEach( async () => {
Expand All @@ -30,3 +32,46 @@ describe( 'AMP settings page anchor linking', () => {
} );
} );

describe( 'AMP developer tools settings', () => {
beforeEach( async () => {
await loginUser();
await visitAdminPageWithHash( 'admin.php', 'page=amp-options', 'other-settings' );
} );

afterEach( async () => {
await cleanUpSettings();
} );

it( 'enables developer tools', async ( ) => {
const fullSelector = `#other-settings .developer-tools .amp-setting-toggle input[type="checkbox"]`;

// Confirm the setting is initially enabled.
await expect( page ).toMatchElement( `${ fullSelector }:checked` );

const links = [ 'edit.php?post_type=amp_validated_url', 'edit-tags.php?taxonomy=amp_validation_error&post_type=amp_validated_url' ];

await Promise.all( links.map( ( link ) => {
return expect( page ).toMatchElement( `a[href$="${ link }"]` );
} ) );
} );

it( 'disables developer tools', async ( ) => {
const fullSelector = `#other-settings .developer-tools .amp-setting-toggle input[type="checkbox"]`;

// Confirm the setting is initially enabled.
await expect( page ).toMatchElement( `${ fullSelector }:checked` );

// Disable it
await scrollToElement( { selector: fullSelector, click: true } );
await expect( page ).toMatchElement( `${ fullSelector }:not(:checked)` );

const links = [ 'edit.php?post_type=amp_validated_url', 'edit-tags.php?taxonomy=amp_validation_error&post_type=amp_validated_url' ];

await saveSettings();

// Check after save.
await Promise.all( links.map( ( link ) => {
return expect( page ).not.toMatchElement( `a[href$="${ link }"]` );
} ) );
} );
} );