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

Issue #864: Native WordPress widget support, including subclasses #870

Merged
merged 28 commits into from
Jan 24, 2018
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3faab47
Issue #864: Dequeue some disallowed scripts, and begin subclasses.
Jan 17, 2018
be58d02
Issue #864: Remove dequeuing of scripts, and create comment subclass.
Jan 17, 2018
436722d
Issue #864: Subclass 'Gallery' widget, to output valid AMP markup.
Jan 18, 2018
44287d8
Issue #864: Subclass 'Image' widget to output valid AMP markup.
Jan 18, 2018
c7268bf
Issue #864: Subclass the 'Video' widget, in order to sanitize markup.
Jan 18, 2018
16caae9
Merge branch 'develop' into add/864-native-widget-support
Jan 18, 2018
9bd0815
Issue #864: Add filtering to widget output, more subclasses.
Jan 18, 2018
fb9b451
Issue #864: Support 'Categories' widget dropdown.
Jan 19, 2018
cbdbd4a
Issue #864: Address Travis errors, including PHPCS issue.
Jan 19, 2018
6ee75ee
Issue #864: Address failed Travis build by adding 'default' to textdo…
Jan 20, 2018
e703850
Issue #864: Remove copied filter doc, and remove non-existent widgets.
Jan 20, 2018
a355243
Issue #864: Only declare widget classes if their parents exist.
Jan 20, 2018
99dd862
Issue #864: Skip tests if the classes aren't declared.
Jan 20, 2018
74d727b
Issue #864: Only add AMP widget support if is_amp_endpoint().
Jan 22, 2018
fc48208
Make sure widget numbers start at 2
westonruter Jan 22, 2018
da6cee6
Merge branch 'develop' of https://github.com/Automattic/amp-wp into a…
westonruter Jan 23, 2018
dc3f5b4
Merge AMP_Widgets into AMP_Theme_Support
westonruter Jan 23, 2018
28b5cdd
Issue #864: Remove @todo tags from widgets, as they're implemented.
Jan 23, 2018
a0b9848
Issue #864: Make 'Archives' widget dropdown AMP-compliant.
Jan 23, 2018
6053f9a
Issue #864: Ensure register_widgets() fires on 'widget_init.'
Jan 23, 2018
e0456e1
Issue #864: Fix PHPUnit tests, accomodating is_amp_endpoint().
Jan 23, 2018
40a73c6
Issue #864: Workaround to include amp-form extension JS.
Jan 23, 2018
62a924f
Issue #864: Merge in develop, resolve conflicts.
Jan 23, 2018
c28578c
Issue #864: Remove widget subclasses that used filter_the_content().
Jan 23, 2018
6f82cc7
Fix obtaining initial widget number in add-test-widgets-to-sidebar
westonruter Jan 24, 2018
2081ab0
Remove manual enqueue of amp-form since whitelist sanitizer now handles
westonruter Jan 24, 2018
0a78830
Fix form sanitization and archives widget submission
westonruter Jan 24, 2018
0e26178
Prevent parse_query notice in is_amp_endpoint() by deferring recent c…
westonruter Jan 24, 2018
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
7 changes: 6 additions & 1 deletion bin/add-test-widgets-to-sidebar.php
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,12 @@ function amp_create_widget( $widget ) {
$title = str_replace( '-', ' ', $title );
$settings['title'] = sprintf( 'Test %s Widget', ucwords( $title ) );
}
$widgets[] = $settings;

$number = max( array_keys( $widgets ) );
$number = max( 2, $number );
$number++;

$widgets[ $number ] = $settings;
update_option( $option_key, $widgets );
}

