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

Add admin screen for managing which post types have AMP support #811

Merged
merged 21 commits into from
Dec 6, 2017
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
c156501
Add Settings menu and page
ThierryA Nov 23, 2017
5499ba6
Add post types settings
ThierryA Nov 23, 2017
c5d0c99
Remove legacy option code
ThierryA Nov 23, 2017
1b63cd5
Load new settings and add listener
ThierryA Nov 23, 2017
2949f25
WPCS compliancy
ThierryA Nov 23, 2017
d66bc62
Move sanitization
ThierryA Nov 23, 2017
99e7bcf
Disable post type checkbox setting if AMP post type support is set
ThierryA Nov 23, 2017
68c5017
Declare post type support based on the admin setting and allow overwrite
ThierryA Nov 24, 2017
dcadc4f
Added missing since tag + minor code improvement
ThierryA Nov 24, 2017
cdf9058
AMP Settings code improvements
ThierryA Nov 28, 2017
f630062
Modified AMP admin post type support error handling
ThierryA Dec 3, 2017
fdc26b3
AMP admin post type support code improvement
ThierryA Dec 3, 2017
5b0e1cd
Correct phpdoc types
westonruter Nov 30, 2017
1befc79
Re-use existing classes; update analytics to use same validation method
westonruter Dec 5, 2017
5ea54e9
Add tests for AMP_Options_Menu
westonruter Dec 6, 2017
ad7c36b
Add tests for AMP_Post_Type_Support
westonruter Dec 6, 2017
febd02d
Add tests for AMP_Options_Manager; re-use `settings_errors()` for ana…
westonruter Dec 6, 2017
63a60bd
Update readme and add changelog
westonruter Dec 6, 2017
832c13b
Move register_settings call; fix phpdoc and improve code style
westonruter Dec 6, 2017
3b922cb
Store supported_post_types as array of post types instead of hash map…
westonruter Dec 6, 2017
8c14063
Move check_supported_post_type_update_errors to AMP_Options_Manager
westonruter Dec 6, 2017
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
1 change: 1 addition & 0 deletions .dev-lib
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
SYNC_README_MD=0
51 changes: 36 additions & 15 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,27 @@
* Plugin URI: https://github.com/automattic/amp-wp
* Author: Automattic
* Author URI: https://automattic.com
* Version: 0.5.1
* Version: 0.6.0-alpha
* Text Domain: amp
* Domain Path: /languages/
* License: GPLv2 or later
*
* @package AMP
*/

define( 'AMP__FILE__', __FILE__ );
define( 'AMP__DIR__', dirname( __FILE__ ) );
define( 'AMP__VERSION', '0.5.1' );

require_once( AMP__DIR__ . '/back-compat/back-compat.php' );
require_once( AMP__DIR__ . '/includes/amp-helper-functions.php' );
require_once( AMP__DIR__ . '/includes/admin/functions.php' );
require_once( AMP__DIR__ . '/includes/admin/class-amp-customizer.php' );
require_once( AMP__DIR__ . '/includes/settings/class-amp-customizer-settings.php' );
require_once( AMP__DIR__ . '/includes/settings/class-amp-customizer-design-settings.php' );

require_once( AMP__DIR__ . '/includes/actions/class-amp-frontend-actions.php' );
require_once( AMP__DIR__ . '/includes/actions/class-amp-paired-post-actions.php' );
define( 'AMP__VERSION', '0.6.0-alpha' );

require_once AMP__DIR__ . '/back-compat/back-compat.php';
require_once AMP__DIR__ . '/includes/amp-helper-functions.php';
require_once AMP__DIR__ . '/includes/class-amp-post-type-support.php';
require_once AMP__DIR__ . '/includes/admin/functions.php';
require_once AMP__DIR__ . '/includes/admin/class-amp-customizer.php';
require_once AMP__DIR__ . '/includes/settings/class-amp-customizer-settings.php';
require_once AMP__DIR__ . '/includes/settings/class-amp-customizer-design-settings.php';
require_once AMP__DIR__ . '/includes/actions/class-amp-frontend-actions.php';
require_once AMP__DIR__ . '/includes/actions/class-amp-paired-post-actions.php';

