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

Support late-defined slugs for paired URL structures and Reader themes #6125

Merged
merged 12 commits into from
Apr 28, 2021
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
25 changes: 1 addition & 24 deletions assets/src/components/reader-theme-carousel/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { AMP_QUERY_VAR, DEFAULT_AMP_QUERY_VAR, AMP_QUERY_VAR_CUSTOMIZED_LATE } from 'amp-settings'; // From WP inline script.

/**
* WordPress dependencies
*/
Expand Down Expand Up @@ -155,25 +150,7 @@ export function ReaderThemeCarousel() {
hasUnavailableThemes && (
<AMPNotice type={ NOTICE_TYPE_INFO }>
<p>
{ AMP_QUERY_VAR_CUSTOMIZED_LATE
/* dangerouslySetInnerHTML reason: Injection of code tags. */
? (
<span
dangerouslySetInnerHTML={ {
__html: sprintf(
/* translators: 1: customized AMP query var, 2: default query var, 3: the AMP_QUERY_VAR constant name, 4: the amp_query_var filter, 5: the plugins_loaded action */
__( 'Some of the themes below are not available because your site (probably the active theme) has customized the AMP query var too late (it is set to %1$s as opposed to the default of %2$s). Please make sure that any customizations done by defining the %3$s constant or adding an %4$s filter are done before the %5$s action with priority 8.', 'amp' ),
`<code>${ AMP_QUERY_VAR }</code>`,
`<code>${ DEFAULT_AMP_QUERY_VAR }</code>`,
'<code>AMP_QUERY_VAR</code>',
'<code>amp_query_var</code>',
'<code>plugins_loaded</code>',
),
} }
/>
)
: __( 'Some supported themes cannot be installed automatically on this site. To use, please install them manually or contact your hosting provider.', 'amp' )
}
{ __( 'Some supported themes cannot be installed automatically on this site. To use, please install them manually or contact your hosting provider.', 'amp' ) }
</p>
<AMPSettingToggle
text={ __( 'Show unavailable themes', 'amp' ) }
Expand Down
23 changes: 2 additions & 21 deletions assets/src/components/reader-theme-selection/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@
* External dependencies
*/
import PropTypes from 'prop-types';
import { AMP_QUERY_VAR, DEFAULT_AMP_QUERY_VAR, AMP_QUERY_VAR_CUSTOMIZED_LATE } from 'amp-settings'; // From WP inline script.

/**
* WordPress dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { __ } from '@wordpress/i18n';
import { useContext } from '@wordpress/element';

/**
Expand Down Expand Up @@ -56,25 +55,7 @@ export function ReaderThemeSelection() {
{ __( 'Unavailable themes', 'amp' ) }
</h3>
<p>
{ AMP_QUERY_VAR_CUSTOMIZED_LATE
/* dangerouslySetInnerHTML reason: Injection of code tags. */
? (
<span
dangerouslySetInnerHTML={ {
__html: sprintf(
/* translators: 1: customized AMP query var, 2: default query var, 3: the AMP_QUERY_VAR constant name, 4: the amp_query_var filter, 5: the plugins_loaded action */
__( 'The following themes are not available because your site (probably the active theme) has customized the AMP query var too late (it is set to %1$s as opposed to the default of %2$s). Please make sure that any customizations done by defining the %3$s constant or adding an %4$s filter are done before the %5$s action with priority 8.', 'amp' ),
`<code>${ AMP_QUERY_VAR }</code>`,
`<code>${ DEFAULT_AMP_QUERY_VAR }</code>`,
'<code>AMP_QUERY_VAR</code>',
'<code>amp_query_var</code>',
'<code>plugins_loaded</code>',
),
} }
/>
)
: __( 'The following themes are compatible but cannot be installed automatically. Please install them manually, or contact your host if you are not able to do so.', 'amp' )
}
{ __( 'The following themes are compatible but cannot be installed automatically. Please install them manually, or contact your host if you are not able to do so.', 'amp' ) }
</p>
<ul className="choose-reader-theme__grid">
{ unavailableThemes.map( ( theme ) => (
Expand Down
4 changes: 2 additions & 2 deletions assets/src/components/reader-themes-context-provider/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { __ } from '@wordpress/i18n';
* External dependencies
*/
import PropTypes from 'prop-types';
import { AMP_QUERY_VAR_CUSTOMIZED_LATE, USING_FALLBACK_READER_THEME, LEGACY_THEME_SLUG } from 'amp-settings';
import { USING_FALLBACK_READER_THEME, LEGACY_THEME_SLUG } from 'amp-settings';

/**
* Internal dependencies
Expand Down Expand Up @@ -282,7 +282,7 @@ export function ReaderThemesContextProvider( { wpAjaxUrl, children, currentTheme
const { availableThemes, unavailableThemes } = useMemo(
() => ( filteredThemes || [] ).reduce(
( collections, theme ) => {
if ( ( AMP_QUERY_VAR_CUSTOMIZED_LATE && theme.slug !== LEGACY_THEME_SLUG ) || theme.availability === 'non-installable' ) {
if ( theme.availability === 'non-installable' ) {
collections.unavailableThemes.push( theme );
} else {
collections.availableThemes.push( theme );
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { LEGACY_THEME_SLUG, AMP_QUERY_VAR_CUSTOMIZED_LATE } from 'amp-settings'; // From WP inline script.

/**
* WordPress dependencies
*/
Expand Down Expand Up @@ -35,9 +30,7 @@ export function ChooseReaderTheme() {
themes &&
readerTheme &&
canGoForward === false &&
! AMP_QUERY_VAR_CUSTOMIZED_LATE
? themes.map( ( { slug } ) => slug ).includes( readerTheme )
: readerTheme === LEGACY_THEME_SLUG
themes.map( ( { slug } ) => slug ).includes( readerTheme )
) {
setCanGoForward( true );
}
Expand Down
16 changes: 15 additions & 1 deletion includes/amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

use AmpProject\AmpWP\Admin\ReaderThemes;
use AmpProject\AmpWP\AmpSlugCustomizationWatcher;
use AmpProject\AmpWP\AmpWpPluginFactory;
use AmpProject\AmpWP\Exception\InvalidService;
use AmpProject\AmpWP\Icon;
Expand Down Expand Up @@ -531,10 +532,23 @@ function _amp_bootstrap_customizer() {
* may be deprecated in the future. Normally the slug should be just 'amp'.
*
* @since 0.7
* @since 2.1 Added $ignore_late_defined_slug argument.
*
* @param bool $ignore_late_defined_slug Whether to ignore the late defined slug.
* @return string Slug used for query var, endpoint, and post type support.
*/
function amp_get_slug() {
function amp_get_slug( $ignore_late_defined_slug = false ) {

// When a slug was defined late according to AmpSlugCustomizationWatcher, the slug will be stored in the
// LATE_DEFINED_SLUG option by the PairedRouting service so that it can be used early. This is only needed until
// the after_setup_theme action fires, because at that time the late-defined slug will have been established.
if ( ! $ignore_late_defined_slug && ! did_action( AmpSlugCustomizationWatcher::LATE_DETERMINATION_ACTION ) ) {
$slug = AMP_Options_Manager::get_option( Option::LATE_DEFINED_SLUG );
if ( ! empty( $slug ) && is_string( $slug ) ) {
return $slug;
}
}

/**
* Filter the AMP query variable.
*
Expand Down
1 change: 1 addition & 0 deletions includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,7 @@ public static function add_hooks() {
if ( has_action( 'wp_head', 'print_emoji_detection_script' ) ) {
remove_action( 'wp_head', 'print_emoji_detection_script', 7 );
remove_action( 'wp_print_styles', 'print_emoji_styles' );
remove_action( 'wp_footer', 'gutenberg_the_skip_link' ); // Temporary workaround for <https://github.com/ampproject/amp-wp/issues/6115>.
add_action( 'wp_print_styles', [ __CLASS__, 'print_emoji_styles' ] );
add_filter( 'the_title', 'wp_staticize_emoji' );
add_filter( 'the_excerpt', 'wp_staticize_emoji' );
Expand Down
22 changes: 0 additions & 22 deletions includes/options/class-amp-options-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,28 +60,6 @@ public static function register_settings() {
'sanitize_callback' => [ __CLASS__, 'validate_options' ],
]
);

add_action( 'update_option_' . self::OPTION_NAME, [ __CLASS__, 'maybe_flush_rewrite_rules' ], 10, 2 );
}

/**
* Flush rewrite rules if the supported_post_types have changed.
*
* @since 0.6.2
*
* @param array $old_options Old options.
* @param array $new_options New options.
*/
public static function maybe_flush_rewrite_rules( $old_options, $new_options ) {
$old_post_types = isset( $old_options[ Option::SUPPORTED_POST_TYPES ] ) ? $old_options[ Option::SUPPORTED_POST_TYPES ] : [];
$new_post_types = isset( $new_options[ Option::SUPPORTED_POST_TYPES ] ) ? $new_options[ Option::SUPPORTED_POST_TYPES ] : [];
sort( $old_post_types );
sort( $new_post_types );
if ( $old_post_types !== $new_post_types ) {
// Flush rewrite rules.
add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );
flush_rewrite_rules( false );
}
}

/**
Expand Down
2 changes: 0 additions & 2 deletions src/Admin/OnboardingWizardSubmenuPage.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,6 @@ public function enqueue_assets( $hook_suffix ) {
$setup_wizard_data = [
'AMP_OPTIONS_KEY' => AMP_Options_Manager::OPTION_NAME,
'AMP_QUERY_VAR' => amp_get_slug(),
'DEFAULT_AMP_QUERY_VAR' => QueryVar::AMP,
'AMP_QUERY_VAR_CUSTOMIZED_LATE' => $amp_slug_customization_watcher->did_customize_late(),
'LEGACY_THEME_SLUG' => ReaderThemes::DEFAULT_READER_THEME,
'APP_ROOT_ID' => self::APP_ROOT_ID,
'CUSTOMIZER_LINK' => add_query_arg(
Expand Down
22 changes: 10 additions & 12 deletions src/Admin/OptionsMenu.php
Original file line number Diff line number Diff line change
Expand Up @@ -214,28 +214,26 @@ public function enqueue_assets( $hook_suffix ) {
$is_reader_theme = $this->reader_themes->theme_data_exists( get_stylesheet() );

$js_data = [
'AMP_QUERY_VAR' => amp_get_slug(),
'DEFAULT_AMP_QUERY_VAR' => QueryVar::AMP,
'AMP_QUERY_VAR_CUSTOMIZED_LATE' => $amp_slug_customization_watcher->did_customize_late(),
'CURRENT_THEME' => [
'AMP_QUERY_VAR' => amp_get_slug(),
'CURRENT_THEME' => [
'name' => $theme->get( 'Name' ),
'description' => $theme->get( 'Description' ),
'is_reader_theme' => $is_reader_theme,
'screenshot' => $theme->get_screenshot() ?: null,
'url' => $theme->get( 'ThemeURI' ),
],
'OPTIONS_REST_PATH' => '/amp/v1/options',
'READER_THEMES_REST_PATH' => '/amp/v1/reader-themes',
'IS_CORE_THEME' => in_array(
'OPTIONS_REST_PATH' => '/amp/v1/options',
'READER_THEMES_REST_PATH' => '/amp/v1/reader-themes',
'IS_CORE_THEME' => in_array(
get_stylesheet(),
AMP_Core_Theme_Sanitizer::get_supported_themes(),
true
),
'LEGACY_THEME_SLUG' => ReaderThemes::DEFAULT_READER_THEME,
'USING_FALLBACK_READER_THEME' => $this->reader_themes->using_fallback_theme(),
'THEME_SUPPORT_ARGS' => AMP_Theme_Support::get_theme_support_args(),
'THEME_SUPPORTS_READER_MODE' => AMP_Theme_Support::supports_reader_mode(),
'UPDATES_NONCE' => wp_create_nonce( 'updates' ),
'LEGACY_THEME_SLUG' => ReaderThemes::DEFAULT_READER_THEME,
'USING_FALLBACK_READER_THEME' => $this->reader_themes->using_fallback_theme(),
'THEME_SUPPORT_ARGS' => AMP_Theme_Support::get_theme_support_args(),
'THEME_SUPPORTS_READER_MODE' => AMP_Theme_Support::supports_reader_mode(),
'UPDATES_NONCE' => wp_create_nonce( 'updates' ),
];

wp_add_inline_script(
Expand Down
Loading