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

Refactor AMP modes #2550

Merged
merged 21 commits into from
Jun 11, 2019
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
bb506c3
Always allow switching themes with built-in Transitional support to N…
westonruter Jun 8, 2019
7952fce
Prevent AMP validation from appearing in Reader mode on non-Story posts
westonruter Jun 8, 2019
a24e715
Remove dead options from validated environment
westonruter Jun 8, 2019
0ba0495
Move AMP Stories section setting right after template mode
westonruter Jun 9, 2019
d7680cb
Add AMP experiences checkboxes
westonruter Jun 10, 2019
98efeb0
Conditionally do functionality for website and stories of experienced…
westonruter Jun 10, 2019
6f8a8d8
Ensure rewrite rules are flushed properly when toggling website exper…
westonruter Jun 10, 2019
07cbb55
Replace most Native instances with Standard
westonruter Jun 10, 2019
ea75efc
Hide validation screens when website experience disabled
westonruter Jun 10, 2019
f180458
Fix typo in validation message
westonruter Jun 10, 2019
55f1f1f
Add experiences to generator meta tag; use new mode identifiers
westonruter Jun 10, 2019
4d7920f
Upate constants for modes, use instead of strings
westonruter Jun 10, 2019
00a756a
Eliminate obsolete enable_amp_stories option
westonruter Jun 10, 2019
05dda18
Fix availability of Latest Stories block when Website experience disa…
westonruter Jun 10, 2019
6e12cf1
Update website experience description
westonruter Jun 11, 2019
2704a26
Use AMP-first terminology
westonruter Jun 11, 2019
bbc6f0f
Further clarify comments
westonruter Jun 11, 2019
9737cd7
Ensure auto-accepting sanitization is always performed for stories; s…
westonruter Jun 11, 2019
5ff39d2
Do not modify editor styles for non-story post types
swissspidy Jun 11, 2019
5589b0e
Simplify the block editor JS
swissspidy Jun 11, 2019
631bcd6
“AMP first” -> “AMP-first” everywhere
swissspidy Jun 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
56 changes: 30 additions & 26 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ function amp_deactivate() {
}
}

flush_rewrite_rules();
flush_rewrite_rules( false );
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
}

/*
Expand Down Expand Up @@ -333,38 +333,42 @@ function amp_init() {
*/
do_action( 'amp_init' );

add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );

add_filter( 'allowed_redirect_hosts', array( 'AMP_HTTP', 'filter_allowed_redirect_hosts' ) );
AMP_HTTP::purge_amp_query_vars();
AMP_HTTP::send_cors_headers();
AMP_HTTP::handle_xhr_request();
AMP_Theme_Support::init();
AMP_Validation_Manager::init();
AMP_Post_Type_Support::add_post_type_support();
AMP_Story_Post_Type::register();
AMP_Service_Worker::init();
add_action( 'init', array( 'AMP_Post_Type_Support', 'add_post_type_support' ), 1000 ); // After post types have been defined.

if ( defined( 'WP_CLI' ) && WP_CLI ) {
WP_CLI::add_command( 'amp', new AMP_CLI() );
}

add_filter( 'request', 'amp_force_query_var_value' );
add_action( 'admin_init', 'AMP_Options_Manager::register_settings' );
add_action( 'wp_loaded', 'amp_editor_core_blocks' );
add_action( 'wp_loaded', 'amp_post_meta_box' );
add_action( 'wp_loaded', 'amp_story_templates' );
add_action( 'wp_loaded', 'amp_add_options_menu' );
add_action( 'wp_loaded', 'amp_admin_pointer' );
add_action( 'parse_query', 'amp_correct_query_when_is_front_page' );
add_action( 'admin_bar_menu', 'amp_add_admin_bar_view_link', 100 );
add_action( 'wp_loaded', 'amp_post_meta_box' ); // Used in both Website and Stories experiences.

if ( AMP_Options_Manager::is_website_experience_enabled() ) {
add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );
AMP_Post_Type_Support::add_post_type_support();
add_action( 'init', array( 'AMP_Post_Type_Support', 'add_post_type_support' ), 1000 ); // After post types have been defined.
add_action( 'parse_query', 'amp_correct_query_when_is_front_page' );
add_action( 'admin_bar_menu', 'amp_add_admin_bar_view_link', 100 );
add_action( 'wp_loaded', 'amp_editor_core_blocks' );
add_filter( 'request', 'amp_force_query_var_value' );

