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 9 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
36 changes: 24 additions & 12 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,17 @@
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' );
require_once AMP__DIR__ . '/back-compat/back-compat.php';
require_once AMP__DIR__ . '/includes/amp-helper-functions.php';
require_once AMP__DIR__ . '/includes/amp-post-types-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/settings/class-amp-settings.php';
require_once AMP__DIR__ . '/includes/settings/class-amp-settings-post-types.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 Down Expand Up @@ -53,14 +55,11 @@ function amp_init() {
return;
}

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

do_action( 'amp_init' );

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 +72,19 @@ 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

define( 'AMP_QUERY_VAR', apply_filters( 'amp_query_var', 'amp' ) );
}
add_action( 'after_setup_theme', 'define_query_var', 7 );

// 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
17 changes: 14 additions & 3 deletions includes/admin/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,29 @@ 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;
}

// Initialize settings.
AMP_Settings::get_instance()->init();
AMP_Settings_Post_Types::get_instance()->init();

$amp_options = new AMP_Options_Menu();
$amp_options->init();
}
Expand Down
2 changes: 1 addition & 1 deletion includes/amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function amp_get_permalink( $post_id ) {
}

function post_supports_amp( $post ) {
// Because `add_rewrite_endpoint` doesn't let us target specific post_types :(
// Because `add_rewrite_endpoint` doesn't let us target specific post_types :(.
if ( ! post_type_supports( $post->post_type, AMP_QUERY_VAR ) ) {
return false;
}
Expand Down
35 changes: 35 additions & 0 deletions includes/amp-post-types-support.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php
/**
* AMP Post types support.
*
* @package AMP
* @since 0.6
*/

/**
* Declare core post types support.
*
* @since 0.6
*/
function amp_core_post_types_support() {
add_post_type_support( 'post', AMP_QUERY_VAR );
}
add_action( 'init', 'amp_core_post_types_support' );

/**
* Declare custom post types support.
*
* 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
*/
function amp_custom_post_types_support() {
// Listen to post types settings.
foreach ( AMP_Settings_Post_Types::get_instance()->get_settings() as $post_type => $enabled ) {
if ( true === $enabled ) {
add_post_type_support( $post_type, AMP_QUERY_VAR );
}
}
}
add_action( 'after_setup_theme', 'amp_custom_post_types_support' );
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this shouldn't have a lower priority, like 5.

The _s starter theme and Twenty Seventeen among others do their add_theme_support calls in after_setup_theme at priority 10.
https://github.com/Automattic/_s/blob/efd37d13a6622bcd20e8c7b35309102c06981c71/functions.php#L10-L84
https://github.com/WordPress/wordpress-develop/blob/a825a181e11b1a89e76d79d22bd2c220f45b3741/src/wp-content/themes/twentyseventeen/functions.php#L20-L217

So if they also do calls to add_post_type_support() I'd expect them to do it here as well, for example: https://github.com/ryelle/Anadama-React/blob/21c8f550bc3ea6bbb7407891bf06845316b0ebf6/functions.php#L78-L82

63 changes: 21 additions & 42 deletions includes/options/class-amp-options-menu.php
Original file line number Diff line number Diff line change
@@ -1,54 +1,33 @@
<?php

require_once( AMP__DIR__ . '/includes/options/class-amp-analytics-options-submenu.php' );
require_once( AMP__DIR__ . '/includes/options/views/class-amp-options-menu-page.php' );
require_once( AMP__DIR__ . '/includes/options/views/class-amp-options-manager.php' );

/**
* AMP Options.
*
* @package AMP
*/

// Includes.
require_once AMP__DIR__ . '/includes/options/class-amp-analytics-options-submenu.php';
require_once AMP__DIR__ . '/includes/options/views/class-amp-options-manager.php';

/**
* AMP_Options_Menu class.
*/
class AMP_Options_Menu {
const ICON_BASE64_SVG = 'data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiIHN0YW5kYWxvbmU9Im5vIj8+PHN2ZyB3aWR0aD0iNjJweCIgaGVpZ2h0PSI2MnB4IiB2aWV3Qm94PSIwIDAgNjIgNjIiIHZlcnNpb249IjEuMSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIiB4bWxuczp4bGluaz0iaHR0cDovL3d3dy53My5vcmcvMTk5OS94bGluayI+ICAgICAgICA8dGl0bGU+QU1QLUJyYW5kLUJsYWNrLUljb248L3RpdGxlPiAgICA8ZGVzYz5DcmVhdGVkIHdpdGggU2tldGNoLjwvZGVzYz4gICAgPGRlZnM+PC9kZWZzPiAgICA8ZyBpZD0iYW1wLWxvZ28taW50ZXJuYWwtc2l0ZSIgc3Ryb2tlPSJub25lIiBzdHJva2Utd2lkdGg9IjEiIGZpbGw9Im5vbmUiIGZpbGwtcnVsZT0iZXZlbm9kZCI+ICAgICAgICA8ZyBpZD0iQU1QLUJyYW5kLUJsYWNrLUljb24iIGZpbGw9IiMwMDAwMDAiPiAgICAgICAgICAgIDxwYXRoIGQ9Ik00MS42Mjg4NjY3LDI4LjE2MTQzMzMgTDI4LjYyNDM2NjcsNDkuODAzNTY2NyBMMjYuMjY4MzY2Nyw0OS44MDM1NjY3IEwyOC41OTc1LDM1LjcwMTY2NjcgTDIxLjM4MzgsMzUuNzEwOTY2NyBDMjEuMzgzOCwzNS43MTA5NjY3IDIxLjMxNTYsMzUuNzEzMDMzMyAyMS4yODM1NjY3LDM1LjcxMzAzMzMgQzIwLjYzMzYsMzUuNzEzMDMzMyAyMC4xMDc2MzMzLDM1LjE4NzA2NjcgMjAuMTA3NjMzMywzNC41MzcxIEMyMC4xMDc2MzMzLDM0LjI1ODEgMjAuMzY3LDMzLjc4NTg2NjcgMjAuMzY3LDMzLjc4NTg2NjcgTDMzLjMyOTEzMzMsMTIuMTY5NTY2NyBMMzUuNzI0NCwxMi4xNzk5IEwzMy4zMzYzNjY3LDI2LjMwMzUgTDQwLjU4NzI2NjcsMjYuMjk0MiBDNDAuNTg3MjY2NywyNi4yOTQyIDQwLjY2NDc2NjcsMjYuMjkzMTY2NyA0MC43MDE5NjY3LDI2LjI5MzE2NjcgQzQxLjM1MTkzMzMsMjYuMjkzMTY2NyA0MS44Nzc5LDI2LjgxOTEzMzMgNDEuODc3OSwyNy40NjkxIEM0MS44Nzc5LDI3LjczMjYgNDEuNzc0NTY2NywyNy45NjQwNjY3IDQxLjYyNzgzMzMsMjguMTYwNCBMNDEuNjI4ODY2NywyOC4xNjE0MzMzIFogTTMxLDAgQzEzLjg3ODcsMCAwLDEzLjg3OTczMzMgMCwzMSBDMCw0OC4xMjEzIDEzLjg3ODcsNjIgMzEsNjIgQzQ4LjEyMDI2NjcsNjIgNjIsNDguMTIxMyA2MiwzMSBDNjIsMTMuODc5NzMzMyA0OC4xMjAyNjY3LDAgMzEsMCBMMzEsMCBaIiBpZD0iRmlsbC0xIj48L3BhdGg+ICAgICAgICA8L2c+ICAgIDwvZz48L3N2Zz4=';

private $menu_page;
private $menu_slug;

public function __construct() {
$this->menu_page = new AMP_Options_Menu_Page();
$this->menu_slug = 'amp-plugin-options';
}

/**
* Initialize.
*/
public function init() {
add_action( 'admin_post_amp_analytics_options', 'AMP_Options_Manager::handle_analytics_submit' );

add_action( 'admin_menu', array( $this, 'add_menu_items' ) );
}

/**
* Add menu.
*/
public function add_menu_items() {
add_menu_page(
__( 'AMP Options', 'amp' ),
__( 'AMP', 'amp' ),
'manage_options',
$this->menu_slug,
array( $this->menu_page, 'render' ),
self::ICON_BASE64_SVG
);

$submenus = array(
new AMP_Analytics_Options_Submenu( $this->menu_slug ),
);

// Create submenu items and calls on the Submenu Page object to render the actual contents of the page.
foreach ( $submenus as $submenu ) {
$submenu->init( $this->menu_slug );
}

$this->remove_toplevel_menu_item();
$submenu = new AMP_Analytics_Options_Submenu( AMP_Settings::MENU_SLUG );
$submenu->init( AMP_Settings::MENU_SLUG );
}

// Helper function to avoid having the top-level menu as
// the first menu item
function remove_toplevel_menu_item() {
global $submenu;
if ( isset( $submenu['amp-plugin-options'][0] ) ) {
unset( $submenu['amp-plugin-options'][0] );
}
}
}
17 changes: 0 additions & 17 deletions includes/options/views/class-amp-options-menu-page.php

This file was deleted.

Loading