Skip to content

Commit

Permalink
Merge pull request #2622 from ampproject/update/reader-mode-availability
Browse files Browse the repository at this point in the history
Always allow switching to Reader mode, but default to mode defined by theme
  • Loading branch information
westonruter authored Jun 14, 2019
2 parents 5727a52 + 68f8c9a commit ba069d4
Show file tree
Hide file tree
Showing 10 changed files with 201 additions and 120 deletions.
32 changes: 20 additions & 12 deletions includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ class AMP_Theme_Support {
*/
const TRANSITIONAL_MODE_SLUG = 'transitional';

/**
* Slug identifying reader website mode.
*
* @since 1.2
* @var string
*/
const READER_MODE_SLUG = 'reader';

/**
* Flag used in args passed to add_theme_support('amp') to indicate transitional mode supported.
*
Expand Down Expand Up @@ -248,7 +256,7 @@ public static function get_support_mode() {
$theme_support = self::get_support_mode_added_via_theme();
}
if ( ! $theme_support ) {
$theme_support = 'disabled';
$theme_support = self::READER_MODE_SLUG;
}
return $theme_support;
}
Expand Down Expand Up @@ -303,20 +311,20 @@ public static function read_theme_support() {

$is_paired = ! empty( $args[ self::PAIRED_FLAG ] );

self::$support_added_via_theme = $is_paired ? self::TRANSITIONAL_MODE_SLUG : self::STANDARD_MODE_SLUG;
self::$support_added_via_theme = $is_paired ? self::TRANSITIONAL_MODE_SLUG : self::STANDARD_MODE_SLUG;
self::$support_added_via_option = $theme_support_option;

/*
* If the theme has transitional support, allow the user to opt for AMP-first mode via an option, since a theme
* in transitional mode entails that it supports serving templates as both AMP and non-AMP, and this it is
* able to serve AMP-first pages just as well as paired pages. Otherwise, consider that the the mode was
* not set at all via option.
*/
self::$support_added_via_option = ( $is_paired && self::STANDARD_MODE_SLUG === $theme_support_option ) ? self::STANDARD_MODE_SLUG : null;
if ( self::STANDARD_MODE_SLUG === self::$support_added_via_option ) {
// Make sure the user option can override what the theme has specified.
if ( $is_paired && self::STANDARD_MODE_SLUG === $theme_support_option ) {
$args[ self::PAIRED_FLAG ] = false;
add_theme_support( 'amp', $args );
add_theme_support( self::SLUG, $args );
} elseif ( ! $is_paired && self::TRANSITIONAL_MODE_SLUG === $theme_support_option ) {
$args[ self::PAIRED_FLAG ] = true;
add_theme_support( self::SLUG, $args );
} elseif ( self::READER_MODE_SLUG === $theme_support_option ) {
remove_theme_support( self::SLUG );
}
} elseif ( 'disabled' !== $theme_support_option ) {
} elseif ( self::READER_MODE_SLUG !== $theme_support_option ) {
$is_paired = ( self::TRANSITIONAL_MODE_SLUG === $theme_support_option );
add_theme_support(
self::SLUG,
Expand Down
33 changes: 27 additions & 6 deletions includes/options/class-amp-options-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class AMP_Options_Manager {
*/
protected static $defaults = array(
'experiences' => array( self::WEBSITE_EXPERIENCE ),
'theme_support' => 'disabled',
'theme_support' => AMP_Theme_Support::READER_MODE_SLUG,
'supported_post_types' => array( 'post' ),
'analytics' => array(),
'auto_accept_sanitization' => true,
Expand Down Expand Up @@ -104,9 +104,17 @@ public static function get_options() {
if ( empty( $options ) ) {
$options = array(); // Ensure empty string becomes array.
}
self::$defaults['enable_response_caching'] = wp_using_ext_object_cache();

$options = array_merge( self::$defaults, $options );
$defaults = self::$defaults;

$defaults['enable_response_caching'] = wp_using_ext_object_cache();

$args = AMP_Theme_Support::get_theme_support_args();
if ( false !== $args ) {
$defaults['theme_support'] = empty( $args[ AMP_Theme_Support::PAIRED_FLAG ] ) ? AMP_Theme_Support::STANDARD_MODE_SLUG : AMP_Theme_Support::TRANSITIONAL_MODE_SLUG;
}

$options = array_merge( $defaults, $options );

// Migrate stories option from 1.2-beta.
if ( ! empty( $options['enable_amp_stories'] ) ) {
Expand All @@ -119,6 +127,19 @@ public static function get_options() {
$options['theme_support'] = AMP_Theme_Support::STANDARD_MODE_SLUG;
} elseif ( 'paired' === $options['theme_support'] ) {
$options['theme_support'] = AMP_Theme_Support::TRANSITIONAL_MODE_SLUG;
} elseif ( 'disabled' === $options['theme_support'] ) {
/*
* Prior to 1.2, the theme support slug for Reader mode was 'disabled'. This would be saved in options for
* themes that had 'amp' theme support defined. Also prior to 1.2, the user could not switch between modes
* when the theme had 'amp' theme support. The result is that a site running 1.1 could be AMP-first and then
* upon upgrading to 1.2, be switched to Reader mode. So when migrating the old 'disabled' slug to the new
* value, we need to make sure we use the default theme support slug as it has been determined above. If the
* site has non-paired 'amp' theme support and the theme support slug is 'disabled' then it should here be
* set to 'standard' as opposed to 'reader', and the same goes for paired 'amp' theme support, as it should
* become 'transitional'. Otherwise, if the theme lacks 'amp' theme support, then this will become the
* default 'reader' mode.
*/
$options['theme_support'] = $defaults['theme_support'];
}

return $options;
Expand Down Expand Up @@ -201,7 +222,7 @@ public static function validate_options( $new_options ) {

// Theme support.
$recognized_theme_supports = array(
'disabled',
AMP_Theme_Support::READER_MODE_SLUG,
AMP_Theme_Support::TRANSITIONAL_MODE_SLUG,
AMP_Theme_Support::STANDARD_MODE_SLUG,
);
Expand Down Expand Up @@ -319,7 +340,7 @@ public static function check_supported_post_type_update_errors() {
}

// If all templates are supported then skip check since all post types are also supported. This option only applies with standard/transitional theme support.
if ( self::get_option( 'all_templates_supported', false ) && 'disabled' !== self::get_option( 'theme_support' ) ) {
if ( self::get_option( 'all_templates_supported', false ) && AMP_Theme_Support::READER_MODE_SLUG !== self::get_option( 'theme_support' ) ) {
return;
}

Expand Down Expand Up @@ -732,7 +753,7 @@ public static function handle_updated_theme_support_option() {
$message .= ' ' . join( ' ', $review_messages );
}
break;
case 'disabled':
case AMP_Theme_Support::READER_MODE_SLUG:
$message = wp_kses_post(
sprintf(
/* translators: %s is an AMP URL */
Expand Down
178 changes: 88 additions & 90 deletions includes/options/class-amp-options-menu.php
Original file line number Diff line number Diff line change
Expand Up @@ -289,101 +289,95 @@ public function render_theme_support() {

$builtin_support = in_array( get_template(), AMP_Core_Theme_Sanitizer::get_supported_themes(), true );
?>
<?php if ( AMP_Theme_Support::STANDARD_MODE_SLUG === AMP_Theme_Support::get_support_mode_added_via_theme() ) : ?>
<div class="notice notice-info notice-alt inline">
<p><?php esc_html_e( 'Your active theme has built-in AMP support.', 'amp' ); ?></p>
</div>
<p>
<?php echo wp_kses_post( $standard_description ); ?>
</p>
<p>
<?php echo wp_kses_post( $ecosystem_description ); ?>
</p>
<?php else : ?>
<fieldset <?php disabled( ! current_user_can( 'manage_options' ) ); ?>>
<?php if ( $builtin_support ) : ?>

<fieldset <?php disabled( ! current_user_can( 'manage_options' ) ); ?>>
<?php if ( AMP_Theme_Support::READER_MODE_SLUG === AMP_Theme_Support::get_support_mode() ) : ?>
<?php if ( AMP_Theme_Support::STANDARD_MODE_SLUG === AMP_Theme_Support::get_support_mode_added_via_theme() ) : ?>
<div class="notice notice-success notice-alt inline">
<p><?php esc_html_e( 'Your active theme is known to work well in standard mode.', 'amp' ); ?></p>
</div>
<?php elseif ( $builtin_support || AMP_Theme_Support::TRANSITIONAL_MODE_SLUG === AMP_Theme_Support::get_support_mode_added_via_theme() ) : ?>
<div class="notice notice-success notice-alt inline">
<p><?php esc_html_e( 'Your active theme is known to work well in standard or transitional mode.', 'amp' ); ?></p>
</div>
<?php endif; ?>
<?php endif; ?>

<?php if ( ! AMP_Theme_Support::get_support_mode_added_via_theme() ) : ?>
<p>
<?php echo wp_kses_post( $ecosystem_description ); ?>
</p>
<?php endif; ?>
<dl>
<dt>
<input type="radio" id="theme_support_standard" name="<?php echo esc_attr( AMP_Options_Manager::OPTION_NAME . '[theme_support]' ); ?>" value="<?php echo esc_attr( AMP_Theme_Support::STANDARD_MODE_SLUG ); ?>" <?php checked( $theme_support, AMP_Theme_Support::STANDARD_MODE_SLUG ); ?>>
<label for="theme_support_standard">
<strong><?php esc_html_e( 'Standard', 'amp' ); ?></strong>
</label>
</dt>
<dd>
<?php echo wp_kses_post( $standard_description ); ?>
</dd>
<dt>
<input type="radio" id="theme_support_transitional" name="<?php echo esc_attr( AMP_Options_Manager::OPTION_NAME . '[theme_support]' ); ?>" value="<?php echo esc_attr( AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); ?>" <?php checked( $theme_support, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); ?>>
<label for="theme_support_transitional">
<strong><?php esc_html_e( 'Transitional', 'amp' ); ?></strong>
</label>
</dt>
<dd>
<?php echo wp_kses_post( $transitional_description ); ?>
</dd>
<?php if ( ! AMP_Theme_Support::get_support_mode_added_via_theme() ) : ?>
<dt>
<input type="radio" id="theme_support_disabled" name="<?php echo esc_attr( AMP_Options_Manager::OPTION_NAME . '[theme_support]' ); ?>" value="disabled" <?php checked( $theme_support, 'disabled' ); ?>>
<label for="theme_support_disabled">
<strong><?php esc_html_e( 'Reader', 'amp' ); ?></strong>
</label>
</dt>
<dd>
<?php echo wp_kses_post( $reader_description ); ?>

<?php if ( ! current_theme_supports( AMP_Theme_Support::SLUG ) && wp_count_posts( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG )->publish > 0 ) : ?>
<div class="notice notice-info inline notice-alt">
<p>
<?php
echo wp_kses_post(
sprintf(
/* translators: %1: link to invalid URLs. 2: link to validation errors. */
__( 'View current site compatibility results for standard and transitional modes: %1$s and %2$s.', 'amp' ),
sprintf(
'<a href="%s">%s</a>',
esc_url( add_query_arg( 'post_type', AMP_Validated_URL_Post_Type::POST_TYPE_SLUG, admin_url( 'edit.php' ) ) ),
esc_html( get_post_type_object( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG )->labels->name )
),
sprintf(
'<a href="%s">%s</a>',
esc_url(
add_query_arg(
array(
'taxonomy' => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
'post_type' => AMP_Validated_URL_Post_Type::POST_TYPE_SLUG,
),
admin_url( 'edit-tags.php' )
)
<?php if ( ! AMP_Theme_Support::get_support_mode_added_via_theme() ) : ?>
<p>
<?php echo wp_kses_post( $ecosystem_description ); ?>
</p>
<?php endif; ?>

<dl>
<dt>
<input type="radio" id="theme_support_standard" name="<?php echo esc_attr( AMP_Options_Manager::OPTION_NAME . '[theme_support]' ); ?>" value="<?php echo esc_attr( AMP_Theme_Support::STANDARD_MODE_SLUG ); ?>" <?php checked( $theme_support, AMP_Theme_Support::STANDARD_MODE_SLUG ); ?>>
<label for="theme_support_standard">
<strong><?php esc_html_e( 'Standard', 'amp' ); ?></strong>
</label>
</dt>
<dd>
<?php echo wp_kses_post( $standard_description ); ?>
</dd>
<dt>
<input type="radio" id="theme_support_transitional" name="<?php echo esc_attr( AMP_Options_Manager::OPTION_NAME . '[theme_support]' ); ?>" value="<?php echo esc_attr( AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); ?>" <?php checked( $theme_support, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); ?>>
<label for="theme_support_transitional">
<strong><?php esc_html_e( 'Transitional', 'amp' ); ?></strong>
</label>
</dt>
<dd>
<?php echo wp_kses_post( $transitional_description ); ?>
</dd>
<dt>
<input type="radio" id="theme_support_disabled" name="<?php echo esc_attr( AMP_Options_Manager::OPTION_NAME . '[theme_support]' ); ?>" value="<?php echo esc_attr( AMP_Theme_Support::READER_MODE_SLUG ); ?>" <?php checked( $theme_support, AMP_Theme_Support::READER_MODE_SLUG ); ?>>
<label for="theme_support_disabled">
<strong><?php esc_html_e( 'Reader', 'amp' ); ?></strong>
</label>
</dt>
<dd>
<?php echo wp_kses_post( $reader_description ); ?>

<?php if ( ! current_theme_supports( AMP_Theme_Support::SLUG ) && wp_count_posts( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG )->publish > 0 ) : ?>
<div class="notice notice-info inline notice-alt">
<p>
<?php
echo wp_kses_post(
sprintf(
/* translators: %1: link to invalid URLs. 2: link to validation errors. */
__( 'View current site compatibility results for standard and transitional modes: %1$s and %2$s.', 'amp' ),
sprintf(
'<a href="%s">%s</a>',
esc_url( add_query_arg( 'post_type', AMP_Validated_URL_Post_Type::POST_TYPE_SLUG, admin_url( 'edit.php' ) ) ),
esc_html( get_post_type_object( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG )->labels->name )
),
sprintf(
'<a href="%s">%s</a>',
esc_url(
add_query_arg(
array(
'taxonomy' => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
'post_type' => AMP_Validated_URL_Post_Type::POST_TYPE_SLUG,
),
esc_html( get_taxonomy( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG )->labels->name )
admin_url( 'edit-tags.php' )
)
)
);
?>
</p>
</div>
<?php endif; ?>
</dd>
),
esc_html( get_taxonomy( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG )->labels->name )
)
)
);
?>
</p>
</div>
<?php endif; ?>
</dl>
</dd>
</dl>

<?php if ( AMP_Theme_Support::get_support_mode_added_via_theme() ) : ?>
<p>
<?php echo wp_kses_post( $ecosystem_description ); ?>
</p>
<?php endif; ?>
</fieldset>
<?php endif; ?>
<?php if ( AMP_Theme_Support::get_support_mode_added_via_theme() ) : ?>
<p>
<?php echo wp_kses_post( $ecosystem_description ); ?>
</p>
<?php endif; ?>
</fieldset>
<?php
}

Expand Down Expand Up @@ -447,7 +441,7 @@ public function render_validation_handling() {
<?php endif; ?>

<script>
(function( $, standardModeSlug ) {
(function( $, standardModeSlug, readerModeSlug ) {
const getThemeSupportMode = () => {
const checkedInput = $( 'input[type=radio][name="amp-options[theme_support]"]:checked' );
if ( 0 === checkedInput.length ) {
Expand All @@ -456,17 +450,21 @@ public function render_validation_handling() {
return checkedInput.val();
};

var updateHiddenClasses = function() {
const updateHiddenClasses = function() {
const themeSupportMode = getThemeSupportMode();
$( '.amp-auto-accept-sanitize' ).toggleClass( 'hidden', standardModeSlug === themeSupportMode );
$( '.amp-validation-field' ).toggleClass( 'hidden', 'disabled' === themeSupportMode );
$( '.amp-validation-field' ).toggleClass( 'hidden', readerModeSlug === themeSupportMode );
$( '.amp-auto-accept-sanitize-canonical' ).toggleClass( 'hidden', standardModeSlug !== themeSupportMode );
};

$( 'input[type=radio][name="amp-options[theme_support]"]' ).change( updateHiddenClasses );

updateHiddenClasses();
})( jQuery, <?php echo wp_json_encode( AMP_Theme_Support::STANDARD_MODE_SLUG ); ?> );
})(
jQuery,
<?php echo wp_json_encode( AMP_Theme_Support::STANDARD_MODE_SLUG ); ?>,
<?php echo wp_json_encode( AMP_Theme_Support::READER_MODE_SLUG ); ?>
);
</script>
</fieldset>
<?php
Expand Down
2 changes: 2 additions & 0 deletions tests/test-amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ public function test_amp_add_amphtml_link( $canonical_url, $amphtml_url ) {
'template_dir' => './',
)
);
AMP_Options_Manager::update_option( 'theme_support', AMP_Theme_Support::STANDARD_MODE_SLUG );
AMP_Theme_Support::read_theme_support();
AMP_Theme_Support::init();
$invalid_url_post_id = AMP_Validated_URL_Post_Type::store_validation_errors(
array(
Expand Down
1 change: 1 addition & 0 deletions tests/test-amp-style-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public function setUp() {
global $wp_styles, $wp_scripts;
$wp_styles = null;
$wp_scripts = null;
delete_option( AMP_Options_Manager::OPTION_NAME ); // Make sure default reader mode option does not override theme support being added.
}

/**
Expand Down
Loading

0 comments on commit ba069d4

Please sign in to comment.