Expand Down
8 changes: 8 additions & 0 deletions includes/class-amp-autoloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ class AMP_Autoloader {
'AMP_Image_Dimension_Extractor' => 'includes/utils/class-amp-image-dimension-extractor',
'AMP_String_Utils' => 'includes/utils/class-amp-string-utils',
'AMP_WP_Utils' => 'includes/utils/class-amp-wp-utils',
'AMP_Widget_Archives' => 'includes/widgets/class-amp-widget-archives',
'AMP_Widget_Categories' => 'includes/widgets/class-amp-widget-categories',
'AMP_Widget_Media_Audio' => 'includes/widgets/class-amp-widget-media-audio',
'AMP_Widget_Media_Gallery' => 'includes/widgets/class-amp-widget-media-gallery',
'AMP_Widget_Media_Image' => 'includes/widgets/class-amp-widget-media-image',
'AMP_Widget_Media_Video' => 'includes/widgets/class-amp-widget-media-video',
'AMP_Widget_Recent_Comments' => 'includes/widgets/class-amp-widget-recent-comments',
'AMP_Widget_RSS' => 'includes/widgets/class-amp-widget-rss',
'WPCOM_AMP_Polldaddy_Embed' => 'wpcom/class-amp-polldaddy-embed',
'AMP_Test_Stub_Sanitizer' => 'tests/stubs',
'AMP_Test_World_Sanitizer' => 'tests/stubs',
Expand Down
27 changes: 26 additions & 1 deletion includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ public static function register_paired_hooks() {
*/
public static function register_hooks() {

add_action( 'widgets_init', array( __CLASS__, 'register_widgets' ) );
Copy link
Contributor Author

@kienstra kienstra Jan 23, 2018

Choose a reason for hiding this comment

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

Order Of Actions

Hi @westonruter,
It looks like 'widgets_init' has already fired by the time register_hooks() executes.

AMP_Theme_Support::init() is called on the 'wp' action, which is after 'widgets_init'.

I had ordered the actions incorrectly, even before your commits today. And my suggestion of checking is_amp_endpoint() probably led to this. That function needs to be called before 'parse_query', which is already after 'widgets_init'.

Of course, I'm happy to debug this. I'm working on verifying the @todos in the widgets now.

Copy link
Member

Choose a reason for hiding this comment

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

@kienstra great point. It looks like maybe the theme-support logic needs to be removed from amp_maybe_add_actions() to instead be called earlier? But yeah, tough spot with the dependency on the parse_query action.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved register_widgets()

Hi @westonruter,
What do you think of this commit, which ensures that register_widgets() fires in the right place?


// Remove core actions which are invalid AMP.
remove_action( 'wp_head', 'locale_stylesheet' );
remove_action( 'wp_head', 'print_emoji_detection_script', 7 );
Expand Down Expand Up @@ -196,6 +198,29 @@ public static function register_hooks() {
// @todo Add character conversion.
}

/**
* Register/override widgets.
*
* @global WP_Widget_Factory
* @return void
*/
public static function register_widgets() {
global $wp_widget_factory;
foreach ( $wp_widget_factory->widgets as $registered_widget ) {
$registered_widget_class_name = get_class( $registered_widget );
if ( ! preg_match( '/^WP_Widget_(.+)$/', $registered_widget_class_name, $matches ) ) {
continue;
}
$amp_class_name = 'AMP_Widget_' . $matches[1];
if ( ! class_exists( $amp_class_name ) || is_a( $amp_class_name, $registered_widget_class_name ) ) {
continue;
}

unregister_widget( $registered_widget_class_name );
register_widget( $amp_class_name );
}
}

/**
* Register content embed handlers.
*
Expand Down Expand Up @@ -465,7 +490,7 @@ public static function get_amp_component_scripts( $html ) {
*
* @since 0.7
*
* @param string $amp_scripts AMP Component scripts, mapping component names to component source URLs.
* @param array $amp_scripts AMP Component scripts, mapping component names to component source URLs.
*/
$amp_scripts = apply_filters( 'amp_component_scripts', $amp_scripts );

Expand Down
31 changes: 31 additions & 0 deletions includes/widgets/class-amp-widget-archives.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php
/**
* Class AMP_Widget_Archives
*
* @package AMP
*/

/**
* Class AMP_Widget_Archives
*
* @package AMP
*/
class AMP_Widget_Archives extends WP_Widget_Archives {

/**
* Echoes the markup of the widget.
*
* @todo filter $output, to strip the onchange attribute
* @see https://github.com/Automattic/amp-wp/issues/864
* @param array $args Widget display data.
* @param array $instance Data for widget.
* @return void.
*/
public function widget( $args, $instance ) {
ob_start();
parent::widget( $args, $instance );
$output = ob_get_clean();
echo AMP_Theme_Support::filter_the_content( $output ); // WPCS: XSS ok.
}

}
91 changes: 91 additions & 0 deletions includes/widgets/class-amp-widget-categories.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<?php
/**
* Class AMP_Widget_Categories
*
* @package AMP
*/

/**
* Class AMP_Widget_Categories
*
* @package AMP
*/
class AMP_Widget_Categories extends WP_Widget_Categories {

/**
* Adds an 'on' attribute to the category dropdown's <select>.
*
* @param string $dropdown The markup of the category dropdown.
* @return string $dropdown The filtered markup of the dropdown.
*/
public function modify_select( $dropdown ) {
$new_select = sprintf( '<select on="change:widget-categories-dropdown-%d.submit"', esc_attr( $this->number ) );
Copy link
Member

Choose a reason for hiding this comment

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

Brilliant use of $this->number here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, Weston!

return str_replace( '<select', $new_select, $dropdown );
}

/**
* Echoes the markup of the widget.
*
* Mainly copied from WP_Widget_Categories::widget()
* There's now an id for the <form>.
* And the dropdown is now filtered with 'wp_dropdown_cats.'
* This enables adding an 'on' attribute, with the id of the form.
* So changing the dropdown value will redirect to the category page, with valid AMP.
*
* @param array $args Widget display data.
* @param array $instance Data for widget.
* @return void.
*/
public function widget( $args, $instance ) {
static $first_dropdown = true;
$title = ! empty( $instance['title'] ) ? $instance['title'] : __( 'Categories', 'default' );
/** This filter is documented in wp-includes/widgets/class-wp-widget-pages.php */
$title = apply_filters( 'widget_title', $title, $instance, $this->id_base );
$c = ! empty( $instance['count'] ) ? '1' : '0';
$h = ! empty( $instance['hierarchical'] ) ? '1' : '0';
$d = ! empty( $instance['dropdown'] ) ? '1' : '0';
echo wp_kses_post( $args['before_widget'] );
if ( $title ) {
echo wp_kses_post( $args['before_title'] . $title . $args['after_title'] );
}
$cat_args = array(
'orderby' => 'name',
'show_count' => $c,
'hierarchical' => $h,
);
if ( $d ) :
echo sprintf( '<form action="%s" method="get" id="widget-categories-dropdown-%d">', esc_url( home_url() ), esc_attr( $this->number ) );
add_filter( 'wp_dropdown_cats', array( $this, 'modify_select' ) );
$dropdown_id = ( $first_dropdown ) ? 'cat' : "{$this->id_base}-dropdown-{$this->number}";
$first_dropdown = false;
echo '<label class="screen-reader-text" for="' . esc_attr( $dropdown_id ) . '">' . esc_html( $title ) . '</label>';
$cat_args['show_option_none'] = __( 'Select Category', 'default' );
$cat_args['id'] = $dropdown_id;

/** This filter is documented in wp-includes/widgets/class-wp-widget-categories.php */
wp_dropdown_categories( apply_filters( 'widget_categories_dropdown_args', $cat_args, $instance ) );
remove_filter( 'wp_dropdown_cats', array( $this, 'modify_select' ) );
echo '</form>';
else :
?>
<ul>
<?php
$cat_args['title_li'] = '';
/**
* Filters the arguments for the Categories widget.
*
* @since 2.8.0
* @since 4.9.0 Added the `$instance` parameter.
*
* @param array $cat_args An array of Categories widget options.
* @param array $instance Array of settings for the current widget.
Copy link
Member

Choose a reason for hiding this comment

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

Instead of copying the phpdoc, you can /** This filter is documented in ... */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, this line applies your suggestion.

Copy link
Member

Choose a reason for hiding this comment

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

You can do the same with the widget_categories_args filter here.

*/
wp_list_categories( apply_filters( 'widget_categories_args', $cat_args, $instance ) );
?>
</ul>
<?php
endif;
echo wp_kses_post( $args['after_widget'] );
}

}
34 changes: 34 additions & 0 deletions includes/widgets/class-amp-widget-media-audio.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php
/**
* Class AMP_Widget_Media_Audio
*
* @package AMP
*/

if ( class_exists( 'WP_Widget_Media_Audio' ) ) {
/**
* Class AMP_Widget_Media_Audio
*
* @package AMP
*/
class AMP_Widget_Media_Audio extends WP_Widget_Media_Audio {

/**
* Echoes the markup of the widget.
*
* @todo filter $output, to convert <audio> to <amp-audio>.
* @see https://github.com/Automattic/amp-wp/issues/864
*
* @param array $instance Data for widget.
*
* @return void.
*/
public function render_media( $instance ) {
ob_start();
parent::render_media( $instance );
$output = ob_get_clean();
echo AMP_Theme_Support::filter_the_content( $output ); // WPCS: XSS ok.
}

}
}
32 changes: 32 additions & 0 deletions includes/widgets/class-amp-widget-media-gallery.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php
/**
* Class AMP_Widget_Media_Gallery
*
* @package AMP
*/

if ( class_exists( 'WP_Widget_Media_Gallery' ) ) {
/**
* Class AMP_Widget_Media_Gallery
*
* @package AMP
*/
class AMP_Widget_Media_Gallery extends WP_Widget_Media_Gallery {

/**
* Echoes the markup of the widget.
*
* @todo filter $output, to convert <imp> to <amp-img> and remove the <style>.
* @see https://github.com/Automattic/amp-wp/issues/864
* @param array $instance Data for widget.
* @return void.
*/
public function render_media( $instance ) {
ob_start();
parent::render_media( $instance );
$output = ob_get_clean();
echo AMP_Theme_Support::filter_the_content( $output ); // WPCS: XSS ok.
}

}
}
34 changes: 34 additions & 0 deletions includes/widgets/class-amp-widget-media-image.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php
/**
* Class AMP_Widget_Media_Image
*
* @package AMP
*/

if ( class_exists( 'WP_Widget_Media_Image' ) ) {
/**
* Class AMP_Widget_Media_Image
*
* @package AMP
*/
class AMP_Widget_Media_Image extends WP_Widget_Media_Image {

/**
* Echoes the markup of the widget.
*
* @todo filter $output, to convert <imp> to <amp-img> and remove the 'style' attribute.
* @see https://github.com/Automattic/amp-wp/issues/864
*
* @param array $instance Data for widget.
*
* @return void.
*/
public function render_media( $instance ) {
ob_start();
parent::render_media( $instance );
$output = ob_get_clean();
echo AMP_Theme_Support::filter_the_content( $output ); // WPCS: XSS ok.
}

}
}
34 changes: 34 additions & 0 deletions includes/widgets/class-amp-widget-media-video.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php
/**
* Class AMP_Widget_Media_Video
*
* @package AMP
*/

if ( class_exists( 'WP_Widget_Media_Video' ) ) {
/**
* Class AMP_Widget_Media_Video
*
* @package AMP
*/
class AMP_Widget_Media_Video extends WP_Widget_Media_Video {

/**
* Echoes the markup of the widget.
*
* @todo filter $output, to convert <video> to <amp-video> and remove the 'style' attribute.
* @see https://github.com/Automattic/amp-wp/issues/864
*
* @param array $instance Data for widget.
*
* @return void.
*/
public function render_media( $instance ) {
ob_start();
parent::render_media( $instance );
$output = ob_get_clean();
echo AMP_Theme_Support::filter_the_content( $output ); // WPCS: XSS ok.
}

}
}
23 changes: 23 additions & 0 deletions includes/widgets/class-amp-widget-recent-comments.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php
/**
* Class AMP_Widget_Recent_Comments
*
* @package AMP
*/

/**
* Class AMP_Widget_Recent_Comments
*
* @package AMP
*/
class AMP_Widget_Recent_Comments extends WP_Widget_Recent_Comments {

/**
* Instantiates the widget, and prevents inline styling.
*/
public function __construct() {
parent::__construct();
add_filter( 'show_recent_comments_widget_style', '__return_false' );
}

}
31 changes: 31 additions & 0 deletions includes/widgets/class-amp-widget-rss.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php
/**
* Class AMP_Widget_RSS
*
* @package AMP
*/

/**
* Class AMP_Widget_RSS
*
* @package AMP
*/
class AMP_Widget_RSS extends WP_Widget_RSS {

/**
* Echoes the markup of the widget.
*
* @todo filter $output, to convert <img> to <amp-img>.
* @see https://github.com/Automattic/amp-wp/issues/864
* @param array $args Widget display data.
* @param array $instance Data for widget.
* @return void.
*/
public function widget( $args, $instance ) {
ob_start();
parent::widget( $args, $instance );
$output = ob_get_clean();
echo AMP_Theme_Support::filter_the_content( $output ); // WPCS: XSS ok.
}

}
Loading