From 52943ed68df1c45aeca59b4c7bb0214a0a366585 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Fri, 4 May 2018 11:22:27 +0300 Subject: [PATCH 01/21] Start adding carousel toggle for Gallery. --- assets/js/amp-editor-blocks.js | 38 ++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index e68bae4b449..fb35276bd48 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -127,8 +127,8 @@ var ampEditorBlocks = ( function() { 'core/audio' ]; - // Gallery settings for shortcode. - if ( 'core/shortcode' === name ) { + // AMP Carousel settings. + if ( 'core/shortcode' === name || 'core/gallery' ) { if ( ! settings.attributes ) { settings.attributes = {}; } @@ -184,6 +184,8 @@ var ampEditorBlocks = ( function() { }, props ) ) ]; } + } else if ( 'core/gallery' === name ) { + inspectorControls = component.setUpGalleryInpsectorControls( props ); } else { inspectorControls = component.setUpInspectorControls( props ); } @@ -290,6 +292,38 @@ var ampEditorBlocks = ( function() { ); }; + /** + * Set up inspector controls for Gallery block. + * Adds ampCarousel attribute for displaying the output as amp-carousel. + * + * @param {Object} props Props. + * @return {*} Inspector controls. + */ + component.setUpGalleryInpsectorControls = function setUpGalleryInpsectorControls( props ) { + var ampCarousel = props.attributes.ampCarousel, + isSelected = props.isSelected, + el = wp.element.createElement, + InspectorControls = wp.blocks.InspectorControls, + ToggleControl = wp.components.ToggleControl, + PanelBody = wp.components.PanelBody, + toggleControl; + + toggleControl = el( ToggleControl, { + label: 'Display as AMP carousel', + checked: ampCarousel, + onChange: function() { + props.setAttributes( { ampCarousel: ! ampCarousel } ); + } + } ); + return isSelected && ( + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: 'AMP Settings' }, + toggleControl + ) + ) + ); + }; + /** * Set up inspector controls for shortcode block. * Adds ampCarousel attribute in case of gallery shortcode. From 54f9555acb91d0939758f2d5d4e7162ca695d751 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Sat, 5 May 2018 16:50:30 +0300 Subject: [PATCH 02/21] Add amp-carousel class. --- assets/js/amp-editor-blocks.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index 330d9388fd5..07a03f26d87 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -381,13 +381,16 @@ var ampEditorBlocks = ( function() { ); } - // In case AMP Layout, add layout to classname. + // In case AMP attributes, add info to classname. if ( component.hasAmpLayoutSet( attributes || '' ) ) { ampClassName += ' amp-layout-' + attributes.ampLayout; } if ( component.hasAmpNoLoadingSet( attributes || '' ) ) { ampClassName += ' amp-noloading'; } + if ( component.hasAmpCarouselSet( attributes || '' ) ) { + ampClassName += ' amp-carousel'; + } if ( '' !== ampClassName && attributes.className !== ampClassName ) { props.className = ampClassName.trim(); @@ -399,6 +402,16 @@ var ampEditorBlocks = ( function() { return element; }; + /** + * Check if AMP Carousel is set. + * + * @param {Object} attributes Attributes. + * @returns {boolean} If is set. + */ + component.hasAmpCarouselSet = function hasAmpCarouselSet( attributes ) { + return attributes.ampCarousel && false !== attributes.ampCarousel; + }; + /** * Check if AMP NoLoading is set. * From ed1f9de2f17766bc1d046ef04ce24010b1f4cbc8 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Sat, 5 May 2018 20:58:49 +0300 Subject: [PATCH 03/21] Filter saving the post for when AMP Plugin gets disabled. Add tests. --- assets/js/amp-editor-blocks.js | 2 +- includes/admin/class-amp-editor-blocks.php | 10 +- includes/amp-helper-functions.php | 3 +- includes/class-amp-autoloader.php | 1 + .../sanitizers/class-amp-block-sanitizer.php | 2 +- .../class-amp-gallery-block-sanitizer.php | 91 +++++++++++++++++++ ...test-class-amp-gallery-block-sanitizer.php | 57 ++++++++++++ 7 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 includes/sanitizers/class-amp-gallery-block-sanitizer.php create mode 100644 tests/test-class-amp-gallery-block-sanitizer.php diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index 07a03f26d87..36ffcb42b9e 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -406,7 +406,7 @@ var ampEditorBlocks = ( function() { * Check if AMP Carousel is set. * * @param {Object} attributes Attributes. - * @returns {boolean} If is set. + * @return {boolean} If is set. */ component.hasAmpCarouselSet = function hasAmpCarouselSet( attributes ) { return attributes.ampCarousel && false !== attributes.ampCarousel; diff --git a/includes/admin/class-amp-editor-blocks.php b/includes/admin/class-amp-editor-blocks.php index f9b5c9fe569..7909a4a3d74 100644 --- a/includes/admin/class-amp-editor-blocks.php +++ b/includes/admin/class-amp-editor-blocks.php @@ -49,7 +49,13 @@ public static function filter_rest_pre_insert_post( $prepared_post ) { // Get the class of the figure. preg_match( "'
array(), 'AMP_Video_Sanitizer' => array(), 'AMP_Audio_Sanitizer' => array(), - 'AMP_Block_Sanitizer' => array(), 'AMP_Playbuzz_Sanitizer' => array(), 'AMP_Iframe_Sanitizer' => array( 'add_placeholder' => true, ), + 'AMP_Gallery_Block_Sanitizer' => array(), + 'AMP_Block_Sanitizer' => array(), // Note: Block sanitizer must come after embed / media sanitizers since it's logic is using the already sanitized content. 'AMP_Style_Sanitizer' => array(), 'AMP_Tag_And_Attribute_Sanitizer' => array(), // Note: This whitelist sanitizer must come at the end to clean up any remaining issues the other sanitizers didn't catch. ), diff --git a/includes/class-amp-autoloader.php b/includes/class-amp-autoloader.php index 651952cdfee..1ffe37bb408 100644 --- a/includes/class-amp-autoloader.php +++ b/includes/class-amp-autoloader.php @@ -72,6 +72,7 @@ class AMP_Autoloader { 'AMP_Base_Sanitizer' => 'includes/sanitizers/class-amp-base-sanitizer', 'AMP_Blacklist_Sanitizer' => 'includes/sanitizers/class-amp-blacklist-sanitizer', 'AMP_Block_Sanitizer' => 'includes/sanitizers/class-amp-block-sanitizer', + 'AMP_Gallery_Block_Sanitizer' => 'includes/sanitizers/class-amp-gallery-block-sanitizer', 'AMP_Iframe_Sanitizer' => 'includes/sanitizers/class-amp-iframe-sanitizer', 'AMP_Img_Sanitizer' => 'includes/sanitizers/class-amp-img-sanitizer', 'AMP_Comments_Sanitizer' => 'includes/sanitizers/class-amp-comments-sanitizer', diff --git a/includes/sanitizers/class-amp-block-sanitizer.php b/includes/sanitizers/class-amp-block-sanitizer.php index d5fca42e11c..35d1d1e29fb 100644 --- a/includes/sanitizers/class-amp-block-sanitizer.php +++ b/includes/sanitizers/class-amp-block-sanitizer.php @@ -47,7 +47,7 @@ public function sanitize() { continue; } - // We are looking for
elements with layout attribute only. + // We are looking for
elements with AMP classes only. if ( false === strpos( $attributes['class'], 'amp-layout-' ) && false === strpos( $attributes['class'], 'amp-noloading' ) ) { continue; } diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php new file mode 100644 index 00000000000..6976c56b77e --- /dev/null +++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php @@ -0,0 +1,91 @@ + element where necessary. + * + * @since 0.2 + */ + public function sanitize() { + $nodes = $this->dom->getElementsByTagName( self::$tag ); + $num_nodes = $nodes->length; + if ( 0 === $num_nodes ) { + return; + } + + for ( $i = $num_nodes - 1; $i >= 0; $i-- ) { + $node = $nodes->item( $i ); + + // We're looking for
    elements that at least one child. + if ( 0 === count( $node->childNodes ) ) { + continue; + } + + $attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node ); + + // We are only looking for
      elements which have amp-carousel as class. + if ( ! isset( $attributes['class'] ) || false === strpos( $attributes['class'], 'amp-carousel' ) ) { + continue; + } + + $images = $node->getElementsByTagName( 'a' ); + $num_images = $images->length; + + if ( 0 === $num_images ) { + continue; + } + + $attributes = array( + 'width' => self::FALLBACK_WIDTH, + 'height' => self::FALLBACK_HEIGHT, + 'type' => 'slides', + 'layout' => 'responsive', + ); + $amp_carousel = AMP_DOM_Utils::create_node( $this->dom, 'amp-carousel', $attributes ); + + for ( $j = $num_images - 1; $j >= 0; $j-- ) { + $amp_carousel->appendChild( $images->item( $j ) ); + } + + $node->parentNode->replaceChild( $amp_carousel, $node ); + $this->did_convert_elements = true; + } + } +} diff --git a/tests/test-class-amp-gallery-block-sanitizer.php b/tests/test-class-amp-gallery-block-sanitizer.php new file mode 100644 index 00000000000..3ca2c4262d2 --- /dev/null +++ b/tests/test-class-amp-gallery-block-sanitizer.php @@ -0,0 +1,57 @@ + array( + '

      Lorem Ipsum Demet Delorit.

      ', + '

      Lorem Ipsum Demet Delorit.

      ', + ), + + 'no_a' => array( + '', + '', + ), + + 'no_amp_carousel' => array( + '
      ', + '
      ', + ), + + 'data_amp_with_gallery' => array( + '', + '', + ), + ); + } + + /** + * Test sanitizer. + * + * @dataProvider get_data + * @param string $source Source. + * @param string $expected Expected. + */ + public function test_sanitizer( $source, $expected ) { + $dom = AMP_DOM_Utils::get_dom_from_content( $source ); + $sanitizer = new AMP_Gallery_Block_Sanitizer( $dom ); + $sanitizer->sanitize(); + $content = AMP_DOM_Utils::get_content_from_dom( $dom ); + $content = preg_replace( '/(?<=>)\s+(?=<)/', '', $content ); + $this->assertEquals( $expected, $content ); + } +} From 0263878b2b784abdadb75c894524da78b64d59c3 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Mon, 7 May 2018 13:37:51 +0300 Subject: [PATCH 04/21] Add lightbox logic for Image block. --- assets/js/amp-editor-blocks.js | 196 +++++++++++++----- .../sanitizers/class-amp-base-sanitizer.php | 34 +++ .../class-amp-gallery-block-sanitizer.php | 2 +- .../sanitizers/class-amp-img-sanitizer.php | 7 + 4 files changed, 188 insertions(+), 51 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index 36ffcb42b9e..a13729fd4cd 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -111,6 +111,16 @@ var ampEditorBlocks = ( function() { }; } + // Add AMP Lightbox settings. + if ( 'core/gallery' === name || 'core/image' === name ) { + if ( ! settings.attributes ) { + settings.attributes = {}; + } + settings.attributes.ampLightbox = { + type: 'boolean' + }; + } + // Layout settings for embeds and media blocks. if ( 0 === name.indexOf( 'core-embed' ) || -1 !== component.data.mediaBlocks.indexOf( name ) ) { if ( ! settings.attributes ) { @@ -165,6 +175,8 @@ var ampEditorBlocks = ( function() { } } else if ( 'core/gallery' === name ) { inspectorControls = component.setUpGalleryInpsectorControls( props ); + } else if ( 'core/image' === name ) { + inspectorControls = component.setUpImageInpsectorControls( props ); } else if ( -1 !== component.data.mediaBlocks.indexOf( name ) || 0 === name.indexOf( 'core-embed/' ) ) { inspectorControls = component.setUpInspectorControls( props ); } @@ -225,50 +237,140 @@ var ampEditorBlocks = ( function() { }; /** - * Default setup for inspector controls. + * Get AMP Layout select control. * * @param {Object} props Props. - * @return {Object|Element|*|{$$typeof, type, key, ref, props, _owner}} Inspector Controls. + * @returns {Object} Element. */ - component.setUpInspectorControls = function setUpInspectorControls( props ) { + component.getAmpLayoutControl = function getAmpLayoutControl( props ) { var ampLayout = props.attributes.ampLayout, - ampNoLoading = props.attributes.ampNoLoading, - isSelected = props.isSelected, - name = props.name, el = wp.element.createElement, - InspectorControls = wp.blocks.InspectorControls, SelectControl = wp.components.SelectControl, - ToggleControl = wp.components.ToggleControl, - PanelBody = wp.components.PanelBody, + name = props.name, label = 'AMP Layout'; if ( 'core/image' === name ) { label = 'AMP Layout (modifies width/height)'; } + return el( SelectControl, { + label: label, + value: ampLayout, + options: component.getLayoutOptions( name ), + onChange: function( value ) { + props.setAttributes( { ampLayout: value } ); + } + } ); + }; + + /** + * Get AMP Noloading toggle control. + * + * @param {Object} props Props. + * @returns {Object} Element. + */ + component.getAmpNoloadingToggle = function getAmpNoloadingToggle( props ) { + var ampNoLoading = props.attributes.ampNoLoading, + el = wp.element.createElement, + ToggleControl = wp.components.ToggleControl, + label = 'AMP Noloading'; + + return el( ToggleControl, { + label: label, + checked: ampNoLoading, + onChange: function() { + props.setAttributes( { ampNoLoading: ! ampNoLoading } ); + } + } ); + }; + + /** + * Get AMP Lightbox toggle control. + * + * @param {Object} props Props. + * @returns {Object} Element. + */ + component.getAmpLightboxToggle = function getAmpLightboxToggle( props ) { + var ampLightbox = props.attributes.ampLightbox, + el = wp.element.createElement, + ToggleControl = wp.components.ToggleControl, + label = 'Add lightbox effect'; + + return el( ToggleControl, { + label: label, + checked: ampLightbox, + onChange: function() { + props.setAttributes( { ampLightbox: ! ampLightbox } ); + } + } ); + }; + + /** + * Get AMP Carousel toggle control. + * + * @param {Object} props Props. + * @returns {Object} Element. + */ + component.getAmpCarouselToggle = function getAmpCarouselToggle( props ) { + var ampCarousel = props.attributes.ampCarousel, + el = wp.element.createElement, + ToggleControl = wp.components.ToggleControl, + label = 'Display as AMP carousel'; + + return el( ToggleControl, { + label: label, + checked: ampCarousel, + onChange: function() { + props.setAttributes( { ampCarousel: ! ampCarousel } ); + } + } ); + }; + + /** + * Default setup for inspector controls. + * + * @param {Object} props Props. + * @return {Object|Element|*|{$$typeof, type, key, ref, props, _owner}} Inspector Controls. + */ + component.setUpInspectorControls = function setUpInspectorControls( props ) { + var isSelected = props.isSelected, + el = wp.element.createElement, + InspectorControls = wp.blocks.InspectorControls, + PanelBody = wp.components.PanelBody; + return isSelected && ( el( InspectorControls, { key: 'inspector' }, el( PanelBody, { title: 'AMP Settings' }, - el( SelectControl, { - label: label, - value: ampLayout, - options: component.getLayoutOptions( name ), - onChange: function( value ) { - props.setAttributes( { ampLayout: value } ); - } - } ), - el( ToggleControl, { - label: 'AMP Noloading', - checked: ampNoLoading, - onChange: function() { - props.setAttributes( { ampNoLoading: ! ampNoLoading } ); - } - } ) + component.getAmpLayoutControl( props ), + component.getAmpNoloadingToggle( props ) ) ) ); }; + /** + * Set up inspector controls for Image block. + * + * @param props + * @returns {Object|Element|*|{$$typeof, type, key, ref, props, _owner}} + */ + component.setUpImageInpsectorControls = function setUpImageInpsectorControls( props ) { + var isSelected = props.isSelected, + el = wp.element.createElement, + InspectorControls = wp.blocks.InspectorControls, + PanelBody = wp.components.PanelBody; + + return isSelected && ( + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: 'AMP Settings' }, + component.getAmpLayoutControl( props ), + component.getAmpNoloadingToggle( props ), + component.getAmpLightboxToggle( props ) + ) + ) + ); + }; + /** * Set up inspector controls for Gallery block. * Adds ampCarousel attribute for displaying the output as amp-carousel. @@ -277,25 +379,16 @@ var ampEditorBlocks = ( function() { * @return {*} Inspector controls. */ component.setUpGalleryInpsectorControls = function setUpGalleryInpsectorControls( props ) { - var ampCarousel = props.attributes.ampCarousel, - isSelected = props.isSelected, + var isSelected = props.isSelected, el = wp.element.createElement, InspectorControls = wp.blocks.InspectorControls, - ToggleControl = wp.components.ToggleControl, - PanelBody = wp.components.PanelBody, - toggleControl; + PanelBody = wp.components.PanelBody; - toggleControl = el( ToggleControl, { - label: 'Display as AMP carousel', - checked: ampCarousel, - onChange: function() { - props.setAttributes( { ampCarousel: ! ampCarousel } ); - } - } ); return isSelected && ( el( InspectorControls, { key: 'inspector' }, el( PanelBody, { title: 'AMP Settings' }, - toggleControl + component.getAmpCarouselToggle( props ), + component.getAmpLightboxToggle( props ) ) ) ); @@ -309,26 +402,16 @@ var ampEditorBlocks = ( function() { * @return {*} Inspector controls. */ component.setUpShortcodeInspectorControls = function setUpShortcodeInspectorControls( props ) { - var ampCarousel = props.attributes.ampCarousel, - isSelected = props.isSelected, + var isSelected = props.isSelected, el = wp.element.createElement, InspectorControls = wp.blocks.InspectorControls, - ToggleControl = wp.components.ToggleControl, - PanelBody = wp.components.PanelBody, - toggleControl; + PanelBody = wp.components.PanelBody; if ( component.isGalleryShortcode( props.attributes ) ) { - toggleControl = el( ToggleControl, { - label: 'Display as AMP carousel', - checked: ampCarousel, - onChange: function() { - props.setAttributes( { ampCarousel: ! ampCarousel } ); - } - } ); return isSelected && ( el( InspectorControls, { key: 'inspector' }, el( PanelBody, { title: 'AMP Settings' }, - toggleControl + component.getAmpCarouselToggle( props ) ) ) ); @@ -391,6 +474,9 @@ var ampEditorBlocks = ( function() { if ( component.hasAmpCarouselSet( attributes || '' ) ) { ampClassName += ' amp-carousel'; } + if ( component.hasAmpLightboxSet( attributes || '' ) ) { + ampClassName += ' amp-lightbox'; + } if ( '' !== ampClassName && attributes.className !== ampClassName ) { props.className = ampClassName.trim(); @@ -402,6 +488,16 @@ var ampEditorBlocks = ( function() { return element; }; + /** + * Check if AMP Lightbox is set. + * + * @param {Object} attributes Attributes. + * @return {boolean} If is set. + */ + component.hasAmpLightboxSet = function hasAmpLightboxSet( attributes ) { + return attributes.ampLightbox && false !== attributes.ampLightbox; + }; + /** * Check if AMP Carousel is set. * diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index a7f5a57e480..7984d089d19 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -368,6 +368,8 @@ public function get_data_amp_attributes( $node ) { $attributes['layout'] = str_replace( 'amp-layout-', '', $class_name ); } elseif ( 'amp-noloading' === $class_name ) { $attributes['noloading'] = true; + } elseif ( 'amp-lightbox' === $class_name ) { + $attributes['lightbox'] = true; } } } @@ -390,6 +392,11 @@ public function set_data_amp_attributes( $attributes, $amp_data ) { if ( isset( $amp_data['noloading'] ) ) { $attributes['data-amp-noloading'] = ''; } + if ( isset( $amp_data['lightbox'] ) ) { + $attributes['data-amp-lightbox'] = ''; + $attributes['on'] = 'tap:amp-image-lightbox-1'; + $attributes['role'] = 'button'; + } return $attributes; } @@ -455,4 +462,31 @@ public function set_attachment_layout_attributes( $node, $new_attributes, $layou return $new_attributes; } + + /** + * Add element to body tag if it doesn't exist yet. + * + * @param array $attributes Attributes. + */ + public function maybe_add_amp_image_lightbox_node( $attributes ) { + if ( ! isset( $attributes['lightbox'] ) ) { + return; + } + + $nodes = $this->dom->getElementById( 'amp-image-lightbox-1' ); + if ( null !== $nodes ) { + return; + } + + $nodes = $this->dom->getElementsByTagName( 'body' ); + if ( ! $nodes->length ) { + return; + } + $body_node = $nodes->item( 0 ); + $amp_image_lightbox = AMP_DOM_Utils::create_node( $this->dom, 'amp-image-lightbox', array( + 'id' => 'amp-image-lightbox-1', + 'layout' => 'nodisplay', + ) ); + $body_node->appendChild( $amp_image_lightbox ); + } } diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php index 6976c56b77e..f03b421fb67 100644 --- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php +++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php @@ -78,7 +78,7 @@ public function sanitize() { 'type' => 'slides', 'layout' => 'responsive', ); - $amp_carousel = AMP_DOM_Utils::create_node( $this->dom, 'amp-carousel', $attributes ); + $amp_carousel = AMP_DOM_Utils::create_node( $this->dom, 'amp-lightbox-gallery', $attributes ); for ( $j = $num_images - 1; $j >= 0; $j-- ) { $amp_carousel->appendChild( $images->item( $j ) ); diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index 69232511980..1a835f9d0c4 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -120,6 +120,7 @@ private function filter_attributes( $attributes ) { case 'class': case 'srcset': case 'on': + case 'role': case 'attribution': $out[ $name ] = $value; break; @@ -137,6 +138,10 @@ private function filter_attributes( $attributes ) { $out['noloading'] = $value; break; + case 'data-amp-lightbox': + $out['lightbox'] = $value; + break; + default: break; } @@ -223,6 +228,8 @@ private function adjust_and_replace_node( $node ) { $new_attributes['layout'] = 'intrinsic'; } + $this->maybe_add_amp_image_lightbox_node( $new_attributes ); + if ( $this->is_gif_url( $new_attributes['src'] ) ) { $this->did_convert_elements = true; From 40486fef1346a800961e1a2899e37ada3959f72a Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Tue, 8 May 2018 15:27:52 +0300 Subject: [PATCH 05/21] Add lightbox for gallery block. --- assets/js/amp-editor-blocks.js | 69 ++++++------ includes/admin/class-amp-editor-blocks.php | 9 +- includes/amp-helper-functions.php | 2 +- .../sanitizers/class-amp-base-sanitizer.php | 79 ++++++++------ .../class-amp-gallery-block-sanitizer.php | 100 ++++++++++++++++-- .../sanitizers/class-amp-img-sanitizer.php | 20 +++- ...test-class-amp-gallery-block-sanitizer.php | 8 +- 7 files changed, 205 insertions(+), 82 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index a13729fd4cd..a17c7f0221e 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -1,6 +1,8 @@ /* exported ampEditorBlocks */ /* eslint no-magic-numbers: [ "error", { "ignore": [ 1, -1, 0 ] } ] */ +var __ = wp.i18n.__; + var ampEditorBlocks = ( function() { var component = { @@ -10,14 +12,14 @@ var ampEditorBlocks = ( function() { data: { dynamicBlocks: [], ampLayoutOptions: [ - { value: 'nodisplay', label: 'No Display' }, - { value: 'fixed', label: 'Fixed' }, // Not supported by amp-audio and amp-pixel. - { value: 'responsive', label: 'Responsive' }, // To ensure your AMP element displays, you must specify a width and height for the containing element. - { value: 'fixed-height', label: 'Fixed height' }, - { value: 'fill', label: 'Fill' }, - { value: 'container', label: 'Container' }, // Not supported by img and video. - { value: 'flex-item', label: 'Flex Item' }, - { value: 'intrinsic', label: 'Intrinsic' } // Not supported by video. + { value: 'nodisplay', label: __( 'No Display' ) }, + { value: 'fixed', label: __( 'Fixed' ) }, // Not supported by amp-audio and amp-pixel. + { value: 'responsive', label: __( 'Responsive' ) }, // To ensure your AMP element displays, you must specify a width and height for the containing element. + { value: 'fixed-height', label: __( 'Fixed height' ) }, + { value: 'fill', label: __( 'Fill' ) }, + { value: 'container', label: __( 'Container' ) }, // Not supported by img and video. + { value: 'flex-item', label: __( 'Flex Item' ) }, + { value: 'intrinsic', label: __( 'Intrinsic' ) } // Not supported by video. ], defaultWidth: 608, // Max-width in the editor. defaultHeight: 400, @@ -25,7 +27,8 @@ var ampEditorBlocks = ( function() { 'core/image', 'core/video', 'core/audio' - ] + ], + ampPanelLabel: __( 'AMP Settings' ) } }; @@ -240,17 +243,17 @@ var ampEditorBlocks = ( function() { * Get AMP Layout select control. * * @param {Object} props Props. - * @returns {Object} Element. + * @return {Object} Element. */ component.getAmpLayoutControl = function getAmpLayoutControl( props ) { var ampLayout = props.attributes.ampLayout, el = wp.element.createElement, SelectControl = wp.components.SelectControl, name = props.name, - label = 'AMP Layout'; + label = __( 'AMP Layout' ); if ( 'core/image' === name ) { - label = 'AMP Layout (modifies width/height)'; + label = __( 'AMP Layout (modifies width/height)' ); } return el( SelectControl, { @@ -267,13 +270,13 @@ var ampEditorBlocks = ( function() { * Get AMP Noloading toggle control. * * @param {Object} props Props. - * @returns {Object} Element. + * @return {Object} Element. */ component.getAmpNoloadingToggle = function getAmpNoloadingToggle( props ) { var ampNoLoading = props.attributes.ampNoLoading, el = wp.element.createElement, ToggleControl = wp.components.ToggleControl, - label = 'AMP Noloading'; + label = __( 'AMP Noloading' ); return el( ToggleControl, { label: label, @@ -288,19 +291,23 @@ var ampEditorBlocks = ( function() { * Get AMP Lightbox toggle control. * * @param {Object} props Props. - * @returns {Object} Element. + * @return {Object} Element. */ component.getAmpLightboxToggle = function getAmpLightboxToggle( props ) { var ampLightbox = props.attributes.ampLightbox, el = wp.element.createElement, ToggleControl = wp.components.ToggleControl, - label = 'Add lightbox effect'; + label = __( 'Add lightbox effect' ); return el( ToggleControl, { label: label, checked: ampLightbox, - onChange: function() { + onChange: function( nextValue ) { props.setAttributes( { ampLightbox: ! ampLightbox } ); + // In case of lightbox set linking images to 'none'. + if ( nextValue && props.attributes.linkTo && 'none' !== props.attributes.linkTo ) { + props.setAttributes( { linkTo: 'none' } ); + } } } ); }; @@ -309,13 +316,13 @@ var ampEditorBlocks = ( function() { * Get AMP Carousel toggle control. * * @param {Object} props Props. - * @returns {Object} Element. + * @return {Object} Element. */ component.getAmpCarouselToggle = function getAmpCarouselToggle( props ) { var ampCarousel = props.attributes.ampCarousel, el = wp.element.createElement, ToggleControl = wp.components.ToggleControl, - label = 'Display as AMP carousel'; + label = __( 'Display as AMP carousel' ); return el( ToggleControl, { label: label, @@ -340,7 +347,7 @@ var ampEditorBlocks = ( function() { return isSelected && ( el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: 'AMP Settings' }, + el( PanelBody, { title: component.data.ampPanelLabel }, component.getAmpLayoutControl( props ), component.getAmpNoloadingToggle( props ) ) @@ -351,8 +358,8 @@ var ampEditorBlocks = ( function() { /** * Set up inspector controls for Image block. * - * @param props - * @returns {Object|Element|*|{$$typeof, type, key, ref, props, _owner}} + * @param {Object} props Props. + * @return {Object} Inspector Controls. */ component.setUpImageInpsectorControls = function setUpImageInpsectorControls( props ) { var isSelected = props.isSelected, @@ -361,14 +368,14 @@ var ampEditorBlocks = ( function() { PanelBody = wp.components.PanelBody; return isSelected && ( - el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: 'AMP Settings' }, - component.getAmpLayoutControl( props ), - component.getAmpNoloadingToggle( props ), - component.getAmpLightboxToggle( props ) - ) + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: component.data.ampPanelLabel }, + component.getAmpLayoutControl( props ), + component.getAmpNoloadingToggle( props ), + component.getAmpLightboxToggle( props ) ) - ); + ) + ); }; /** @@ -386,7 +393,7 @@ var ampEditorBlocks = ( function() { return isSelected && ( el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: 'AMP Settings' }, + el( PanelBody, { title: component.data.ampPanelLabel }, component.getAmpCarouselToggle( props ), component.getAmpLightboxToggle( props ) ) @@ -410,7 +417,7 @@ var ampEditorBlocks = ( function() { if ( component.isGalleryShortcode( props.attributes ) ) { return isSelected && ( el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: 'AMP Settings' }, + el( PanelBody, { title: component.data.ampPanelLabel }, component.getAmpCarouselToggle( props ) ) ) diff --git a/includes/admin/class-amp-editor-blocks.php b/includes/admin/class-amp-editor-blocks.php index 7909a4a3d74..6cdb80a1814 100644 --- a/includes/admin/class-amp-editor-blocks.php +++ b/includes/admin/class-amp-editor-blocks.php @@ -17,7 +17,7 @@ class AMP_Editor_Blocks { public function init() { if ( function_exists( 'gutenberg_init' ) ) { add_action( 'admin_enqueue_scripts', array( $this, 'add_editor_filters' ) ); - add_filter( 'wp_kses_allowed_html', array( $this, 'whitelist_layout_in_wp_kses_allowed_html' ), 10 ); + add_filter( 'wp_kses_allowed_html', array( $this, 'whitelist_block_atts_in_wp_kses_allowed_html' ), 10 ); } } @@ -96,10 +96,11 @@ public static function filter_rest_pre_insert_post( $prepared_post ) { * @param array $context Array of contexts. * @return mixed Modified array. */ - public function whitelist_layout_in_wp_kses_allowed_html( $context ) { + public function whitelist_block_atts_in_wp_kses_allowed_html( $context ) { foreach ( $context as $tag ) { - $tag['data-amp-layout'] = true; - $tag['data-amp-noloading'] = true; + $tag['data-amp-layout'] = true; + $tag['data-amp-noloading'] = true; + $tag['data-close-button-aria-label'] = true; } return $context; } diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 1f639af1e62..5050a6929b3 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -488,7 +488,7 @@ function amp_get_content_sanitizers( $post = null ) { 'AMP_Iframe_Sanitizer' => array( 'add_placeholder' => true, ), - 'AMP_Gallery_Block_Sanitizer' => array(), + 'AMP_Gallery_Block_Sanitizer' => array(), // Note: Gallery block sanitizer must come after image sanitizers since itś logic is using the already sanitized images. 'AMP_Block_Sanitizer' => array(), // Note: Block sanitizer must come after embed / media sanitizers since it's logic is using the already sanitized content. 'AMP_Style_Sanitizer' => array(), 'AMP_Tag_And_Attribute_Sanitizer' => array(), // Note: This whitelist sanitizer must come at the end to clean up any remaining issues the other sanitizers didn't catch. diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index 7984d089d19..464c8d72bae 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -19,6 +19,15 @@ abstract class AMP_Base_Sanitizer { */ const FALLBACK_HEIGHT = 400; + /** + * Value for ID. + * + * @since 1.0 + * + * @const string + */ + const AMP_IMAGE_LIGHTBOX_ID = 'amp-image-lightbox-1'; + /** * Placeholder for default args, to be set in child classes. * @@ -394,12 +403,44 @@ public function set_data_amp_attributes( $attributes, $amp_data ) { } if ( isset( $amp_data['lightbox'] ) ) { $attributes['data-amp-lightbox'] = ''; - $attributes['on'] = 'tap:amp-image-lightbox-1'; + $attributes['on'] = 'tap:' . self::AMP_IMAGE_LIGHTBOX_ID; $attributes['role'] = 'button'; + $attributes['tabindex'] = 0; } return $attributes; } + /** + * Sets width and height by src for attributes. + * + * @param string $src Src. + * @param array $attributes Array of attributes. + */ + public function set_attachment_width_and_height_by_src( $src, &$attributes ) { + + // Get the width and height from the file. + $ext = pathinfo( $src, PATHINFO_EXTENSION ); + $name = wp_basename( $src, ".$ext" ); + $args = array( + 'name' => $name, + 'post_type' => 'attachment', + 'post_status' => 'inherit', + 'numberposts' => 1, + ); + + $attachment = get_posts( $args ); + + if ( ! empty( $attachment ) ) { + $meta_data = wp_get_attachment_metadata( $attachment[0]->ID ); + if ( ! is_numeric( $attributes['width'] ) && is_numeric( $meta_data['width'] ) ) { + $attributes['width'] = $meta_data['width']; + } + if ( ! is_numeric( $attributes['height'] ) && is_numeric( $meta_data['height'] ) ) { + $attributes['height'] = $meta_data['height']; + } + } + } + /** * Set attributes to node's parent element according to layout. * @@ -412,28 +453,7 @@ public function set_attachment_layout_attributes( $node, $new_attributes, $layou // If either height or width is missing, try to get these from original file. if ( empty( $new_attributes['width'] ) || empty( $new_attributes['height'] ) ) { - - // Get the width and height from the file. - $ext = pathinfo( $new_attributes['src'], PATHINFO_EXTENSION ); - $name = wp_basename( $new_attributes['src'], ".$ext" ); - $args = array( - 'name' => $name, - 'post_type' => 'attachment', - 'post_status' => 'inherit', - 'numberposts' => 1, - ); - - $attachment = get_posts( $args ); - - if ( ! empty( $attachment ) ) { - $meta_data = wp_get_attachment_metadata( $attachment[0]->ID ); - if ( empty( $new_attributes['width'] ) && ! empty( $meta_data['width'] ) ) { - $new_attributes['width'] = $meta_data['width']; - } - if ( empty( $new_attributes['height'] ) && ! empty( $meta_data['height'] ) ) { - $new_attributes['height'] = $meta_data['height']; - } - } + $this->set_attachment_width_and_height_by_src( $new_attributes['src'], $new_attributes ); } // The width has to be unset / auto in case of fixed-height. @@ -465,15 +485,10 @@ public function set_attachment_layout_attributes( $node, $new_attributes, $layou /** * Add element to body tag if it doesn't exist yet. - * - * @param array $attributes Attributes. */ - public function maybe_add_amp_image_lightbox_node( $attributes ) { - if ( ! isset( $attributes['lightbox'] ) ) { - return; - } + public function maybe_add_amp_image_lightbox_node() { - $nodes = $this->dom->getElementById( 'amp-image-lightbox-1' ); + $nodes = $this->dom->getElementById( self::AMP_IMAGE_LIGHTBOX_ID ); if ( null !== $nodes ) { return; } @@ -482,9 +497,9 @@ public function maybe_add_amp_image_lightbox_node( $attributes ) { if ( ! $nodes->length ) { return; } - $body_node = $nodes->item( 0 ); + $body_node = $nodes->item( 0 ); $amp_image_lightbox = AMP_DOM_Utils::create_node( $this->dom, 'amp-image-lightbox', array( - 'id' => 'amp-image-lightbox-1', + 'id' => self::AMP_IMAGE_LIGHTBOX_ID, 'layout' => 'nodisplay', ) ); $body_node->appendChild( $amp_image_lightbox ); diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php index f03b421fb67..4ef4d32e8c9 100644 --- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php +++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php @@ -59,33 +59,115 @@ public function sanitize() { } $attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node ); + if ( ! isset( $attributes['class'] ) ) { + continue; + } + $is_amp_lightbox = false !== strpos( $attributes['class'], 'amp-lightbox' ); + $is_amp_carousel = false !== strpos( $attributes['class'], 'amp-carousel' ); // We are only looking for
        elements which have amp-carousel as class. - if ( ! isset( $attributes['class'] ) || false === strpos( $attributes['class'], 'amp-carousel' ) ) { + if ( ! isset( $attributes['class'] ) || ( ! $is_amp_carousel && ! $is_amp_lightbox ) ) { continue; } - $images = $node->getElementsByTagName( 'a' ); - $num_images = $images->length; + // If lightbox is set, we should add lightbox feature to the gallery images. + if ( $is_amp_lightbox ) { + $this->add_lightbox_attributes_to_image_nodes( $node ); + $this->maybe_add_amp_image_lightbox_node(); + } + + // If amp-carousel is not set, nothing else to do here. + if ( ! $is_amp_carousel ) { + continue; + } + + $num_images = 0; + + // If it's not AMP lightbox, look for links first. + if ( ! $is_amp_lightbox ) { + $images = $node->getElementsByTagName( 'a' ); + $num_images = $images->length; + } + + if ( 0 === $num_images ) { + + // If not linking to anything then look for . + $images = $node->getElementsByTagName( 'amp-img' ); + $num_images = $images->length; + } if ( 0 === $num_images ) { continue; } - $attributes = array( - 'width' => self::FALLBACK_WIDTH, - 'height' => self::FALLBACK_HEIGHT, + $carousel_height = $this->get_carousel_height( $node ); + $carousel_attributes = array( + 'height' => $carousel_height, 'type' => 'slides', - 'layout' => 'responsive', + 'layout' => 'fixed-height', ); - $amp_carousel = AMP_DOM_Utils::create_node( $this->dom, 'amp-lightbox-gallery', $attributes ); + $amp_carousel = AMP_DOM_Utils::create_node( $this->dom, 'amp-carousel', $carousel_attributes ); for ( $j = $num_images - 1; $j >= 0; $j-- ) { $amp_carousel->appendChild( $images->item( $j ) ); } $node->parentNode->replaceChild( $amp_carousel, $node ); - $this->did_convert_elements = true; + } + $this->did_convert_elements = true; + } + + /** + * Get carousel height by containing images. + * + * @param DOMNode $node Node
          . + * @return int + */ + protected function get_carousel_height( $node ) { + $images = $node->getElementsByTagName( 'amp-img' ); + $num_images = $images->length; + $height = false; + if ( 0 === $num_images ) { + return self::FALLBACK_HEIGHT; + } + for ( $i = $num_images - 1; $i >= 0; $i-- ) { + $image = $images->item( $i ); + $image_height = $image->getAttribute( 'height' ); + if ( ! $image_height || ! is_numeric( $image_height ) ) { + continue; + } + if ( ! $height ) { + $height = $image_height; + } elseif ( $height > $image_height ) { + $height = $image_height; + } + } + + return false === $height ? self::FALLBACK_HEIGHT : $height; + } + + /** + * Set lightbox related attributes to within gallery. + * + * @param DOMNode $node
            node. + */ + protected function add_lightbox_attributes_to_image_nodes( $node ) { + $images = $node->getElementsByTagName( 'amp-img' ); + $num_images = $images->length; + if ( 0 === $num_images ) { + return; + } + $attributes = array( + 'data-amp-lightbox' => '', + 'on' => 'tap:' . self::AMP_IMAGE_LIGHTBOX_ID, + 'role' => 'button', + ); + + for ( $j = $num_images - 1; $j >= 0; $j-- ) { + $image_node = $images->item( $j ); + foreach ( $attributes as $att => $value ) { + $image_node->setAttribute( $att, $value ); + } } } } diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index 1a835f9d0c4..6cee2880b86 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -121,6 +121,7 @@ private function filter_attributes( $attributes ) { case 'srcset': case 'on': case 'role': + case 'tabindex': case 'attribution': $out[ $name ] = $value; break; @@ -166,6 +167,21 @@ private function determine_dimensions( $need_dimensions ) { if ( ! $node instanceof DOMElement ) { continue; } + + if ( + ! is_numeric( $node->getAttribute( 'width' ) ) || + ! is_numeric( $node->getAttribute( 'height' ) ) + ) { + $attributes = array( + 'width' => $node->getAttribute( 'width' ), + 'height' => $node->getAttribute( 'height' ), + ); + $this->set_attachment_width_and_height_by_src( $node->getAttribute( 'src' ), $attributes ); + $node->setAttribute( 'width', $attributes['width'] ); + $node->setAttribute( 'height', $attributes['height'] ); + } + + // Set fallback dimensions if needed. if ( ! is_numeric( $node->getAttribute( 'width' ) ) && ! is_numeric( $node->getAttribute( 'height' ) ) @@ -228,7 +244,9 @@ private function adjust_and_replace_node( $node ) { $new_attributes['layout'] = 'intrinsic'; } - $this->maybe_add_amp_image_lightbox_node( $new_attributes ); + if ( isset( $new_attributes['lightbox'] ) ) { + $this->maybe_add_amp_image_lightbox_node(); + } if ( $this->is_gif_url( $new_attributes['src'] ) ) { $this->did_convert_elements = true; diff --git a/tests/test-class-amp-gallery-block-sanitizer.php b/tests/test-class-amp-gallery-block-sanitizer.php index 3ca2c4262d2..979db9062a5 100644 --- a/tests/test-class-amp-gallery-block-sanitizer.php +++ b/tests/test-class-amp-gallery-block-sanitizer.php @@ -22,9 +22,9 @@ public function get_data() { '

            Lorem Ipsum Demet Delorit.

            ', ), - 'no_a' => array( - '', - '', + 'no_a_no_amp_img' => array( + '', + '', ), 'no_amp_carousel' => array( @@ -34,7 +34,7 @@ public function get_data() { 'data_amp_with_gallery' => array( '', - '', + '', ), ); } From 96bd515620dc633d9f7b5ecbef4a0066f532e13e Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Tue, 8 May 2018 16:05:20 +0300 Subject: [PATCH 06/21] Fix CSS. --- assets/css/amp-default.css | 4 ++++ includes/sanitizers/class-amp-base-sanitizer.php | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/assets/css/amp-default.css b/assets/css/amp-default.css index d745dd9b186..692d3f8ace8 100644 --- a/assets/css/amp-default.css +++ b/assets/css/amp-default.css @@ -9,3 +9,7 @@ /** Force the image into a box of fixed dimensions and use object-fit to scale. **/ object-fit: contain; } + +#amp-image-lightbox-1 img { + object-fit: contain; +} diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index 464c8d72bae..2447123bbc7 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -499,8 +499,9 @@ public function maybe_add_amp_image_lightbox_node() { } $body_node = $nodes->item( 0 ); $amp_image_lightbox = AMP_DOM_Utils::create_node( $this->dom, 'amp-image-lightbox', array( - 'id' => self::AMP_IMAGE_LIGHTBOX_ID, - 'layout' => 'nodisplay', + 'id' => self::AMP_IMAGE_LIGHTBOX_ID, + 'layout' => 'nodisplay', + 'data-close-button-aria-label' => __( 'Close', 'amp' ), ) ); $body_node->appendChild( $amp_image_lightbox ); } From 22060ec2e89ded8ea4827fa440813055198d1b44 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Tue, 8 May 2018 20:53:05 +0300 Subject: [PATCH 07/21] Add block-related JS with blocks filter instead of admin filter. --- includes/admin/class-amp-editor-blocks.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/admin/class-amp-editor-blocks.php b/includes/admin/class-amp-editor-blocks.php index 6cdb80a1814..582504b513d 100644 --- a/includes/admin/class-amp-editor-blocks.php +++ b/includes/admin/class-amp-editor-blocks.php @@ -16,7 +16,7 @@ class AMP_Editor_Blocks { */ public function init() { if ( function_exists( 'gutenberg_init' ) ) { - add_action( 'admin_enqueue_scripts', array( $this, 'add_editor_filters' ) ); + add_action( 'enqueue_block_editor_assets', array( $this, 'add_editor_filters' ) ); add_filter( 'wp_kses_allowed_html', array( $this, 'whitelist_block_atts_in_wp_kses_allowed_html' ), 10 ); } } @@ -113,7 +113,7 @@ public function add_editor_filters() { wp_enqueue_script( 'amp-editor-blocks', amp_get_asset_url( 'js/amp-editor-blocks.js' ), - array( 'amp-runtime', 'underscore', 'wp-hooks' ), + array( 'amp-runtime', 'underscore', 'wp-hooks', 'wp-components' ), AMP__VERSION, true ); From 5b63476c5510db23a872ac187b862f7533f041a6 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Wed, 9 May 2018 12:54:45 +0300 Subject: [PATCH 08/21] Fix issue with assigning props directly. --- assets/js/amp-editor-blocks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index a17c7f0221e..0453cb5edaa 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -486,7 +486,7 @@ var ampEditorBlocks = ( function() { } if ( '' !== ampClassName && attributes.className !== ampClassName ) { - props.className = ampClassName.trim(); + props = _.extend( {}, props, { className: ampClassName.trim() } ); return wp.element.createElement( element.type, props From 2e102eaa495b91f9fd4602d6bf9defb241a2dcd2 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Wed, 9 May 2018 15:48:00 +0300 Subject: [PATCH 09/21] Add lightbox to gallery shortcode block. --- assets/js/amp-editor-blocks.js | 94 ++++++++++++++----- includes/admin/class-amp-editor-blocks.php | 2 +- includes/embeds/class-amp-gallery-embed.php | 60 +++++++++--- ...test-class-amp-gallery-block-sanitizer.php | 18 +++- 4 files changed, 133 insertions(+), 41 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index 0453cb5edaa..63c5446f595 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -112,10 +112,13 @@ var ampEditorBlocks = ( function() { settings.attributes.ampCarousel = { type: 'boolean' }; + settings.attributes.ampLightbox = { + type: 'boolean' + }; } // Add AMP Lightbox settings. - if ( 'core/gallery' === name || 'core/image' === name ) { + if ( 'core/image' === name ) { if ( ! settings.attributes ) { settings.attributes = {}; } @@ -162,10 +165,14 @@ var ampEditorBlocks = ( function() { } if ( 'core/shortcode' === name ) { - // Lets remove amp-carousel from from edit view. + // Lets remove amp-carousel from edit view. if ( component.hasGalleryShortcodeCarouselAttribute( attributes.text || '' ) ) { props.setAttributes( { text: component.removeAmpCarouselFromShortcodeAtts( attributes.text ) } ); } + // Lets remove amp-lightbox from edit view. + if ( component.hasGalleryShortcodeLightboxAttribute( attributes.text || '' ) ) { + props.setAttributes( { text: component.removeAmpLightboxFromShortcodeAtts( attributes.text ) } ); + } inspectorControls = component.setUpShortcodeInspectorControls( props ); if ( '' === inspectorControls ) { @@ -418,7 +425,8 @@ var ampEditorBlocks = ( function() { return isSelected && ( el( InspectorControls, { key: 'inspector' }, el( PanelBody, { title: component.data.ampPanelLabel }, - component.getAmpCarouselToggle( props ) + component.getAmpCarouselToggle( props ), + component.getAmpLightboxToggle( props ) ) ) ); @@ -439,36 +447,54 @@ var ampEditorBlocks = ( function() { var text, ampClassName = element.props.className || '', props = element.props; + if ( 'core/shortcode' === blockType.name && component.isGalleryShortcode( attributes ) ) { + if ( ! attributes.ampLightbox ) { + if ( component.hasGalleryShortcodeLightboxAttribute( attributes.text || '' ) ) { + text = component.removeAmpLightboxFromShortcodeAtts( attributes.text ); + } + } else { + text = attributes.text || ''; + } if ( attributes.ampCarousel ) { - // If the text contains amp-carousel, lets remove it. - if ( component.hasGalleryShortcodeCarouselAttribute( attributes.text || '' ) ) { - text = component.removeAmpCarouselFromShortcodeAtts( attributes.text ); - - return wp.element.createElement( - wp.element.RawHTML, - {}, - text - ); + // If the text contains amp-carousel or amp-lightbox, lets remove it. + if ( component.hasGalleryShortcodeCarouselAttribute( text ) ) { + text = component.removeAmpCarouselFromShortcodeAtts( text ); } - // Else lets return original. - return element; + // If lightbox is not set, we can return here. + if ( ! attributes.ampLightbox ) { + if ( attributes.text !== text ) { + return wp.element.createElement( + wp.element.RawHTML, + {}, + text + ); + } + + // Else lets return original. + return element; + } + } else if ( ! component.hasGalleryShortcodeCarouselAttribute( attributes.text || '' ) ) { + // Add amp-carousel=false attribut to the shortcode. + text = attributes.text.replace( '[gallery', '[gallery amp-carousel=false' ); + } else { + text = attributes.text; } - // If the text already contains amp-carousel, return original. - if ( component.hasGalleryShortcodeCarouselAttribute( attributes.text || '' ) ) { - return element; + if ( attributes.ampLightbox && ! component.hasGalleryShortcodeLightboxAttribute( text ) ) { + text = text.replace( '[gallery', '[gallery amp-lightbox=true' ); } - // Add amp-carousel=false attribut to the shortcode. - text = attributes.text.replace( '[gallery', '[gallery amp-carousel=false' ); + if ( attributes.text !== text ) { + return wp.element.createElement( + wp.element.RawHTML, + {}, + text + ); + } - return wp.element.createElement( - wp.element.RawHTML, - {}, - text - ); + return element; } // In case AMP attributes, add info to classname. @@ -545,6 +571,16 @@ var ampEditorBlocks = ( function() { return shortcode.replace( ' amp-carousel=false', '' ); }; + /** + * Removes amp-lightbox=true from attributes. + * + * @param {string} shortcode Shortcode text. + * @return {string} Modified shortcode. + */ + component.removeAmpLightboxFromShortcodeAtts = function removeAmpLightboxFromShortcodeAtts( shortcode ) { + return shortcode.replace( ' amp-lightbox=true', '' ); + }; + /** * Check if shortcode includes amp-carousel attribute. * @@ -555,6 +591,16 @@ var ampEditorBlocks = ( function() { return -1 !== text.indexOf( 'amp-carousel=false' ); }; + /** + * Check if shortcode includes amp-lightbox attribute. + * + * @param {string} text Shortcode. + * @return {boolean} If has amp-lightbox. + */ + component.hasGalleryShortcodeLightboxAttribute = function hasGalleryShortcodeLightboxAttribute( text ) { + return -1 !== text.indexOf( 'amp-lightbox=true' ); + }; + /** * Check if className has AMP attributes in it. * diff --git a/includes/admin/class-amp-editor-blocks.php b/includes/admin/class-amp-editor-blocks.php index 582504b513d..3f16752f6ff 100644 --- a/includes/admin/class-amp-editor-blocks.php +++ b/includes/admin/class-amp-editor-blocks.php @@ -55,7 +55,7 @@ public static function filter_rest_pre_insert_post( $prepared_post ) { preg_match( "'
              'none', ), $attr, 'gallery' ); + if ( ! empty( $attr['amp-lightbox'] ) ) { + $atts['lightbox'] = filter_var( $attr['amp-lightbox'], FILTER_VALIDATE_BOOLEAN ); + } + $id = intval( $atts['id'] ); if ( ! empty( $atts['include'] ) ) { @@ -99,10 +103,12 @@ public function shortcode( $attr ) { } $href = null; - if ( ! empty( $atts['link'] ) && 'file' === $atts['link'] ) { - $href = $url; - } elseif ( ! empty( $atts['link'] ) && 'post' === $atts['link'] ) { - $href = get_attachment_link( $attachment_id ); + if ( empty( $atts['lightbox'] ) ) { + if ( ! empty( $atts['link'] ) && 'file' === $atts['link'] ) { + $href = $url; + } elseif ( ! empty( $atts['link'] ) && 'post' === $atts['link'] ) { + $href = get_attachment_link( $attachment_id ); + } } $urls[] = array( @@ -113,9 +119,24 @@ public function shortcode( $attr ) { ); } - return $this->render( array( + $args = array( 'images' => $urls, - ) ); + ); + if ( ! empty( $atts['lightbox'] ) ) { + $args['lightbox'] = true; + $lightbox_tag = AMP_HTML_Utils::build_tag( + 'amp-image-lightbox', + array( + 'id' => AMP_Base_Sanitizer::AMP_IMAGE_LIGHTBOX_ID, + 'layout' => 'nodisplay', + 'data-close-button-aria-label' => __( 'Close', 'amp' ), + ) + ); + /* We need to add lightbox tag, too. @todo Could there be a better alternative for this? */ + return $this->render( $args ) . $lightbox_tag; + } + + return $this->render( $args ); } /** @@ -129,7 +150,15 @@ public function shortcode( $attr ) { * @return string $html Markup for the gallery. */ public function maybe_override_gallery( $html, $attributes ) { + $is_lightbox = isset( $attributes['amp-lightbox'] ) && true === filter_var( $attributes['amp-lightbox'], FILTER_VALIDATE_BOOLEAN ); if ( isset( $attributes['amp-carousel'] ) && false === filter_var( $attributes['amp-carousel'], FILTER_VALIDATE_BOOLEAN ) ) { + if ( true === $is_lightbox ) { + remove_filter( 'post_gallery', array( $this, 'maybe_override_gallery' ), 10 ); + $attributes['link'] = 'none'; + $html = '
                ' . gallery_shortcode( $attributes ) . '
              '; + add_filter( 'post_gallery', array( $this, 'maybe_override_gallery' ), 10, 2 ); + } + return $html; } return $this->shortcode( $attributes ); @@ -154,14 +183,21 @@ public function render( $args ) { $images = array(); foreach ( $args['images'] as $props ) { + $image_atts = array( + 'src' => $props['url'], + 'width' => $props['width'], + 'height' => $props['height'], + 'layout' => 'responsive', + ); + if ( ! empty( $args['lightbox'] ) ) { + $image_atts['lightbox'] = ''; + $image_atts['on'] = 'tap:' . AMP_Img_Sanitizer::AMP_IMAGE_LIGHTBOX_ID; + $image_atts['role'] = 'button'; + $image_atts['tabindex'] = 0; + } $image = AMP_HTML_Utils::build_tag( 'amp-img', - array( - 'src' => $props['url'], - 'width' => $props['width'], - 'height' => $props['height'], - 'layout' => 'responsive', - ) + $image_atts ); if ( ! empty( $props['href'] ) ) { diff --git a/tests/test-class-amp-gallery-block-sanitizer.php b/tests/test-class-amp-gallery-block-sanitizer.php index 979db9062a5..80c51831088 100644 --- a/tests/test-class-amp-gallery-block-sanitizer.php +++ b/tests/test-class-amp-gallery-block-sanitizer.php @@ -17,25 +17,35 @@ class AMP_Gallery_Block_Sanitizer_Test extends WP_UnitTestCase { */ public function get_data() { return array( - 'no_ul' => array( + 'no_ul' => array( '

              Lorem Ipsum Demet Delorit.

              ', '

              Lorem Ipsum Demet Delorit.

              ', ), - 'no_a_no_amp_img' => array( + 'no_a_no_amp_img' => array( '', '', ), - 'no_amp_carousel' => array( + 'no_amp_carousel' => array( '
              ', '
              ', ), - 'data_amp_with_gallery' => array( + 'data_amp_with_carousel' => array( '', '', ), + + 'data_amp_with_lightbox' => array( + '
              ', + '
              ', + ), + + 'data_amp_with_lightbox_and_carousel' => array( + '', + '', + ), ); } From ca133a44b61efd30bc71bd56d4535a6fe271f8a8 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Wed, 16 May 2018 13:43:18 +0300 Subject: [PATCH 10/21] Adjust logic to using data- attributes. --- assets/css/amp-default.css | 4 ++ assets/js/amp-editor-blocks.js | 37 ++++++++++++++++--- .../sanitizers/class-amp-block-sanitizer.php | 3 -- .../class-amp-gallery-block-sanitizer.php | 13 +++---- .../sanitizers/class-amp-img-sanitizer.php | 12 ++---- ...test-class-amp-gallery-block-sanitizer.php | 8 ++-- 6 files changed, 48 insertions(+), 29 deletions(-) diff --git a/assets/css/amp-default.css b/assets/css/amp-default.css index 692d3f8ace8..024aea38906 100644 --- a/assets/css/amp-default.css +++ b/assets/css/amp-default.css @@ -13,3 +13,7 @@ #amp-image-lightbox-1 img { object-fit: contain; } + +.entry__content .wp-block-gallery[class*="columns-"] figure { + width: 100%; +} diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index d00b4ba6d20..d22d7d7b766 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -107,7 +107,18 @@ var ampEditorBlocks = ( function() { */ component.addAMPExtraProps = function addAMPExtraProps( props, blockType, attributes ) { var ampAttributes = {}; - if ( ! attributes.ampLayout && ! attributes.ampNoLoading ) { + + // Shortcode props are handled differently. + if ( 'core/shortcode' === blockType ) { + return props; + } + + if ( + ! attributes.ampLayout && + ! attributes.ampNoLoading && + ! attributes.ampLightbox && + ! attributes.ampCarousel + ) { return props; } @@ -117,6 +128,12 @@ var ampEditorBlocks = ( function() { if ( attributes.ampNoLoading ) { ampAttributes[ 'data-amp-noloading' ] = attributes.ampNoLoading; } + if ( attributes.ampLightbox ) { + ampAttributes[ 'data-amp-lightbox' ] = attributes.ampLightbox; + } + if ( attributes.ampCarousel ) { + ampAttributes[ 'data-amp-carousel' ] = attributes.ampLightbox; + } return _.assign( ampAttributes, props ); }; @@ -248,6 +265,10 @@ var ampEditorBlocks = ( function() { if ( ! attributes.height ) { props.setAttributes( { height: component.data.defaultHeight } ); } + // Lightbox doesn't work with fixed height, so unset it. + if ( attributes.ampLightbox ) { + props.setAttributes( { ampLightbox: false } ); + } break; case 'fixed': @@ -331,9 +352,15 @@ var ampEditorBlocks = ( function() { checked: ampLightbox, onChange: function( nextValue ) { props.setAttributes( { ampLightbox: ! ampLightbox } ); - // In case of lightbox set linking images to 'none'. - if ( nextValue && props.attributes.linkTo && 'none' !== props.attributes.linkTo ) { - props.setAttributes( { linkTo: 'none' } ); + if ( nextValue ) { + // Lightbox doesn't work with fixed height, so change. + if ( 'fixed-height' === props.attributes.ampLayout ) { + props.setAttributes( { ampLayout: 'fixed' } ); + } + // In case of lightbox set linking images to 'none'. + if ( props.attributes.linkTo && 'none' !== props.attributes.linkTo ) { + props.setAttributes( { linkTo: 'none' } ); + } } } } ); @@ -510,8 +537,6 @@ var ampEditorBlocks = ( function() { text ); } - - } return element; }; diff --git a/includes/sanitizers/class-amp-block-sanitizer.php b/includes/sanitizers/class-amp-block-sanitizer.php index a5ad41dd7bd..1cf0c0eff89 100644 --- a/includes/sanitizers/class-amp-block-sanitizer.php +++ b/includes/sanitizers/class-amp-block-sanitizer.php @@ -91,9 +91,6 @@ protected function set_attributes( $node, $parent_node, $attributes ) { if ( isset( $attributes['data-amp-noloading'] ) && true === filter_var( $attributes['data-amp-noloading'], FILTER_VALIDATE_BOOLEAN ) ) { $node->setAttribute( 'noloading', '' ); } - if ( isset( $attributes['data-amp-lightbox'] ) && true === filter_var( $attributes['data-amp-lightbox'], FILTER_VALIDATE_BOOLEAN ) ) { - $node->setAttribute( 'lightbox', '' ); - } $layout = $node->getAttribute( 'layout' ); diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php index 4ef4d32e8c9..e0a1ee36ad4 100644 --- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php +++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php @@ -58,15 +58,12 @@ public function sanitize() { continue; } - $attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node ); - if ( ! isset( $attributes['class'] ) ) { - continue; - } - $is_amp_lightbox = false !== strpos( $attributes['class'], 'amp-lightbox' ); - $is_amp_carousel = false !== strpos( $attributes['class'], 'amp-carousel' ); + $attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node ); + $is_amp_lightbox = isset( $attributes['data-amp-lightbox'] ) && true === filter_var( $attributes['data-amp-lightbox'], FILTER_VALIDATE_BOOLEAN ); + $is_amp_carousel = isset( $attributes['data-amp-carousel'] ) && true === filter_var( $attributes['data-amp-carousel'], FILTER_VALIDATE_BOOLEAN ); - // We are only looking for
                elements which have amp-carousel as class. - if ( ! isset( $attributes['class'] ) || ( ! $is_amp_carousel && ! $is_amp_lightbox ) ) { + // We are only looking for
                  elements which have amp-carousel / amp-lightbox true. + if ( ! $is_amp_carousel && ! $is_amp_lightbox ) { continue; } diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index c0c84931d5a..fc9d392fe35 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -139,10 +139,6 @@ private function filter_attributes( $attributes ) { $out['noloading'] = $value; break; - case 'data-amp-lightbox': - $out['lightbox'] = $value; - break; - default: break; } @@ -254,15 +250,15 @@ private function adjust_and_replace_node( $node ) { $layout = isset( $amp_data['layout'] ) ? $amp_data['layout'] : false; $new_attributes = $this->set_attachment_layout_attributes( $node, $new_attributes, $layout ); + if ( isset( $old_attributes['data-amp-lightbox'] ) ) { + $this->maybe_add_amp_image_lightbox_node(); + } + $this->add_or_append_attribute( $new_attributes, 'class', 'amp-wp-enforced-sizes' ); if ( empty( $new_attributes['layout'] ) && ! empty( $new_attributes['height'] ) && ! empty( $new_attributes['width'] ) ) { $new_attributes['layout'] = 'intrinsic'; } - if ( isset( $new_attributes['lightbox'] ) ) { - $this->maybe_add_amp_image_lightbox_node(); - } - if ( $this->is_gif_url( $new_attributes['src'] ) ) { $this->did_convert_elements = true; diff --git a/tests/test-class-amp-gallery-block-sanitizer.php b/tests/test-class-amp-gallery-block-sanitizer.php index 80c51831088..b5c461113ed 100644 --- a/tests/test-class-amp-gallery-block-sanitizer.php +++ b/tests/test-class-amp-gallery-block-sanitizer.php @@ -33,17 +33,17 @@ public function get_data() { ), 'data_amp_with_carousel' => array( - '', + '
                  ', '', ), 'data_amp_with_lightbox' => array( - '
                  ', - '
                  ', + '
                  ', + '
                  ', ), 'data_amp_with_lightbox_and_carousel' => array( - '', + '
                  ', '', ), ); From b70af66f90e5c9b45227199d0cf13a4c00293591 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Mon, 21 May 2018 12:10:56 +0300 Subject: [PATCH 11/21] Update method comments. Remove unused dynamicBlocks data. --- assets/js/amp-editor-blocks.js | 15 +++++---------- includes/admin/class-amp-editor-blocks.php | 16 +--------------- 2 files changed, 6 insertions(+), 25 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index b3561642f94..3e8d35397c8 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -12,7 +12,6 @@ var ampEditorBlocks = ( function() { * Holds data. */ data: { - dynamicBlocks: [], ampLayoutOptions: [ { value: 'nodisplay', @@ -92,13 +91,9 @@ var ampEditorBlocks = ( function() { }; /** - * Set data, add filters. - * - * @param {Array} data Data. + * Add filters. */ - component.boot = function boot( data ) { - _.extend( component.data, data ); - + component.boot = function boot() { wp.hooks.addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/addAttributes', component.addAMPAttributes ); wp.hooks.addFilter( 'blocks.getSaveElement', 'ampEditorBlocks/filterSave', component.filterBlocksSave ); wp.hooks.addFilter( 'blocks.BlockEdit', 'ampEditorBlocks/filterEdit', component.filterBlocksEdit ); @@ -472,7 +467,7 @@ var ampEditorBlocks = ( function() { * Adds ampCarousel attribute for displaying the output as amp-carousel. * * @param {Object} props Props. - * @return {*} Inspector controls. + * @return {Object} Inspector controls. */ component.setUpGalleryInpsectorControls = function setUpGalleryInpsectorControls( props ) { var isSelected = props.isSelected, @@ -495,7 +490,7 @@ var ampEditorBlocks = ( function() { * Adds ampCarousel attribute in case of gallery shortcode. * * @param {Object} props Props. - * @return {*} Inspector controls. + * @return {Object} Inspector controls. */ component.setUpShortcodeInspectorControls = function setUpShortcodeInspectorControls( props ) { var isSelected = props.isSelected, @@ -523,7 +518,7 @@ var ampEditorBlocks = ( function() { * @param {Object} element Element. * @param {string} blockType Block type. * @param {Object} attributes Attributes. - * @return {*} Output element. + * @return {Object} Output element. */ component.filterBlocksSave = function filterBlocksSave( element, blockType, attributes ) { var text; diff --git a/includes/admin/class-amp-editor-blocks.php b/includes/admin/class-amp-editor-blocks.php index 363b8798605..6cfe2f23894 100644 --- a/includes/admin/class-amp-editor-blocks.php +++ b/includes/admin/class-amp-editor-blocks.php @@ -50,20 +50,6 @@ public function add_editor_filters() { true ); - $dynamic_blocks = array(); - $block_type_registry = WP_Block_Type_Registry::get_instance(); - $block_types = $block_type_registry->get_all_registered(); - - foreach ( $block_types as $block_type ) { - if ( $block_type->is_dynamic() ) { - $dynamic_blocks[] = $block_type->name; - } - } - - wp_add_inline_script( 'amp-editor-blocks', sprintf( 'ampEditorBlocks.boot( %s );', - wp_json_encode( array( - 'dynamicBlocks' => $dynamic_blocks, - ) ) - ) ); + wp_add_inline_script( 'amp-editor-blocks', sprintf( 'ampEditorBlocks.boot();' ) ); } } From 6900e2b3b88718f5c50e406a3881c4d3ebce4635 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Mon, 28 May 2018 14:24:02 +0300 Subject: [PATCH 12/21] Fixes. --- assets/css/amp-default.css | 2 +- includes/sanitizers/class-amp-base-sanitizer.php | 2 +- tests/test-class-amp-gallery-block-sanitizer.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/assets/css/amp-default.css b/assets/css/amp-default.css index 1779a20aff8..77e7e909268 100644 --- a/assets/css/amp-default.css +++ b/assets/css/amp-default.css @@ -14,7 +14,7 @@ amp-fit-text h6 { font-size: inherit; } -#amp-image-lightbox-1 img { +#amp-image-lightbox .i-amphtml-image-lightbox-viewer-image { object-fit: contain; } diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index d339d2382a2..4e186e4ce8c 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -26,7 +26,7 @@ abstract class AMP_Base_Sanitizer { * * @const string */ - const AMP_IMAGE_LIGHTBOX_ID = 'amp-image-lightbox-1'; + const AMP_IMAGE_LIGHTBOX_ID = 'amp-image-lightbox'; /** * Placeholder for default args, to be set in child classes. diff --git a/tests/test-class-amp-gallery-block-sanitizer.php b/tests/test-class-amp-gallery-block-sanitizer.php index b5c461113ed..611ef71d41b 100644 --- a/tests/test-class-amp-gallery-block-sanitizer.php +++ b/tests/test-class-amp-gallery-block-sanitizer.php @@ -39,12 +39,12 @@ public function get_data() { 'data_amp_with_lightbox' => array( '
                  ', - '
                  ', + '
                  ', ), 'data_amp_with_lightbox_and_carousel' => array( '
                  ', - '', + '', ), ); } From ff2cdd25ea57a13c0622ffde38495e5e38f7b637 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Wed, 30 May 2018 18:06:08 +0300 Subject: [PATCH 13/21] Merge develop. --- assets/css/amp-default.css | 2 +- assets/js/amp-editor-blocks.js | 78 +++++++++++----------- includes/admin/class-amp-editor-blocks.php | 9 ++- 3 files changed, 47 insertions(+), 42 deletions(-) diff --git a/assets/css/amp-default.css b/assets/css/amp-default.css index 77e7e909268..c6dc8129cd6 100644 --- a/assets/css/amp-default.css +++ b/assets/css/amp-default.css @@ -14,7 +14,7 @@ amp-fit-text h6 { font-size: inherit; } -#amp-image-lightbox .i-amphtml-image-lightbox-viewer-image { +#amp-image-lightbox div div > :first-child { object-fit: contain; } diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index 97d1994ebed..e6ee393657f 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -434,29 +434,29 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars } return isSelected && ( - el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: __( 'AMP Settings', 'amp' ) }, - el( SelectControl, { - label: label, - value: ampLayout, - options: component.getLayoutOptions( name ), - onChange: function( value ) { - props.setAttributes( { ampLayout: value } ); - if ( 'core/image' === props.name ) { - component.setImageBlockLayoutAttributes( props, value ); - } - } - } ), - el( ToggleControl, { - label: __( 'AMP loading indicator disabled', 'amp' ), - checked: ampNoLoading, - onChange: function() { - props.setAttributes( { ampNoLoading: ! ampNoLoading } ); + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: __( 'AMP Settings', 'amp' ) }, + el( SelectControl, { + label: label, + value: ampLayout, + options: component.getLayoutOptions( name ), + onChange: function( value ) { + props.setAttributes( { ampLayout: value } ); + if ( 'core/image' === props.name ) { + component.setImageBlockLayoutAttributes( props, value ); } - } ) - ) + } + } ), + el( ToggleControl, { + label: __( 'AMP loading indicator disabled', 'amp' ), + checked: ampNoLoading, + onChange: function() { + props.setAttributes( { ampNoLoading: ! ampNoLoading } ); + } + } ) ) - ); + ) + ); }; /** @@ -653,14 +653,14 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars PanelBody = wp.components.PanelBody; return isSelected && ( - el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: component.data.ampPanelLabel }, - component.getAmpLayoutControl( props ), - component.getAmpNoloadingToggle( props ), - component.getAmpLightboxToggle( props ) - ) + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: component.data.ampPanelLabel }, + component.getAmpLayoutControl( props ), + component.getAmpNoloadingToggle( props ), + component.getAmpLightboxToggle( props ) ) - ); + ) + ); }; /** @@ -677,13 +677,13 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars PanelBody = wp.components.PanelBody; return isSelected && ( - el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: component.data.ampPanelLabel }, - component.getAmpCarouselToggle( props ), - component.getAmpLightboxToggle( props ) - ) + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: component.data.ampPanelLabel }, + component.getAmpCarouselToggle( props ), + component.getAmpLightboxToggle( props ) ) - ); + ) + ); }; /** @@ -711,12 +711,12 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars } } ); return isSelected && ( - el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: __( 'AMP Settings', 'amp' ) }, - toggleControl - ) + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: __( 'AMP Settings', 'amp' ) }, + toggleControl ) - ); + ) + ); } return ''; diff --git a/includes/admin/class-amp-editor-blocks.php b/includes/admin/class-amp-editor-blocks.php index a916f5ddf6a..6d1c3225b62 100644 --- a/includes/admin/class-amp-editor-blocks.php +++ b/includes/admin/class-amp-editor-blocks.php @@ -115,6 +115,12 @@ public function enqueue_block_editor_assets() { AMP__VERSION ); + wp_add_inline_script( + 'amp-editor-blocks-build', + 'wp.i18n.setLocaleData( ' . wp_json_encode( gutenberg_get_jed_locale_data( 'amp' ) ) . ', "amp" );', + 'before' + ); + // Scripts. wp_enqueue_script( 'amp-editor-blocks-build', @@ -133,8 +139,7 @@ public function enqueue_block_editor_assets() { wp_add_inline_script( 'amp-editor-blocks', - 'wp.i18n.setLocaleData( ' . wp_json_encode( gutenberg_get_jed_locale_data( 'amp' ) ) . ', "amp" );' . sprintf( 'ampEditorBlocks.boot();' ), - 'before' + 'ampEditorBlocks.boot();' ); } From d5169effab32f63fa397282d682014b99b9ca6fa Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Wed, 30 May 2018 18:06:40 +0300 Subject: [PATCH 14/21] Fixes after merge. --- assets/js/amp-editor-blocks.js | 137 ++++++++++++--------------------- 1 file changed, 51 insertions(+), 86 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index e6ee393657f..36e6ea5ccb2 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -360,6 +360,28 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars } }; + /** + * Default setup for inspector controls. + * + * @param {Object} props Props. + * @return {Object|Element|*|{$$typeof, type, key, ref, props, _owner}} Inspector Controls. + */ + component.setUpInspectorControls = function setUpInspectorControls( props ) { + var isSelected = props.isSelected, + el = wp.element.createElement, + InspectorControls = wp.editor.InspectorControls, + PanelBody = wp.components.PanelBody; + + return isSelected && ( + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: component.data.ampPanelLabel }, + component.getAmpLayoutControl( props ), + component.getAmpNoloadingToggle( props ) + ) + ) + ); + }; + /** * Get AMP Layout select control. * @@ -411,54 +433,6 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars } ); }; - /** - * Default setup for inspector controls. - * - * @param {Object} props Props. - * @return {Object|Element|*|{$$typeof, type, key, ref, props, _owner}} Inspector Controls. - */ - component.setUpInspectorControls = function setUpInspectorControls( props ) { - var ampLayout = props.attributes.ampLayout, - ampNoLoading = props.attributes.ampNoLoading, - isSelected = props.isSelected, - name = props.name, - el = wp.element.createElement, - InspectorControls = wp.editor.InspectorControls, - SelectControl = wp.components.SelectControl, - ToggleControl = wp.components.ToggleControl, - PanelBody = wp.components.PanelBody, - label = __( 'AMP Layout', 'amp' ); - - if ( 'core/image' === name ) { - label = __( 'AMP Layout (modifies width/height)', 'amp' ); - } - - return isSelected && ( - el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: __( 'AMP Settings', 'amp' ) }, - el( SelectControl, { - label: label, - value: ampLayout, - options: component.getLayoutOptions( name ), - onChange: function( value ) { - props.setAttributes( { ampLayout: value } ); - if ( 'core/image' === props.name ) { - component.setImageBlockLayoutAttributes( props, value ); - } - } - } ), - el( ToggleControl, { - label: __( 'AMP loading indicator disabled', 'amp' ), - checked: ampNoLoading, - onChange: function() { - props.setAttributes( { ampNoLoading: ! ampNoLoading } ); - } - } ) - ) - ) - ); - }; - /** * Setup inspector controls for text blocks. * @@ -588,6 +562,33 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars ); }; + /** + * Set up inspector controls for shortcode block. + * Adds ampCarousel attribute in case of gallery shortcode. + * + * @param {Object} props Props. + * @return {Object} Inspector controls. + */ + component.setUpShortcodeInspectorControls = function setUpShortcodeInspectorControls( props ) { + var isSelected = props.isSelected, + el = wp.element.createElement, + InspectorControls = wp.editor.InspectorControls, + PanelBody = wp.components.PanelBody; + + if ( component.isGalleryShortcode( props.attributes ) ) { + return isSelected && ( + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: component.data.ampPanelLabel }, + component.getAmpCarouselToggle( props ), + component.getAmpLightboxToggle( props ) + ) + ) + ); + } + + return ''; + }; + /** * Get AMP Lightbox toggle control. * @@ -686,42 +687,6 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars ); }; - /** - * Set up inspector controls for shortcode block. - * Adds ampCarousel attribute in case of gallery shortcode. - * - * @param {Object} props Props. - * @return {*} Inspector controls. - */ - component.setUpShortcodeInspectorControls = function setUpShortcodeInspectorControls( props ) { - var ampCarousel = props.attributes.ampCarousel, - isSelected = props.isSelected, - el = wp.element.createElement, - InspectorControls = wp.editor.InspectorControls, - ToggleControl = wp.components.ToggleControl, - PanelBody = wp.components.PanelBody, - toggleControl; - - if ( component.isGalleryShortcode( props.attributes ) ) { - toggleControl = el( ToggleControl, { - label: __( 'Display as AMP carousel', 'amp' ), - checked: ampCarousel, - onChange: function() { - props.setAttributes( { ampCarousel: ! ampCarousel } ); - } - } ); - return isSelected && ( - el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: __( 'AMP Settings', 'amp' ) }, - toggleControl - ) - ) - ); - } - - return ''; - }; - /** * Filters blocks' save function. * @@ -731,7 +696,7 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars * @return {Object} Output element. */ component.filterBlocksSave = function filterBlocksSave( element, blockType, attributes ) { - var text, + var text = '', fitTextProps = { layout: 'fixed-height', children: element @@ -863,7 +828,7 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars * @param {string} text Shortcode. * @return {boolean} If has amp-carousel. */ - component.hasGalleryShortcodeCarouselAttribute = function galleryShortcodeHasCarouselAttribute( text ) { + component.hasGalleryShortcodeCarouselAttribute = function hasGalleryShortcodeCarouselAttribute( text ) { return -1 !== text.indexOf( 'amp-carousel=false' ); }; From 13267f6f73d7f7e1e83526e947a9e3ca2c95b544 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Wed, 30 May 2018 19:49:00 +0300 Subject: [PATCH 15/21] Fix displaying AMP carousel. --- assets/js/amp-editor-blocks.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index 36e6ea5ccb2..b225e49a9cb 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -696,7 +696,7 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars * @return {Object} Output element. */ component.filterBlocksSave = function filterBlocksSave( element, blockType, attributes ) { - var text = '', + var text = attributes.text || '', fitTextProps = { layout: 'fixed-height', children: element @@ -707,8 +707,6 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars if ( component.hasGalleryShortcodeLightboxAttribute( attributes.text || '' ) ) { text = component.removeAmpLightboxFromShortcodeAtts( attributes.text ); } - } else { - text = attributes.text || ''; } if ( attributes.ampCarousel ) { // If the text contains amp-carousel or amp-lightbox, lets remove it. From 134335e189df2d0da68e81e9fd063065dd6aaa31 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Wed, 30 May 2018 20:28:05 +0300 Subject: [PATCH 16/21] Fix checking for shortcode. --- assets/js/amp-editor-blocks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index b225e49a9cb..ff4756b29ff 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -161,7 +161,7 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars var ampAttributes = {}; // Shortcode props are handled differently. - if ( 'core/shortcode' === blockType ) { + if ( 'core/shortcode' === blockType.name ) { return props; } From 26c491e7ce47c8e22cea57e73c378040cf0e1aef Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Wed, 30 May 2018 21:03:11 +0300 Subject: [PATCH 17/21] Fix jscs. --- assets/js/amp-editor-blocks.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index ff4756b29ff..43a73982b95 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -373,13 +373,13 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars PanelBody = wp.components.PanelBody; return isSelected && ( - el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: component.data.ampPanelLabel }, - component.getAmpLayoutControl( props ), - component.getAmpNoloadingToggle( props ) - ) + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: component.data.ampPanelLabel }, + component.getAmpLayoutControl( props ), + component.getAmpNoloadingToggle( props ) ) - ); + ) + ); }; /** @@ -577,13 +577,13 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars if ( component.isGalleryShortcode( props.attributes ) ) { return isSelected && ( - el( InspectorControls, { key: 'inspector' }, - el( PanelBody, { title: component.data.ampPanelLabel }, - component.getAmpCarouselToggle( props ), - component.getAmpLightboxToggle( props ) - ) + el( InspectorControls, { key: 'inspector' }, + el( PanelBody, { title: component.data.ampPanelLabel }, + component.getAmpCarouselToggle( props ), + component.getAmpLightboxToggle( props ) ) - ); + ) + ); } return ''; From 201654fa66c1c0dec3f7336c3dd2bf382d39e73e Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 30 May 2018 15:34:15 -0700 Subject: [PATCH 18/21] Fix AMP carousel for gallery block. * Let gallery have be sized according to the max height. * Constrain height of AMP gallery carousel by ratio with max width * Remove duplicate data for jed_locale_data. --- assets/js/amp-editor-blocks.js | 2 +- includes/admin/class-amp-editor-blocks.php | 6 -- .../class-amp-gallery-block-sanitizer.php | 71 ++++++++++--------- ...test-class-amp-gallery-block-sanitizer.php | 4 +- 4 files changed, 41 insertions(+), 42 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index 9fd6e3b0860..6a5fdad6575 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -180,7 +180,7 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars ampAttributes[ 'data-amp-lightbox' ] = attributes.ampLightbox; } if ( attributes.ampCarousel ) { - ampAttributes[ 'data-amp-carousel' ] = attributes.ampLightbox; + ampAttributes[ 'data-amp-carousel' ] = attributes.ampCarousel; } return _.extend( ampAttributes, props ); diff --git a/includes/admin/class-amp-editor-blocks.php b/includes/admin/class-amp-editor-blocks.php index 5e0dc96fb70..b5d6a2cb084 100644 --- a/includes/admin/class-amp-editor-blocks.php +++ b/includes/admin/class-amp-editor-blocks.php @@ -117,12 +117,6 @@ public function enqueue_block_editor_assets() { AMP__VERSION ); - wp_add_inline_script( - 'amp-editor-blocks-build', - 'wp.i18n.setLocaleData( ' . wp_json_encode( gutenberg_get_jed_locale_data( 'amp' ) ) . ', "amp" );', - 'before' - ); - // Scripts. wp_enqueue_script( 'amp-editor-blocks-build', diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php index e0a1ee36ad4..37c5ba37ab9 100644 --- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php +++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php @@ -78,35 +78,30 @@ public function sanitize() { continue; } - $num_images = 0; + $images = null; // If it's not AMP lightbox, look for links first. if ( ! $is_amp_lightbox ) { - $images = $node->getElementsByTagName( 'a' ); - $num_images = $images->length; + $images = $node->getElementsByTagName( 'a' ); } - if ( 0 === $num_images ) { - - // If not linking to anything then look for . - $images = $node->getElementsByTagName( 'amp-img' ); - $num_images = $images->length; + // If not linking to anything then look for . + if ( ! $images || 0 === $images->length ) { + $images = $node->getElementsByTagName( 'amp-img' ); } - if ( 0 === $num_images ) { + // Skip if no images found. + if ( ! $images || 0 === $images->length ) { continue; } - $carousel_height = $this->get_carousel_height( $node ); - $carousel_attributes = array( - 'height' => $carousel_height, + $amp_carousel = AMP_DOM_Utils::create_node( $this->dom, 'amp-carousel', array( + 'height' => $this->get_carousel_height( $node ), 'type' => 'slides', 'layout' => 'fixed-height', - ); - $amp_carousel = AMP_DOM_Utils::create_node( $this->dom, 'amp-carousel', $carousel_attributes ); - - for ( $j = $num_images - 1; $j >= 0; $j-- ) { - $amp_carousel->appendChild( $images->item( $j ) ); + ) ); + foreach ( $images as $image ) { + $amp_carousel->appendChild( $image ); } $node->parentNode->replaceChild( $amp_carousel, $node ); @@ -117,39 +112,47 @@ public function sanitize() { /** * Get carousel height by containing images. * - * @param DOMNode $node Node
                    . - * @return int + * @param DOMElement $element The UL element. + * @return int Height. */ - protected function get_carousel_height( $node ) { - $images = $node->getElementsByTagName( 'amp-img' ); + protected function get_carousel_height( $element ) { + $images = $element->getElementsByTagName( 'amp-img' ); $num_images = $images->length; - $height = false; + $max_height = 0; + $max_width = 0; if ( 0 === $num_images ) { return self::FALLBACK_HEIGHT; } - for ( $i = $num_images - 1; $i >= 0; $i-- ) { - $image = $images->item( $i ); + foreach ( $images as $image ) { + /** + * Image. + * + * @var DOMElement $image + */ $image_height = $image->getAttribute( 'height' ); - if ( ! $image_height || ! is_numeric( $image_height ) ) { - continue; + if ( is_numeric( $image_height ) ) { + $max_height = max( $max_height, $image_height ); } - if ( ! $height ) { - $height = $image_height; - } elseif ( $height > $image_height ) { - $height = $image_height; + $image_width = $image->getAttribute( 'height' ); + if ( is_numeric( $image_width ) ) { + $max_width = max( $max_width, $image_width ); } } - return false === $height ? self::FALLBACK_HEIGHT : $height; + if ( ! empty( $this->args['content_max_width'] ) && $max_height > 0 && $max_width > $this->args['content_max_width'] ) { + $max_height = ( $max_width * $this->args['content_max_width'] ) / $max_height; + } + + return ! $max_height ? self::FALLBACK_HEIGHT : $max_height; } /** * Set lightbox related attributes to within gallery. * - * @param DOMNode $node
                      node. + * @param DOMElement $element The UL element. */ - protected function add_lightbox_attributes_to_image_nodes( $node ) { - $images = $node->getElementsByTagName( 'amp-img' ); + protected function add_lightbox_attributes_to_image_nodes( $element ) { + $images = $element->getElementsByTagName( 'amp-img' ); $num_images = $images->length; if ( 0 === $num_images ) { return; diff --git a/tests/test-class-amp-gallery-block-sanitizer.php b/tests/test-class-amp-gallery-block-sanitizer.php index 611ef71d41b..97831d2e914 100644 --- a/tests/test-class-amp-gallery-block-sanitizer.php +++ b/tests/test-class-amp-gallery-block-sanitizer.php @@ -58,7 +58,9 @@ public function get_data() { */ public function test_sanitizer( $source, $expected ) { $dom = AMP_DOM_Utils::get_dom_from_content( $source ); - $sanitizer = new AMP_Gallery_Block_Sanitizer( $dom ); + $sanitizer = new AMP_Gallery_Block_Sanitizer( $dom, array( + 'content_max_width' => 600, + ) ); $sanitizer->sanitize(); $content = AMP_DOM_Utils::get_content_from_dom( $dom ); $content = preg_replace( '/(?<=>)\s+(?=<)/', '', $content ); From f9f3a054d7d9c66c935fbf4271cd4bc91679479c Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 30 May 2018 15:56:32 -0700 Subject: [PATCH 19/21] Pass through all attributes from img to amp-img See #1147 --- includes/sanitizers/class-amp-img-sanitizer.php | 12 +----------- tests/test-amp-img-sanitizer.php | 10 +++++----- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index 72726addc16..121837d97b8 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -115,17 +115,6 @@ private function filter_attributes( $attributes ) { foreach ( $attributes as $name => $value ) { switch ( $name ) { - case 'src': - case 'alt': - case 'class': - case 'srcset': - case 'on': - case 'role': - case 'tabindex': - case 'attribution': - $out[ $name ] = $value; - break; - case 'width': case 'height': $out[ $name ] = $this->sanitize_dimension( $value, $name ); @@ -140,6 +129,7 @@ private function filter_attributes( $attributes ) { break; default: + $out[ $name ] = $value; break; } } diff --git a/tests/test-amp-img-sanitizer.php b/tests/test-amp-img-sanitizer.php index 3b17c344559..ed4f3765f33 100644 --- a/tests/test-amp-img-sanitizer.php +++ b/tests/test-amp-img-sanitizer.php @@ -127,11 +127,6 @@ public function get_data() { '', ), - 'image_with_blacklisted_attribute' => array( - '', - '', - ), - 'image_with_no_dimensions_is_forced' => array( '', '', @@ -183,6 +178,11 @@ public function get_data() { '
                      This is an example caption.
                      ', '
                      This is an example caption.
                      ', ), + + 'image_with_custom_lightbox_attrs' => array( + '', + '', + ), ); } From cc53173f92f166bc557d37954dece76b6ca651d9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 30 May 2018 16:17:31 -0700 Subject: [PATCH 20/21] Force carousels to be used when AMP theme support is absent (for back-compat) --- assets/js/amp-editor-blocks.js | 15 +++++++++---- includes/admin/class-amp-editor-blocks.php | 4 +++- includes/amp-helper-functions.php | 4 +++- .../class-amp-gallery-block-sanitizer.php | 21 ++++++++++++++++++- 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/assets/js/amp-editor-blocks.js b/assets/js/amp-editor-blocks.js index 6a5fdad6575..7b9128f5d3f 100644 --- a/assets/js/amp-editor-blocks.js +++ b/assets/js/amp-editor-blocks.js @@ -99,13 +99,20 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars larger: 48 }, ampPanelLabel: __( 'AMP Settings' ) - } + }, + hasThemeSupport: true }; /** * Add filters. + * + * @param {Object} data Data. */ - component.boot = function boot() { + component.boot = function boot( data ) { + if ( data ) { + _.extend( component.data, data ); + } + wp.hooks.addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/addAttributes', component.addAMPAttributes ); wp.hooks.addFilter( 'blocks.getSaveElement', 'ampEditorBlocks/filterSave', component.filterBlocksSave ); wp.hooks.addFilter( 'blocks.BlockEdit', 'ampEditorBlocks/filterEdit', component.filterBlocksEdit ); @@ -575,7 +582,7 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars return isSelected && ( el( InspectorControls, { key: 'inspector' }, el( PanelBody, { title: component.data.ampPanelLabel }, - component.getAmpCarouselToggle( props ), + component.data.hasThemeSupport && component.getAmpCarouselToggle( props ), component.getAmpLightboxToggle( props ) ) ) @@ -676,7 +683,7 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars return isSelected && ( el( InspectorControls, { key: 'inspector' }, el( PanelBody, { title: component.data.ampPanelLabel }, - component.getAmpCarouselToggle( props ), + component.data.hasThemeSupport && component.getAmpCarouselToggle( props ), component.getAmpLightboxToggle( props ) ) ) diff --git a/includes/admin/class-amp-editor-blocks.php b/includes/admin/class-amp-editor-blocks.php index b5d6a2cb084..a93e396b169 100644 --- a/includes/admin/class-amp-editor-blocks.php +++ b/includes/admin/class-amp-editor-blocks.php @@ -141,7 +141,9 @@ public function enqueue_block_editor_assets() { wp_add_inline_script( 'amp-editor-blocks', - 'ampEditorBlocks.boot();' + sprintf( 'ampEditorBlocks.boot( %s );', wp_json_encode( array( + 'hasThemeSupport' => current_theme_supports( 'amp' ), + ) ) ) ); } diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index c16cd4bc83d..066cf60f7d8 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -489,7 +489,9 @@ function amp_get_content_sanitizers( $post = null ) { 'AMP_Iframe_Sanitizer' => array( 'add_placeholder' => true, ), - 'AMP_Gallery_Block_Sanitizer' => array(), // Note: Gallery block sanitizer must come after image sanitizers since itś logic is using the already sanitized images. + 'AMP_Gallery_Block_Sanitizer' => array( // Note: Gallery block sanitizer must come after image sanitizers since itś logic is using the already sanitized images. + 'carousel_required' => ! current_theme_supports( 'amp' ), // For back-compat. + ), 'AMP_Block_Sanitizer' => array(), // Note: Block sanitizer must come after embed / media sanitizers since it's logic is using the already sanitized content. 'AMP_Style_Sanitizer' => array(), 'AMP_Tag_And_Attribute_Sanitizer' => array(), // Note: This whitelist sanitizer must come at the end to clean up any remaining issues the other sanitizers didn't catch. diff --git a/includes/sanitizers/class-amp-gallery-block-sanitizer.php b/includes/sanitizers/class-amp-gallery-block-sanitizer.php index 37c5ba37ab9..09b753d2625 100644 --- a/includes/sanitizers/class-amp-gallery-block-sanitizer.php +++ b/includes/sanitizers/class-amp-gallery-block-sanitizer.php @@ -38,6 +38,25 @@ class AMP_Gallery_Block_Sanitizer extends AMP_Base_Sanitizer { */ public static $tag = 'ul'; + /** + * Array of flags used to control sanitization. + * + * @var array { + * @type int $content_max_width Max width of content. + * @type bool $carousel_required Whether carousels are required. This is used when amp theme support is not present, for back-compat. + * } + */ + protected $args; + + /** + * Default args. + * + * @var array + */ + protected $DEFAULT_ARGS = array( + 'carousel_required' => false, + ); + /** * Sanitize the gallery block contained by
                        element where necessary. * @@ -60,7 +79,7 @@ public function sanitize() { $attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node ); $is_amp_lightbox = isset( $attributes['data-amp-lightbox'] ) && true === filter_var( $attributes['data-amp-lightbox'], FILTER_VALIDATE_BOOLEAN ); - $is_amp_carousel = isset( $attributes['data-amp-carousel'] ) && true === filter_var( $attributes['data-amp-carousel'], FILTER_VALIDATE_BOOLEAN ); + $is_amp_carousel = ! empty( $this->args['carousel_required'] ) || ( isset( $attributes['data-amp-carousel'] ) && true === filter_var( $attributes['data-amp-carousel'], FILTER_VALIDATE_BOOLEAN ) ); // We are only looking for
                          elements which have amp-carousel / amp-lightbox true. if ( ! $is_amp_carousel && ! $is_amp_lightbox ) { From dbe281c2283d83a19d0c676a7ebbaca4fd2954cb Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 30 May 2018 16:23:57 -0700 Subject: [PATCH 21/21] Remove apparently unneessary object-fit from amp-image-lightbox; remove non-standard entry__content --- assets/css/amp-default.css | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/assets/css/amp-default.css b/assets/css/amp-default.css index c6dc8129cd6..2c837e2f469 100644 --- a/assets/css/amp-default.css +++ b/assets/css/amp-default.css @@ -4,7 +4,7 @@ object-fit: contain; } -.entry__content amp-fit-text blockquote, +amp-fit-text blockquote, amp-fit-text h1, amp-fit-text h2, amp-fit-text h3, @@ -14,10 +14,3 @@ amp-fit-text h6 { font-size: inherit; } -#amp-image-lightbox div div > :first-child { - object-fit: contain; -} - -.entry__content .wp-block-gallery[class*="columns-"] figure { - width: 100%; -}