Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Gutenberg] 1008: Extend core blocks to add AMP-specific functionality. #1026

Merged
merged 33 commits into from
May 21, 2018
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
700b5cd
Initial take on extending Gutenberg core blocks to add ampLayout.
miina Mar 19, 2018
c7de18a
Add amp-runtime. Add amp-layout to Edit as well. Add option for using…
miina Mar 20, 2018
93f6a11
Use data-amp-layout.
miina Mar 21, 2018
55aa714
Add layout frontend support for amp-img and amp-audio added from Gute…
miina Mar 21, 2018
78ca3a9
Merge develop & resolve conflicts.
miina Apr 23, 2018
f023f73
Fixes to audio and video sanitizers for layout.
miina Apr 23, 2018
2e4cbd1
CS Fixes.
miina Apr 23, 2018
5bd7f07
Video amp-layout support.
miina Apr 23, 2018
011724e
Fix jshint issues.
miina Apr 23, 2018
e195c74
Start adding AMP layout conf for embeds.
miina Apr 24, 2018
73f4ad5
Layout for embeds.
miina Apr 25, 2018
fea3434
Add control to shortcode block for making use of amp-carousel optional.
miina Apr 26, 2018
7bf9df3
Add support for AMP noloading.
miina Apr 26, 2018
6c7f8e2
Fixes from review.
miina Apr 27, 2018
75f08c4
Add possible editor changes.
miina Apr 27, 2018
83b34a2
Remove incorrect changes.
miina Apr 27, 2018
703f8f1
Add tests.
miina May 1, 2018
f92e06b
Update dev-lib.
miina May 1, 2018
f271308
Merge develop & resolve conflicts.
miina May 1, 2018
deeea31
Update composer.lock to use XWP fork of PHP-CSS-Parser
westonruter May 1, 2018
c0ea186
Merge remote-tracking branch 'origin/develop' into add/1008-core_bloc…
miina May 4, 2018
b107296
Fix issue with setting fixed-height.
miina May 4, 2018
991c73c
Rework using CSS classes for AMP attributes.
miina May 4, 2018
9313375
Revert creating issue to test.
miina May 4, 2018
9e54106
Remove unnecessary code.
miina May 4, 2018
369650e
Restrict AMP attributes to media and embeds only.
miina May 4, 2018
fc7387f
Revert using CSS class for saving AMP attributes.
miina May 16, 2018
948e421
Fix tests.
miina May 16, 2018
589bf18
Fic phpcs.
miina May 16, 2018
7954b23
Let strings be translatable
westonruter May 19, 2018
dbc1c7c
Fixes from feedback.
miina May 21, 2018
e3dab51
Remove unused dynamicBlocks var.
miina May 21, 2018
f05bfb7
Eliminate inclusion of amp-runtime in Gutenberg editor since AMP comp…
westonruter May 21, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ function amp_init() {
AMP_Post_Type_Support::add_post_type_support();
add_filter( 'request', 'amp_force_query_var_value' );
add_action( 'admin_init', 'AMP_Options_Manager::register_settings' );
add_action( 'wp_loaded', 'amp_editor_core_blocks' );
add_action( 'wp_loaded', 'amp_post_meta_box' );
add_action( 'wp_loaded', 'amp_add_options_menu' );
add_action( 'parse_query', 'amp_correct_query_when_is_front_page' );
Expand Down
351 changes: 351 additions & 0 deletions assets/js/amp-editor-blocks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,351 @@
/* exported ampEditorBlocks */
/* eslint no-magic-numbers: [ "error", { "ignore": [ 1, -1, 0 ] } ] */

var ampEditorBlocks = ( function() {
var component = {

/**
* Holds data.
*/
data: {
dynamicBlocks: [],
ampLayoutOptions: [
{ value: 'nodisplay', label: 'No Display' },
Copy link
Member

Choose a reason for hiding this comment

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

Strings need localization.

{ 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: 600
}
};

/**
* Set data, add filters.
*
* @param {Array} data Data.
*/
component.boot = function boot( 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 );
wp.hooks.addFilter( 'blocks.getSaveContent.extraProps', 'ampEditorBlocks/addLayoutAttribute', component.addAMPExtraProps );
};

/**
* Get layout options depending on the block.
*
* @param {string} blockName Block name.
* @return {[*]} Options.
*/
component.getLayoutOptions = function getLayoutOptions( blockName ) {
var layoutOptions = [
{ value: '', label: 'None' }
Copy link
Member

Choose a reason for hiding this comment

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

Probably “Default” is more clear than “None”, since the latter could be confused with “No Display”.

],
embedBlocks = [
'core-embed/youtube',
'core-embed/facebook',
'core-embed/instagram'
];

_.each( component.data.ampLayoutOptions, function( option ) {
// Exclude options from layout that are not supported.
if ( 'core/image' === blockName || 'core/video' === blockName || 'core-embed/twitter' === blockName ) {
Copy link
Member

Choose a reason for hiding this comment

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

It may be easier to read if the logic in this each loop were broken up into isAvailable( blockName ) functions that are defined with each of the ampLayoutOptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed within dbc1c7c, used notAvailable param for ampLayoutOptions, let me know if isAvailable param seems to make more sense.

if ( 'container' === option.value ) {
return true;
Copy link
Member

Choose a reason for hiding this comment

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

This might as well just return.

}
} else if ( 'core/audio' === blockName ) {
if ( -1 !== [ 'responsive', 'fill', 'container', 'flex-item', 'intrinsic' ].indexOf( option.value ) ) {
return true;
}
} else if ( -1 !== embedBlocks.indexOf( blockName ) ) {
if ( 'container' === option.value || 'intrinsic' === option.value ) {
return true;
}
} else if (
'core-embed/vimeo' === blockName ||
'core-embed/dailymotion' === blockName ||
'core-embed/hulu' === blockName ||
'core-embed/reddit' === blockName
) {
if ( 'container' === option.value || 'intrinsic' === option.value || 'nodisplay' === option.value ) {
return true;
}
} else if ( 'core-embed/soundcloud' === blockName ) {
if ( 'fixed-height' !== option.value ) {
return true;
}
}

layoutOptions.push( { value: option.value, label: option.label } );
} );

return layoutOptions;
};

/**
* Add extra data-amp-layout attribute to save to DB.
*
* @param {Object} props Properties.
* @param {string} blockType Block type.
* @param {Object} attributes Attributes.
* @return {*} Props.
*/
component.addAMPExtraProps = function addAMPExtraProps( props, blockType, attributes ) {
if ( _.isEmpty( attributes.ampLayout ) ) {
return props;
}

return _.assign( { 'data-amp-layout': attributes.ampLayout }, props );
};

/**
* Add AMP attributes (in this test case just ampLayout) to every core block.
*
* @param {Object} settings Settings.
* @param {string} name Block name.
* @return {*} Settings.
*/
component.addAMPAttributes = function addAMPAttributes( settings, name ) {
var mediaBlocks = [
'core/image',
'core/video',
'core/audio'
];

// Gallery settings for shortcode.
if ( 'core/shortcode' === name ) {
if ( ! settings.attributes ) {
settings.attributes = {};
}
settings.attributes.ampCarousel = {
type: 'boolean'
};
}

// Layout settings for embeds and media blocks.
if ( 0 === name.indexOf( 'core-embed' ) || -1 !== mediaBlocks.indexOf( name ) ) {
if ( ! settings.attributes ) {
settings.attributes = {};
}
settings.attributes.ampLayout = {
type: 'string'
};
}
return settings;
};

/**
* Filters blocks edit function of all blocks.
*
* @param {Function} BlockEdit Edit function.
* @return {Function} Edit function.
*/
component.filterBlocksEdit = function filterBlocksEdit( BlockEdit ) {
var el = wp.element.createElement;

return function( props ) {
var attributes = props.attributes,
isSelected = props.isSelected,
name = props.name,
ampLayout,
inspectorControls,
width;

ampLayout = attributes.ampLayout;

if ( 'core/shortcode' === name ) {
// Lets remove amp-carousel from from edit view.
if ( component.hasGalleryShortcodeCarouselAttribute( attributes.text ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm getting a JS error here when first adding a shortcode because text prop is undefined. So I think the value passed into this function should be attributes.text || ''.

props.setAttributes( { text: component.removeAmpCarouselFromShortcodeAtts( attributes.text ) } );
}

inspectorControls = component.setUpShortcodeInspectorControls( props );
if ( '' === inspectorControls ) {
// Return original.
return [
el( BlockEdit, _.assign( {
key: 'original'
}, props ) )
];
}
} else {
inspectorControls = component.setUpInspectorControls( props );
}

// For editor view, add a wrapper to any tags except for embeds, these will break due to embedding logic.
if ( ! _.isEmpty( attributes.ampLayout ) && ! isSelected && -1 === name.indexOf( 'core-embed/' ) ) {
if ( 'fixed-height' === attributes.ampLayout ) {
width = 'auto';
} else {
width = component.data.defaultWidth;
}
// @todo Should we try to add width and height according to the layout?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@westonruter Another issue with AMP Layout is rendering in Gutenberg -- in case of embeds the content of the block in the editor is replaced once the content (e.g. from Youtube) has been loaded and the added attributes would get removed. The point being that currently looks like for trying to match the layout behavior in Gutenberg editor, possibly the whole edit method of the core block should be replaced. Or perhaps the width and height could be modified later, not sure. Also, since the block within Gutenberg is quite separated from the outer divs of the content, then likely it would still not show a matching result. Thoughts on showing the layout changes in Gutenberg?


return [
inspectorControls,
el( 'amp-layout',
{ key: 'amp', 'data-amp-layout': attributes.ampLayout, width: width, height: 400, children: el( BlockEdit, _.assign( {
key: 'original'
}, props ) ) }
)
];
}

// Return original.
Copy link
Member

Choose a reason for hiding this comment

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

Is this this the original or copied?

return [
inspectorControls,
el( BlockEdit, _.assign( {
key: 'original',
'data-amp-layout': ampLayout
}, props ) )
];
};
};

/**
* 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,
isSelected = props.isSelected,
name = props.name,
el = wp.element.createElement,
InspectorControls = wp.blocks.InspectorControls,
Copy link
Member

Choose a reason for hiding this comment

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

wp.blocks.InspectorControls is deprecated and will be removed from Gutenberg in 3.1. Please use wp.editor.InspectorControls instead.

SelectControl = wp.components.SelectControl;

return isSelected && (
el( InspectorControls, { key: 'inspector' },
el( SelectControl, {
label: 'AMP Layout',
value: ampLayout,
options: component.getLayoutOptions( name ),
onChange: function( value ) {
props.setAttributes( { ampLayout: value } );
}
} )
)
);
};

/**
* 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.blocks.InspectorControls,
ToggleControl = wp.components.ToggleControl,
PanelBody = wp.components.PanelBody,
toggleControl;

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
)
)
);
}

return '';
};

/**
* Filters blocks save function for core blocks except for dynamic blocks.
*
* @param {Object} element Element.
* @param {string} blockType Block type.
* @param {Object} attributes Attributes.
* @return {*} Output element.
*/
component.filterBlocksSave = function filterBlocksSave( element, blockType, attributes ) {
var text;
if ( 'core/shortcode' === blockType.name && component.isGalleryShortcode( attributes ) ) {
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
);
}

// Else lets return original.
return element;
}

// If the text already contains amp-carousel, return original.
if ( component.hasGalleryShortcodeCarouselAttribute( attributes.text ) ) {
return element;
}

// Add amp-carousel=false attribut to the shortcode.
text = attributes.text.replace( '[gallery', '[gallery amp-carousel=false' );

return wp.element.createElement(
wp.element.RawHTML,
{},
text
);
}

return element;
};

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

/**
* Check if shortcode includes amp-carousel attribute.
*
* @param {string} text Shortcode.
* @return {boolean} If has amp-carousel.
*/
component.hasGalleryShortcodeCarouselAttribute = function galleryShortcodeHasCarouselAttribute( text ) {
return -1 !== text.indexOf( 'amp-carousel=false' );
};

/**
* Check if shortcode is gallery shortcode.
*
* @param {Object} attributes Attributes.
* @return {boolean} If is gallery shortcode.
*/
component.isGalleryShortcode = function isGalleryShortcode( attributes ) {
return attributes.text && -1 !== attributes.text.indexOf( 'gallery' );
};

return component;
}() );
14 changes: 6 additions & 8 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading