Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework how and whether galleries are rendered as carousels #4775

Merged
merged 53 commits into from
Jul 24, 2020
Merged

Rework how and whether galleries are rendered as carousels #4775

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
7a2ca29
Rework how and whether galleries are rendered as carousels
westonruter May 26, 2020
9fb2644
Merge branch 'develop' into fix/gallery-carousel-defaults
pierlon Jun 30, 2020
d186af5
Remove `loading` attribute from img tag since dimensions are not prov…
pierlon Jun 30, 2020
a078832
Merge branch 'develop' into fix/gallery-carousel-defaults
pierlon Jul 6, 2020
2a7ace2
Fix display of HTML captions in AMP carousel
pierlon Jul 7, 2020
6d055ad
Fix lightbox not showing images
pierlon Jul 7, 2020
0f213bd
Merge branch 'develop' into fix/gallery-carousel-defaults
pierlon Jul 13, 2020
38298f3
Merge branch 'develop' into fix/gallery-carousel-defaults
pierlon Jul 17, 2020
19b45c9
Refactor logic handling gallery embed shortcodes and blocks
pierlon Jul 20, 2020
eba5869
Merge branch 'develop' into fix/gallery-carousel-defaults
pierlon Jul 20, 2020
8e261da
Fix lint errors
pierlon Jul 20, 2020
fd3e9f2
Fix test errors
pierlon Jul 20, 2020
03df2b2
Indicate `$caption_node` is an instance of DOMElement
pierlon Jul 20, 2020
9467dcc
Use constants for tag names
pierlon Jul 21, 2020
b7df289
Merge branch 'develop' of github.com:ampproject/amp-wp into fix/galle…
westonruter Jul 21, 2020
bab1fc9
Prevent gallery_style filter from applying to other shortcodes
westonruter Jul 21, 2020
c097f62
Harden matching the class attribute
westonruter Jul 21, 2020
b64cc1e
Use __FUNCTION__ to refer to the current function
westonruter Jul 21, 2020
a6269b5
Improve means of preventing infinite recursion when calling gallery_s…
westonruter Jul 21, 2020
9336e13
Reuse Tag::FIGCAPTION
westonruter Jul 22, 2020
81a8004
Make insertion of `data` attributes more robust
pierlon Jul 22, 2020
2421fb3
Remove re-importing of element
pierlon Jul 22, 2020
7d9a41d
Use already retrieved attributes
pierlon Jul 22, 2020
0bd44f9
Enforce `DOMElement` type for caption slide
pierlon Jul 22, 2020
0ce7c51
Enforce slide node as being an element same as caption
westonruter Jul 22, 2020
adc609d
Ensure gallery caption is retained
pierlon Jul 22, 2020
0c568e6
Fix failing tests
pierlon Jul 22, 2020
a4becff
Remove deprecated AMP props from blocks that may contain them
pierlon Jul 23, 2020
dc70071
Define deprecated array if not defined already
westonruter Jul 23, 2020
edbd630
Improve jsdoc for static analysis
westonruter Jul 23, 2020
f0e9d77
Fix ampLayout attribute name
westonruter Jul 23, 2020
d5fa797
Reset `deprecatedProps` object when done re-rendering block
pierlon Jul 23, 2020
7b1b765
Fix propType for `ampNoLoading` toggle
pierlon Jul 23, 2020
32f224e
Move `deprecatedProps` into `save` method
pierlon Jul 23, 2020
f7be98b
Escape block property value
pierlon Jul 23, 2020
405359b
Simplify check for deprecated prop
pierlon Jul 23, 2020
b7de0f1
Use for/of loop; move mappedAttributes out of removeDeprecatedAmpProps
westonruter Jul 23, 2020
da088e0
Ensure PHP boolean is stringified as true/false
westonruter Jul 23, 2020
fd4f8b6
Eliminate gratuitous use of wp_json_encode()
westonruter Jul 23, 2020
7d9b3ed
Fix issue with image captions not showing in amp-carousel when render…
pierlon Jul 23, 2020
48343af
Supply missing attribute value for gallery shortcode output
westonruter Jul 24, 2020
bb85a02
Ensure data-amp-carousel=true attribute is present for proper styling…
westonruter Jul 24, 2020
9aa0552
Prevent removal of data attributes in sanitize_raw_embeds and fix boo…
westonruter Jul 24, 2020
4bc1439
Use DOMElement as return type for HasCaption::get_caption_element()
westonruter Jul 24, 2020
6687931
Modify the block attributes during its deprecation
pierlon Jul 24, 2020
b2901f0
Introduce ObsoleteBlockAttributeRemover service
westonruter Jul 24, 2020
e3c7afb
Improve logic for obtaining obsolete attribute pattern
westonruter Jul 24, 2020
48eb9d1
Improve phpdoc for ObsoleteBlockAttributeRemover
westonruter Jul 24, 2020
1fa5923
Harden pattern to match block comment and start tag
westonruter Jul 24, 2020
9745353
Revert block deprecation logic
pierlon Jul 24, 2020
6eb20db
Skip adding data attributes for gallery shortcodes
westonruter Jul 24, 2020
88d5ae9
Add tests for ObsoleteBlockAttributeRemover
westonruter Jul 24, 2020
9d7260f
Fix test failure in WP 4.9
westonruter Jul 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
216 changes: 14 additions & 202 deletions assets/src/block-editor/helpers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { ReactElement } from 'react';
* WordPress dependencies
*/
import { __, _x } from '@wordpress/i18n';
import { cloneElement, RawHTML, render } from '@wordpress/element';
import { cloneElement, render } from '@wordpress/element';
import { TextControl, SelectControl, ToggleControl, Notice, PanelBody, FontSizePicker } from '@wordpress/components';
import { InspectorControls } from '@wordpress/block-editor';
import { select } from '@wordpress/data';
Expand Down Expand Up @@ -94,15 +94,17 @@ const ampLayoutOptions = [
*/
export const addAMPAttributes = ( settings, name ) => {
// AMP Carousel settings.
if ( 'core/shortcode' === name || 'core/gallery' === name ) {
if ( 'core/gallery' === name ) {
if ( ! settings.attributes ) {
settings.attributes = {};
}
settings.attributes.ampCarousel = {
type: 'boolean',
default: ! select( 'amp/block-editor' ).hasThemeSupport(), // @todo We could just default this to false now even in Reader mode since block styles are loaded.
Copy link
Member Author

Choose a reason for hiding this comment

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

Let's revisit this…

};
settings.attributes.ampLightbox = {
type: 'boolean',
default: false,
};
}

Expand All @@ -113,6 +115,7 @@ export const addAMPAttributes = ( settings, name ) => {
}
settings.attributes.ampLightbox = {
type: 'boolean',
default: false,
};
}

Expand All @@ -124,6 +127,7 @@ export const addAMPAttributes = ( settings, name ) => {
settings.attributes = {};
}
settings.attributes.ampFitText = {
type: 'boolean',
default: false,
};
settings.attributes.minFont = {
Expand Down Expand Up @@ -173,58 +177,12 @@ export const addAMPAttributes = ( settings, name ) => {
* @return {Object} Output element.
*/
export const filterBlocksSave = ( element, blockType, attributes ) => { // eslint-disable-line complexity
let text = attributes.text || '',
content = '';

const fitTextProps = {
layout: 'fixed-height',
};

if ( 'core/shortcode' === blockType.name && isGalleryShortcode( attributes ) ) {
if ( ! attributes.ampLightbox ) {
if ( hasGalleryShortcodeLightboxAttribute( attributes.text || '' ) ) {
text = removeAmpLightboxFromShortcodeAtts( attributes.text );
}
}
if ( attributes.ampCarousel ) {
// If the text contains amp-carousel or amp-lightbox, lets remove it.
if ( hasGalleryShortcodeCarouselAttribute( text ) ) {
text = removeAmpCarouselFromShortcodeAtts( text );
}

// If lightbox is not set, we can return here.
if ( ! attributes.ampLightbox ) {
if ( attributes.text !== text ) {
return (
<RawHTML>
{ text }
</RawHTML>
);
}

// Else lets return original.
return element;
}
} else if ( ! hasGalleryShortcodeCarouselAttribute( attributes.text || '' ) ) {
// Add amp-carousel=false attribute to the shortcode.
text = attributes.text.replace( '[gallery', '[gallery amp-carousel=false' );
} else {
text = attributes.text;
}

if ( attributes.ampLightbox && ! hasGalleryShortcodeLightboxAttribute( text ) ) {
text = text.replace( '[gallery', '[gallery amp-lightbox=true' );
}

if ( attributes.text !== text ) {
return (
<RawHTML>
{ text }
</RawHTML>
);
}
} else if ( 'core/paragraph' === blockType.name && ! attributes.ampFitText ) {
content = getAmpFitTextContent( attributes.content );
if ( 'core/paragraph' === blockType.name && ! attributes.ampFitText ) {
const content = getAmpFitTextContent( attributes.content );
if ( content !== attributes.content ) {
return cloneElement(
element,
Expand Down Expand Up @@ -302,48 +260,6 @@ export const getLayoutOptions = ( block ) => {
return layoutOptions;
};

/**
* Add extra data-amp-layout attribute to save to DB.
*
* @param {Object} props Properties.
* @param {Object} blockType Block type.
* @param {Object} blockType.name Block type name.
* @param {Object} attributes Attributes.
*
* @return {Object} Props.
*/
export const addAMPExtraProps = ( props, blockType, attributes ) => {
const ampAttributes = {};

// Shortcode props are handled differently.
if ( 'core/shortcode' === blockType.name ) {
return props;
}

// AMP blocks handle layout and other props on their own.
if ( 'amp/' === blockType.name.substr( 0, 4 ) ) {
return props;
}

if ( attributes.ampLayout ) {
ampAttributes[ 'data-amp-layout' ] = attributes.ampLayout;
}
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.ampCarousel;
}

return {
...ampAttributes,
...props,
};
};

/**
* Filters blocks edit function of all blocks.
*
Expand All @@ -353,26 +269,11 @@ export const addAMPExtraProps = ( props, blockType, attributes ) => {
*/
export const filterBlocksEdit = ( BlockEdit ) => {
const EnhancedBlockEdit = function( props ) {
const { attributes: { text, ampLayout }, setAttributes, name } = props;
const { attributes: { ampLayout }, name } = props;

let inspectorControls;

if ( 'core/shortcode' === name ) {
// Lets remove amp-carousel from edit view.
if ( hasGalleryShortcodeCarouselAttribute( text || '' ) ) {
setAttributes( { text: removeAmpCarouselFromShortcodeAtts( text ) } );
}
// Lets remove amp-lightbox from edit view.
if ( hasGalleryShortcodeLightboxAttribute( text || '' ) ) {
setAttributes( { text: removeAmpLightboxFromShortcodeAtts( text ) } );
}

inspectorControls = setUpShortcodeInspectorControls( props );
if ( '' === inspectorControls ) {
// Return original.
return <BlockEdit { ...props } />;
}
} else if ( 'core/gallery' === name ) {
if ( 'core/gallery' === name ) {
inspectorControls = setUpGalleryInspectorControls( props );
} else if ( 'core/image' === name ) {
inspectorControls = setUpImageInspectorControls( props );
Expand Down Expand Up @@ -666,46 +567,14 @@ const setUpTextBlocksInspectorControls = ( props ) => {
setUpTextBlocksInspectorControls.propTypes = {
isSelected: PropTypes.bool,
attributes: PropTypes.shape( {
ampFitText: PropTypes.string,
ampFitText: PropTypes.bool,
minFont: PropTypes.number,
maxFont: PropTypes.number,
height: PropTypes.number,
} ),
setAttributes: PropTypes.func.isRequired,
};

/**
* Set up inspector controls for shortcode block.
* Adds ampCarousel attribute in case of gallery shortcode.
*
* @param {Object} props Props.
*
* @return {ReactElement} Inspector controls.
*/
const setUpShortcodeInspectorControls = ( props ) => {
const { isSelected } = props;

if ( ! isGalleryShortcode( props.attributes ) || ! isSelected ) {
return null;
}

const hasThemeSupport = select( 'amp/block-editor' ).hasThemeSupport();

return (
<InspectorControls>
<PanelBody title={ __( 'AMP Settings', 'amp' ) }>
{ hasThemeSupport && <AmpCarouselToggle { ...props } /> }
<AmpLightboxToggle { ...props } />
</PanelBody>
</InspectorControls>
);
};

setUpShortcodeInspectorControls.propTypes = {
isSelected: PropTypes.bool,
attributes: PropTypes.object,
};

/**
* Get AMP Lightbox toggle control.
*
Expand Down Expand Up @@ -739,7 +608,7 @@ const AmpLightboxToggle = ( props ) => {

AmpLightboxToggle.propTypes = {
attributes: PropTypes.shape( {
ampLightbox: PropTypes.string,
ampLightbox: PropTypes.bool,
ampLayout: PropTypes.string,
linkTo: PropTypes.string,
} ),
Expand Down Expand Up @@ -770,7 +639,7 @@ const AmpCarouselToggle = ( props ) => {

AmpCarouselToggle.propTypes = {
attributes: PropTypes.shape( {
ampCarousel: PropTypes.string,
ampCarousel: PropTypes.bool,
} ),
setAttributes: PropTypes.func.isRequired,
};
Expand Down Expand Up @@ -821,12 +690,10 @@ const setUpGalleryInspectorControls = ( props ) => {
return null;
}

const hasThemeSupport = select( 'amp/block-editor' ).hasThemeSupport();

return (
<InspectorControls>
<PanelBody title={ __( 'AMP Settings', 'amp' ) }>
{ hasThemeSupport && <AmpCarouselToggle { ...props } /> }
<AmpCarouselToggle { ...props } />
<AmpLightboxToggle { ...props } />
</PanelBody>
</InspectorControls>
Expand All @@ -837,61 +704,6 @@ setUpGalleryInspectorControls.propTypes = {
isSelected: PropTypes.bool,
};

/**
* Removes amp-carousel=false from shortcode attributes.
*
* @param {string} shortcode Shortcode text.
*
* @return {string} Modified shortcode.
*/
export const removeAmpCarouselFromShortcodeAtts = ( shortcode ) => {
return shortcode.replace( ' amp-carousel=false', '' );
};

/**
* Removes amp-lightbox=true from shortcode attributes.
*
* @param {string} shortcode Shortcode text.
*
* @return {string} Modified shortcode.
*/
export const removeAmpLightboxFromShortcodeAtts = ( shortcode ) => {
return shortcode.replace( ' amp-lightbox=true', '' );
};

/**
* Determines whether a shortcode includes the amp-carousel attribute.
*
* @param {string} text Shortcode.
*
* @return {boolean} Whether the shortcode includes the attribute.
*/
export const hasGalleryShortcodeCarouselAttribute = ( text ) => {
return -1 !== text.indexOf( 'amp-carousel=false' );
};

/**
* Determines whether a shortcode includes the amp-lightbox attribute.
*
* @param {string} text Shortcode.
*
* @return {boolean} Whether the shortcode includes the attribute.
*/
export const hasGalleryShortcodeLightboxAttribute = ( text ) => {
return -1 !== text.indexOf( 'amp-lightbox=true' );
};

/**
* Determines whether the current shortcode is a gallery shortcode.
*
* @param {Object} attributes Shortcode attributes.
*
* @return {boolean} Whether it is a gallery shortcode.
*/
export const isGalleryShortcode = ( attributes ) => {
return attributes.text && -1 !== attributes.text.indexOf( 'gallery' );
};

/**
* Determines whether AMP is enabled for the current post or not.
*
Expand Down
3 changes: 1 addition & 2 deletions assets/src/block-editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import domReady from '@wordpress/dom-ready';
import { withFeaturedImageNotice } from '../common/components';
import { getMinimumFeaturedImageDimensions } from '../common/helpers';
import { withMediaLibraryNotice, AMPPreview } from './components';
import { addAMPAttributes, addAMPExtraProps, filterBlocksEdit, filterBlocksSave, renderPreviewButton } from './helpers';
import { addAMPAttributes, filterBlocksEdit, filterBlocksSave, renderPreviewButton } from './helpers';
import './store';

const {
Expand All @@ -31,7 +31,6 @@ plugins.keys().forEach( ( modulePath ) => {
addFilter( 'blocks.registerBlockType', 'ampEditorBlocks/addAttributes', addAMPAttributes );
addFilter( 'blocks.getSaveElement', 'ampEditorBlocks/filterSave', filterBlocksSave );
addFilter( 'editor.BlockEdit', 'ampEditorBlocks/filterEdit', filterBlocksEdit, 20 );
addFilter( 'blocks.getSaveContent.extraProps', 'ampEditorBlocks/addExtraAttributes', addAMPExtraProps );
addFilter( 'editor.PostFeaturedImage', 'ampEditorBlocks/withFeaturedImageNotice', withFeaturedImageNotice );
addFilter( 'editor.MediaUpload', 'ampEditorBlocks/withMediaLibraryNotice', ( InitialMediaUpload ) => withMediaLibraryNotice( InitialMediaUpload, getMinimumFeaturedImageDimensions() ) );

Expand Down
20 changes: 20 additions & 0 deletions includes/embeds/class-amp-core-block-handler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,26 @@ public function unregister_embed() {
* @return string Filtered block content.
*/
public function filter_rendered_block( $block_content, $block ) {

if ( isset( $block['attrs'] ) ) {
$injected_attributes = '';
$prop_attribute_mapping = [
'ampLayout' => 'data-amp-layout',
'ampNoLoading' => 'data-amp-noloading',
'ampLightbox' => 'data-amp-lightbox',
'ampCarousel' => 'data-amp-carousel',
];
foreach ( $prop_attribute_mapping as $prop => $attr ) {
if ( isset( $block['attrs'][ $prop ] ) ) {
$property_value = rest_sanitize_boolean( $block['attrs'][ $prop ] );
$injected_attributes .= sprintf( ' %s="%s"', $attr, esc_attr( wp_json_encode( $property_value ) ) );
}
}
if ( $injected_attributes ) {
$block_content = preg_replace( '/(<\w+)/', '$1' . $injected_attributes, $block_content, 1 );
}
}

if ( ! isset( $block['blockName'] ) ) {
return $block_content;
}
Expand Down
Loading