// Add actions for reader mode templates.
add_action( 'wp', 'amp_maybe_add_actions' );

// Redirect the old url of amp page to the updated url.
add_filter( 'old_slug_redirect_url', 'amp_redirect_old_slug_to_new_url' );
}

// Redirect the old url of amp page to the updated url.
add_filter( 'old_slug_redirect_url', 'amp_redirect_old_slug_to_new_url' );
if ( AMP_Options_Manager::is_stories_experience_enabled() ) {
AMP_Story_Post_Type::register();
add_action( 'wp_loaded', 'amp_story_templates' );
}

// Add actions for legacy post templates.
add_action( 'wp', 'amp_maybe_add_actions' );
if ( defined( 'WP_CLI' ) && WP_CLI ) {
WP_CLI::add_command( 'amp', new AMP_CLI() );
}

/*
* Broadcast plugin updates.
Expand Down Expand Up @@ -513,7 +517,7 @@ function amp_correct_query_when_is_front_page( WP_Query $query ) {
*
* add_theme_support( AMP_Theme_Support::SLUG );
*
* This will serve templates in native AMP, allowing you to use AMP components in your theme templates.
* This will serve templates in AMP-first, allowing you to use AMP components in your theme templates.
* If you want to make available in transitional mode, where templates are served in AMP or non-AMP documents, do:
*
* add_theme_support( AMP_Theme_Support::SLUG, array(
Expand All @@ -526,7 +530,7 @@ function amp_correct_query_when_is_front_page( WP_Query $query ) {
* 'template_dir' => 'amp',
* ) );
*
* If you want to have AMP-specific templates in addition to serving native AMP, do:
* If you want to have AMP-specific templates in addition to serving AMP-first, do:
*
* add_theme_support( AMP_Theme_Support::SLUG, array(
* 'paired' => false,
Expand All @@ -549,16 +553,16 @@ function amp_correct_query_when_is_front_page( WP_Query $query ) {
* ) );
*
* @see AMP_Theme_Support::read_theme_support()
* @return boolean Whether this is in AMP 'canonical' mode, that is whether it is native and there is not separate AMP URL current URL.
* @return boolean Whether this is in AMP 'canonical' mode, that is whether it is AMP-first and there is not a separate (paired) AMP URL.
*/
function amp_is_canonical() {
if ( ! current_theme_supports( AMP_Theme_Support::SLUG ) ) {
return false;
}

$args = AMP_Theme_Support::get_theme_support_args();
if ( isset( $args['paired'] ) ) {
return empty( $args['paired'] );
if ( isset( $args[ AMP_Theme_Support::PAIRED_FLAG ] ) ) {
return empty( $args[ AMP_Theme_Support::PAIRED_FLAG ] );
}

// If there is a template_dir, then transitional mode is implied.
Expand Down
47 changes: 30 additions & 17 deletions assets/src/block-editor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,23 @@ import './store';

const { ampLatestStoriesBlockData } = window;

addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/addAttributes', addAMPAttributes );
addFilter( 'blocks.getSaveElement', 'ampEditorBlocks/filterSave', filterBlocksSave );
addFilter( 'editor.BlockEdit', 'ampEditorBlocks/filterEdit', filterBlocksEdit, 20 );
addFilter( 'blocks.getSaveContent.extraProps', 'ampEditorBlocks/addExtraAttributes', addAMPExtraProps );
addFilter( 'editor.PostFeaturedImage', 'ampEditorBlocks/withFeaturedImageNotice', withFeaturedImageNotice );
addFilter( 'editor.MediaUpload', 'ampEditorBlocks/addCroppedFeaturedImage', ( InitialMediaUpload ) => withCroppedFeaturedImage( InitialMediaUpload, getMinimumFeaturedImageDimensions() ) );
// Add filters if AMP for Website experience is enabled.
if ( select( 'amp/block-editor' ).isWebsiteEnabled() ) {
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/addAttributes', addAMPAttributes );
addFilter( 'blocks.getSaveElement', 'ampEditorBlocks/filterSave', filterBlocksSave );
addFilter( 'editor.BlockEdit', 'ampEditorBlocks/filterEdit', filterBlocksEdit, 20 );
addFilter( 'blocks.getSaveContent.extraProps', 'ampEditorBlocks/addExtraAttributes', addAMPExtraProps );
addFilter( 'editor.PostFeaturedImage', 'ampEditorBlocks/withFeaturedImageNotice', withFeaturedImageNotice );
addFilter( 'editor.MediaUpload', 'ampEditorBlocks/addCroppedFeaturedImage', ( InitialMediaUpload ) => withCroppedFeaturedImage( InitialMediaUpload, getMinimumFeaturedImageDimensions() ) );

const plugins = require.context( './plugins', true, /.*\.js$/ );
const plugins = require.context( './plugins', true, /.*\.js$/ );

plugins.keys().forEach( ( modulePath ) => {
const { name, render, icon } = plugins( modulePath );
plugins.keys().forEach( ( modulePath ) => {
const { name, render, icon } = plugins( modulePath );

registerPlugin( name, { icon, render } );
} );
registerPlugin( name, { icon, render } );
} );
}

/*
* If there's no theme support, unregister blocks that are only meant for AMP.
Expand All @@ -52,14 +55,24 @@ const blocks = require.context( './blocks', true, /(?<!test\/)index\.js$/ );
blocks.keys().forEach( ( modulePath ) => {
const { name, settings } = blocks( modulePath );

// Prevent registering latest-stories block if not enabled.
if ( 'amp/amp-latest-stories' === name && typeof ampLatestStoriesBlockData === 'undefined' ) {
return;
}
const isLatestStoriesBlock = 'amp/amp-latest-stories' === name;

const blockRequiresAmp = AMP_DEPENDENT_BLOCKS.includes( name );
const shouldRegister = (
(
select( 'amp/block-editor' ).isWebsiteEnabled() &&
(
( ! isLatestStoriesBlock && select( 'amp/block-editor' ).isStandardMode() && AMP_DEPENDENT_BLOCKS.includes( name ) ) ||
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
( isLatestStoriesBlock && select( 'amp/block-editor' ).isStoriesEnabled() )
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
)
) ||
(
select( 'amp/block-editor' ).isStoriesEnabled() &&
isLatestStoriesBlock &&
typeof ampLatestStoriesBlockData !== 'undefined'
)
);

if ( ! blockRequiresAmp || select( 'amp/block-editor' ).isNativeAMP() ) {
if ( shouldRegister ) {
registerBlockType( name, settings );
}
} );
30 changes: 26 additions & 4 deletions assets/src/block-editor/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,36 @@ export function hasThemeSupport( state ) {
}

/**
* Returns whether the current site uses native AMP.
* Returns whether the current site is in Standard mode (AMP first) as opposed to Transitional (paired).
*
* @param {Object} state Editor state.
*
* @return {boolean} Whether the current site uses native AMP.
* @return {boolean} Whether the current site is AMP first.
*/
export function isNativeAMP( state ) {
return Boolean( state.isNativeAMP );
export function isStandardMode( state ) {
return Boolean( state.isStandardMode );
}

/**
* Returns whether the website experience is enabled.
*
* @param {Object} state Editor state.
*
* @return {boolean} Whether website experienced enabled.
*/
export function isWebsiteEnabled( state ) {
return Boolean( state.isWebsiteEnabled );
}

/**
* Returns whether the stories experience is enabled.
*
* @param {Object} state Editor state.
*
* @return {boolean} Whether stories experienced enabled.
*/
export function isStoriesEnabled( state ) {
return Boolean( state.isStoriesEnabled );
}

export function getDefaultStatus( state ) {
Expand Down
91 changes: 47 additions & 44 deletions assets/src/block-validation/helpers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ export const updateValidationErrors = () => {
export const maybeDisplayNotice = () => {
const { getValidationErrors, isSanitizationAutoAccepted, getReviewLink } = select( 'amp/block-validation' );
const { createWarningNotice } = dispatch( 'core/notices' );
const { getCurrentPost } = select( 'core/editor' );

const validationErrors = getValidationErrors();
const validationErrorCount = validationErrors.length;
Expand All @@ -174,54 +175,56 @@ export const maybeDisplayNotice = () => {
const blockValidationErrors = validationErrors.filter( ( { clientId } ) => clientId );
const blockValidationErrorCount = blockValidationErrors.length;

if ( blockValidationErrorCount > 0 ) {
noticeMessage += ' ' + sprintf(
/* translators: %s: number of block errors. */
_n(
'%s issue is directly due to content here.',
'%s issues are directly due to content here.',
blockValidationErrorCount,
'amp'
),
blockValidationErrorCount
);
} else if ( validationErrors.length === 1 ) {
noticeMessage += ' ' + __( 'The issue is not directly due to content here.', 'amp' );
} else {
noticeMessage += ' ' + __( 'The issues are not directly due to content here.', 'amp' );
}

noticeMessage += ' ';

if ( isSanitizationAutoAccepted() ) {
const rejectedBlockValidationErrors = blockValidationErrors.filter( ( error ) => {
return (
VALIDATION_ERROR_NEW_REJECTED_STATUS === error.status ||
VALIDATION_ERROR_ACK_REJECTED_STATUS === error.status
);
} );

const rejectedValidationErrors = validationErrors.filter( ( error ) => {
return (
VALIDATION_ERROR_NEW_REJECTED_STATUS === error.status ||
VALIDATION_ERROR_ACK_REJECTED_STATUS === error.status
if ( 'amp_story' !== getCurrentPost().type ) {
if ( blockValidationErrorCount > 0 ) {
noticeMessage += ' ' + sprintf(
/* translators: %s: number of block errors. */
_n(
'%s issue is directly due to content here.',
'%s issues are directly due to content here.',
blockValidationErrorCount,
'amp'
),
blockValidationErrorCount
);
} );

const totalRejectedErrorsCount = rejectedBlockValidationErrors.length + rejectedValidationErrors.length;
} else if ( validationErrors.length === 1 ) {
noticeMessage += ' ' + __( 'The issue is not directly due to content here.', 'amp' );
} else {
noticeMessage += ' ' + __( 'The issues are not directly due to content here.', 'amp' );
}

if ( totalRejectedErrorsCount === 0 ) {
noticeMessage += __( 'However, your site is configured to automatically accept sanitization of the offending markup.', 'amp' );
noticeMessage += ' ';

if ( isSanitizationAutoAccepted() ) {
const rejectedBlockValidationErrors = blockValidationErrors.filter( ( error ) => {
return (
VALIDATION_ERROR_NEW_REJECTED_STATUS === error.status ||
VALIDATION_ERROR_ACK_REJECTED_STATUS === error.status
);
} );

const rejectedValidationErrors = validationErrors.filter( ( error ) => {
return (
VALIDATION_ERROR_NEW_REJECTED_STATUS === error.status ||
VALIDATION_ERROR_ACK_REJECTED_STATUS === error.status
);
} );

const totalRejectedErrorsCount = rejectedBlockValidationErrors.length + rejectedValidationErrors.length;

if ( totalRejectedErrorsCount === 0 ) {
noticeMessage += __( 'However, your site is configured to automatically accept sanitization of the offending markup.', 'amp' );
} else {
noticeMessage += _n(
'Your site is configured to automatically accept sanitization errors, but this error could be from when auto-acceptance was not selected, or from manually rejecting an error.',
'Your site is configured to automatically accept sanitization errors, but these errors could be from when auto-acceptance was not selected, or from manually rejecting an error.',
validationErrors.length,
'amp'
);
}
} else {
noticeMessage += _n(
'Your site is configured to automatically accept sanitization errors, but this error could be from when auto-acceptance was not selected, or from manually rejecting an error.',
'Your site is configured to automatically accept sanitization errors, but these errors could be from when auto-acceptance was not selected, or from manually rejecting an error.',
validationErrors.length,
'amp'
);
noticeMessage += __( 'Non-accepted validation errors prevent AMP from being served, and the user will be redirected to the non-AMP version.', 'amp' );
}
} else {
noticeMessage += __( 'Non-accepted validation errors prevent AMP from being served, and the user will be redirected to the non-AMP version.', 'amp' );
}

const options = {
Expand Down
2 changes: 1 addition & 1 deletion assets/src/block-validation/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function getReviewLink( state ) {
/**
* Returns whether sanitization errors are auto-accepted.
*
* Auto-acceptance is from either checking 'Automatically accept sanitization...' or from being in Native mode.
* Auto-acceptance is from either checking 'Automatically accept sanitization...' or from being in Standard mode.
*
* @param {Object} state Editor state.
*
Expand Down
6 changes: 3 additions & 3 deletions includes/admin/class-amp-admin-pointers.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ private function get_pointers() {
'selector' => '#toplevel_page_amp-options',
'heading' => __( 'AMP', 'amp' ),
'subheading' => __( 'New AMP Template Modes', 'amp' ),
'description' => __( 'You can now reuse your theme\'s templates and styles in AMP responses, in both &#8220;Transitional&#8221; and &#8220;Native&#8221; modes.', 'amp' ),
'description' => __( 'You can now reuse your theme\'s templates and styles in AMP responses, in both &#8220;Transitional&#8221; and &#8220;Standard&#8221; modes.', 'amp' ),
'position' => array(
'align' => 'middle',
),
Expand All @@ -87,7 +87,7 @@ private function get_pointers() {
if ( 'toplevel_page_amp-options' === $hook_suffix ) {
return false;
}
return ! AMP_Options_Manager::get_option( 'enable_amp_stories' );
return ! AMP_Options_Manager::is_stories_experience_enabled();
},
)
),
Expand All @@ -104,7 +104,7 @@ private function get_pointers() {
if ( 'edit.php' === $hook_suffix && AMP_Story_Post_Type::POST_TYPE_SLUG === filter_input( INPUT_GET, 'post_type' ) ) {
return false;
}
return AMP_Story_Post_Type::has_required_block_capabilities() && AMP_Options_Manager::get_option( 'enable_amp_stories' );
return AMP_Options_Manager::is_stories_experience_enabled();
},
)
),
Expand Down
4 changes: 2 additions & 2 deletions includes/admin/class-amp-editor-blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ public function init() {
add_filter( 'wp_kses_allowed_html', array( $this, 'whitelist_block_atts_in_wp_kses_allowed_html' ), 10, 2 );

/*
* Dirty AMP is required when a site is in native mode but not all templates are being served
* Dirty AMP is required when a site is in AMP-first mode but not all templates are being served
* as AMP. In particular, if a single post is using AMP-specific Gutenberg Blocks which make
* use of AMP components, and the singular template is served as AMP but the blog page is not,
* then the non-AMP blog page need to load the AMP runtime scripts so that the AMP components
* in the posts displayed there will be rendered properly. This is only relevant on native AMP
* in the posts displayed there will be rendered properly. This is only relevant on AMP-first
* sites because the AMP Gutenberg blocks are only made available in that mode; they are not
* presented in the Gutenberg inserter in transitional mode. In general, using AMP components in
* non-AMP documents is still not officially supported, so it's occurrence is being minimized
Expand Down
4 changes: 3 additions & 1 deletion includes/admin/class-amp-post-meta-box.php
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,10 @@ public function enqueue_block_assets() {
'possibleStatuses' => array( self::ENABLED_STATUS, self::DISABLED_STATUS ),
'defaultStatus' => $enabled_status,
'errorMessages' => $error_messages,
'isWebsiteEnabled' => AMP_Options_Manager::is_website_experience_enabled(),
'isStoriesEnabled' => AMP_Options_Manager::is_stories_experience_enabled(),
'hasThemeSupport' => current_theme_supports( AMP_Theme_Support::SLUG ),
'isNativeAMP' => amp_is_canonical(),
'isStandardMode' => amp_is_canonical(),
);

wp_localize_script(
Expand Down
2 changes: 1 addition & 1 deletion includes/admin/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
* And this does not need to toggle between the AMP and normal display.
*/
function amp_init_customizer() {
if ( amp_is_canonical() ) {
if ( amp_is_canonical() || ! AMP_Options_Manager::is_website_experience_enabled() ) {
return;
}

Expand Down
Loading