register_activation_hook( __FILE__, 'amp_activate' );
function amp_activate() {
Expand All @@ -47,20 +49,20 @@ function amp_deactivate() {
flush_rewrite_rules();
}

AMP_Post_Type_Support::init();

add_action( 'init', 'amp_init' );
function amp_init() {
if ( false === apply_filters( 'amp_is_enabled', true ) ) {
return;
}

define( 'AMP_QUERY_VAR', apply_filters( 'amp_query_var', 'amp' ) );

do_action( 'amp_init' );
add_action( 'admin_init', 'AMP_Options_Manager::register_settings' );
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make sense to move that to the includes/admin/functions.php file since all the other admin classes are booted in there?


load_plugin_textdomain( 'amp', false, plugin_basename( AMP__DIR__ ) . '/languages' );

add_rewrite_endpoint( AMP_QUERY_VAR, EP_PERMALINK );
add_post_type_support( 'post', AMP_QUERY_VAR );

add_filter( 'request', 'amp_force_query_var_value' );
add_action( 'wp', 'amp_maybe_add_actions' );
Expand All @@ -73,6 +75,25 @@ function amp_init() {
}
}

/**
* Define AMP query var.
*
* This function must be invoked through the 'after_setup_theme' action to allow
* the AMP setting to declare the post types support earlier than plugins/theme.
*
* @since 0.6
*/
function define_query_var() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add hook docs for this filter here since we're touching it.

It was introduced in 0.3.2 via c486a1c

/**
* Filter the AMP query variable.
*
* @since 0.3.2
* @param string $query_var The AMP query variable.
*/
define( 'AMP_QUERY_VAR', apply_filters( 'amp_query_var', 'amp' ) );
}
add_action( 'after_setup_theme', 'define_query_var', 3 );

// Make sure the `amp` query var has an explicit value.
// Avoids issues when filtering the deprecated `query_string` hook.
function amp_force_query_var_value( $query_vars ) {
Expand Down
13 changes: 10 additions & 3 deletions includes/admin/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,22 @@ function amp_add_customizer_link() {
}

/**
* Registers a top-level menu for AMP configuration options
* Registers AMP settings.
*/
function amp_add_options_menu() {
if ( ! is_admin() ) {
return;
}

$show_options_menu = apply_filters( 'amp_options_menu_is_enabled', true );
if ( true !== $show_options_menu ) {
/**
* Filter whether to enable the AMP settings.
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you find when this filter was introduced and add a @since tag for the version?

* @since 0.5
* @param bool $enable Whether to enable the AMP settings. Default true.
*/
$short_circuit = apply_filters( 'amp_options_menu_is_enabled', true );

if ( true !== $short_circuit ) {
return;
}

Expand Down
1 change: 1 addition & 0 deletions includes/class-amp-content.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ private function sanitize( $content ) {
}
}

// @todo Split this class out into a separate file.
class AMP_Content_Sanitizer {
public static function sanitize( $content, array $sanitizer_classes, $global_args = array() ) {
$scripts = array();
Expand Down
69 changes: 69 additions & 0 deletions includes/class-amp-post-type-support.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php
/**
* AMP Post type support.
*
* @package AMP
* @since 0.6
*/

/**
* Class AMP_Post_Type_Support.
*/
class AMP_Post_Type_Support {

/**
* Add hooks.
*/
public static function init() {
add_action( 'after_setup_theme', array( __CLASS__, 'add_post_type_support' ), 5 );
if ( did_action( 'after_setup_theme' ) ) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which scenario would that be used since plugins always load before the after_setup_theme action is called?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a plugin invokes init. I don't have a scenario where this would happen, so we can remove it.

self::add_post_type_support();
}
}

/**
* Get post types that plugin supports out of the box (which cannot be disabled).
*
* @return string[] Post types.
*/
public static function get_builtin_supported_post_types() {
return array_filter( array( 'post' ), 'post_type_exists' );
}

/**
* Get post types that are eligible for AMP support.
*
* @since 0.6
* @return string[] Post types eligible for AMP.
*/
public static function get_eligible_post_types() {
return array_merge(
self::get_builtin_supported_post_types(),
array_values( get_post_types(
array(
'public' => true,
'_builtin' => false,
),
'names'
) )
);
}

/**
* Declare custom post types support.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is also add support of built-in post types.

*
* This function should only be invoked through the 'after_setup_theme' action to
* allow plugins/theme to overwrite the post types support.
*
* @since 0.6
*/
public static function add_post_type_support() {
$post_types = array_merge(
self::get_builtin_supported_post_types(),
array_keys( array_filter( AMP_Options_Manager::get_option( 'supported_post_types', array() ) ) )
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the array_filter() used for since only enabled post types are saved in the db?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. Eventually I think we should just store an array of post types rather than an array mapping post type names to true. In fact, there's no reason why we can't do that now.

);
foreach ( $post_types as $post_type ) {
add_post_type_support( $post_type, AMP_QUERY_VAR );
}
}
}
5 changes: 4 additions & 1 deletion includes/options/class-amp-analytics-options-submenu.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ private function add_submenu() {
public function amp_options_styles() {
?>
<style>
.analytics-data-container #delete {
.analytics-data-container .button.delete,
.analytics-data-container .button.delete:hover,
.analytics-data-container .button.delete:active,
.analytics-data-container .button.delete:focus {
background: red;
border-color: red;
text-shadow: 0 0 0;
Expand Down
Loading