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

[amp-stories sub-task] Add front-end animation. #1385

Merged
merged 31 commits into from
Sep 2, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
47b3f15
Ensure amp-force-sanitize-canonical notice is hidden when paired mode…
westonruter Aug 29, 2018
54b1272
Display when validation results are stale due to active theme/plugin …
westonruter Aug 29, 2018
6b5cb8b
Fix array key to check for stale plugins; italicize re-check row acti…
westonruter Aug 29, 2018
2b51ab1
Merge pull request #1375 from Automattic/add/staleness
westonruter Aug 29, 2018
5e9a535
Add animation controls for AMP Story layer blocks.
miina Aug 29, 2018
853f7fb
Merge pull request #1374 from Automattic/fix/built-in-support-notice
Aug 29, 2018
afa038c
Refactor AMP_Response_Headers and methods from AMP_Theme_Support into…
westonruter Aug 29, 2018
1870954
Fix handling of CORS requests
westonruter Aug 30, 2018
8518dae
Merge amp-stories and resolve conflicts.
miina Aug 30, 2018
10826df
Start adding animation for core blocks.
miina Aug 30, 2018
6c569c8
Add animation to core blocks. Whitelist animation attributes.
miina Aug 30, 2018
32b7524
Enable animations for CTA layer, too.
miina Aug 30, 2018
e111fa4
Update spec to file revision 720 (v1534879991178)
westonruter Aug 30, 2018
549c7b0
Improve ability to see differences in sanitized markup
westonruter Aug 30, 2018
e9ffbcb
Add tests that intentionally fail, showing reference point problems
westonruter Aug 30, 2018
d73ab0d
Replace duplicate test assertion
westonruter Aug 30, 2018
64cad60
Merge pull request #1382 from Automattic/fix/cors-requests
westonruter Aug 30, 2018
2eb230d
Eliminate whitelisted_attr_regex in favor of data-* attr check
westonruter Aug 30, 2018
596a0c7
Remove dead code: AMP_Rule_Spec::$node_types_to_remove_if_invalid
westonruter Aug 30, 2018
04ed6a7
Extract reference points from spec and use attr_spec_list on direct e…
westonruter Aug 31, 2018
4923569
Change animation settings source to attribute.
miina Aug 31, 2018
ad4ecbb
Replace deprecated isCleanNewPost.
miina Aug 31, 2018
002bda3
Fix special case of reference point attributes for amp-selector
westonruter Aug 31, 2018
38932ff
Split apart code from process_node into smaller get_rule_spec_list_to…
westonruter Aug 31, 2018
cb0fd56
Add tests for amp-viqeo-player, amp-image-slider, amp-pan-zoom, and a…
westonruter Aug 31, 2018
673206e
Fix comment regarding reference points being skipped
westonruter Aug 31, 2018
c3d5c34
Merge pull request #1387 from Automattic/fix/deprecated_isCleanNewPost
Aug 31, 2018
0a2916d
Improve docs for AMP_Tag_And_Attribute_Sanitizer::process_node()
westonruter Sep 2, 2018
c2a297c
Merge pull request #1386 from Automattic/update/spec-1534879991178
westonruter Sep 2, 2018
224ef00
Revert "Whitelist animation attributes."
westonruter Sep 2, 2018
def0ce0
Merge branch 'develop' of https://github.com/Automattic/amp-wp into a…
westonruter Sep 2, 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
180 changes: 160 additions & 20 deletions assets/js/amp-story-editor-blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,84 @@ var ampStoryEditorBlocks = ( function() { // eslint-disable-line no-unused-vars
value: 'lower-third',
label: __( 'Lower Third', 'amp' )
}
],
ampAnimationTypeOptions: [
Copy link
Member

Choose a reason for hiding this comment

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

This list is duplicated with the array in getAmpStoryAnimationControls, is it not? I think there should be a common getAnimationTypeOptions function in the utils module to eliminate this duplication. Even if the list is not exactly the same, the function could still return the full list and then you could filter out the ones that aren't allowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is duplicated, however, the utils.js gets compiled, not sure what would be the best way to use the module like this within amp-story-editor-blocks.js, thoughts?

Created a separate helpers file for AMP Stories specifically within 4923569 since the utils.js always gets enqueued, however, getAmpStoryAnimationControls is needed for AMP Stories post type only, thought that then maybe we could add an inline script for defining the getAnimationTypeOptions globally to avoid duplication but not sure if that would be a good way to go.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, right. So this is actually dependent on #1298 which will convert amp-story-editor-blocks.js into a module. So once that happens then this problem would go away, right?

{
value: '',
label: __( 'None', 'amp' )
},
{
value: 'drop',
label: __( 'Drop', 'amp' )
},
{
value: 'fade-in',
label: __( 'Fade In', 'amp' )
},
{
value: 'fly-in-bottom',
label: __( 'Fly In Bottom', 'amp' )
},
{
value: 'fly-in-left',
label: __( 'Fly In Left', 'amp' )
},
{
value: 'fly-in-right',
label: __( 'Fly In Right', 'amp' )
},
{
value: 'fly-in-top',
label: __( 'Fly In Top', 'amp' )
},
{
value: 'pulse',
label: __( 'Pulse', 'amp' )
},
{
value: 'rotate-in-left',
label: __( 'Rotate In Left', 'amp' )
},
{
value: 'rotate-in-right',
label: __( 'Rotate In Right', 'amp' )
},
{
value: 'twirl-in',
label: __( 'Twirl In', 'amp' )
},
{
value: 'whoosh-in-left',
label: __( 'Whoosh In Left', 'amp' )
},
{
value: 'whoosh-in-right',
label: __( 'Whoosh In Right', 'amp' )
},
{
value: 'pan-left',
label: __( 'Pan Left', 'amp' )
},
{
value: 'pan-right',
label: __( 'Pan Right', 'amp' )
},
{
value: 'pan-down',
label: __( 'Pan Down', 'amp' )
},
{
value: 'pan-up',
label: __( 'Pan Up', 'amp' )
},
{
value: 'zoom-in',
label: __( 'Zoom In', 'amp' )
},
{
value: 'zoom-out',
label: __( 'Zoom Out', 'amp' )
}
]
}
};
Expand Down Expand Up @@ -119,6 +197,16 @@ var ampStoryEditorBlocks = ( function() { // eslint-disable-line no-unused-vars
if ( attributes.ampStoryPosition ) {
ampAttributes[ 'grid-area' ] = attributes.ampStoryPosition;
}
if ( attributes.ampAnimationType ) {
ampAttributes[ 'animate-in' ] = attributes.ampAnimationType;

if ( attributes.ampAnimationDelay ) {
ampAttributes[ 'animate-in-delay' ] = attributes.ampAnimationDelay + 'ms';
}
if ( attributes.ampAnimationDuration ) {
ampAttributes[ 'animate-in-duration' ] = attributes.ampAnimationDuration + 'ms';
}
}

return _.extend( ampAttributes, props );
};
Expand All @@ -131,14 +219,26 @@ var ampStoryEditorBlocks = ( function() { // eslint-disable-line no-unused-vars
* @return {Object} Settings.
*/
component.addAMPAttributes = function addAMPAttributes( settings, name ) {
// Add the "thirds" template position option.
// Add the "thirds" template position option and animation settings.
if ( -1 !== component.data.allowedBlocks.indexOf( name ) ) {
if ( ! settings.attributes ) {
settings.attributes = {};
}
settings.attributes.ampStoryPosition = {
type: 'string'
};
// @todo We could map all the blocks to their tag and use attribute as source instead.
Copy link
Member

Choose a reason for hiding this comment

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

Yes, this would be good. Is it not done at the moment because AMP requires the value to have the time unit, e.g. ms or s, whereas the control populates the pop with an integer?

Maybe the hpq library allows a custom matcher function to be used for a given attribute to parse its value.

Or, if integer could be as the type maybe this could be automatic since 123 === parseInt('123ms')?

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe it's rather because we don't know the element name to select?

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 these settings to source: attribute by using parseInt() within 4923569. Exception is for core/list since it could be either ul or ol.

settings.attributes.ampAnimationType = {
type: 'string'
};
settings.attributes.ampAnimationDelay = {
type: 'number',
default: 0
};
settings.attributes.ampAnimationDuration = {
type: 'number',
default: 0
};
}
return settings;
};
Expand Down Expand Up @@ -183,27 +283,28 @@ var ampStoryEditorBlocks = ( function() { // eslint-disable-line no-unused-vars
}

if ( 'thirds' !== parentBlock.attributes.template ) {
// Return original.
return [
el( BlockEdit, _.extend( {
key: 'original'
}, props ) )
];
inspectorControls = el( InspectorControls, { key: 'inspector' },
el( PanelBody, { title: __( 'AMP Story Settings', 'amp' ), key: 'amp-story' },
component.getAnimationControls( props )
)
);
} else {
inspectorControls = el( InspectorControls, { key: 'inspector' },
el( PanelBody, { title: __( 'AMP Story Settings', 'amp' ), key: 'amp-story' },
el( SelectControl, {
key: 'position',
label: __( 'Placement', 'amp' ),
value: attributes.ampStoryPosition,
options: component.data.ampStoryPositionOptions,
onChange: function( value ) {
props.setAttributes( { ampStoryPosition: value } );
}
} ),
component.getAnimationControls( props )
)
);
}

inspectorControls = el( InspectorControls, { key: 'inspector' },
el( PanelBody, { title: __( 'AMP Story Settings', 'amp' ) },
el( SelectControl, {
label: __( 'Placement', 'amp' ),
value: attributes.ampStoryPosition,
options: component.data.ampStoryPositionOptions,
onChange: function( value ) {
props.setAttributes( { ampStoryPosition: value } );
}
} )
)
);

return [
inspectorControls,
el( BlockEdit, _.extend( {
Expand All @@ -213,5 +314,44 @@ var ampStoryEditorBlocks = ( function() { // eslint-disable-line no-unused-vars
};
};

component.getAnimationControls = function getAnimationControls( props ) {
var RangeControl = wp.components.RangeControl,
el = wp.element.createElement,
SelectControl = wp.components.SelectControl,
attributes = props.attributes;

return [
el( SelectControl, {
key: 'animation-type',
label: __( 'Animation type', 'amp' ),
value: attributes.ampAnimationType,
options: component.data.ampAnimationTypeOptions,
onChange: function( value ) {
props.setAttributes( { ampAnimationType: value } );
}
} ),
el( RangeControl, {
key: 'animation-duration',
label: __( 'Animation duration (ms)', 'amp' ),
value: attributes.ampAnimationDuration,
min: 0,
max: 5000,
onChange: function( value ) {
props.setAttributes( { ampAnimationDuration: value } );
}
} ),
el( RangeControl, {
key: 'animation-delay',
label: __( 'Animation delay (ms)', 'amp' ),
value: attributes.ampAnimationDelay,
min: 0,
max: 5000,
onChange: function( value ) {
props.setAttributes( { ampAnimationDelay: value } );
}
} )
];
};

return component;
}() );
48 changes: 44 additions & 4 deletions blocks/amp-story/amp-story-cta-layer.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { getAmpStoryAnimationControls } from '../utils';

const { __ } = wp.i18n;
const {
registerBlockType
} = wp.blocks;
const {
InspectorControls,
InnerBlocks
} = wp.editor;
const {
Notice
Notice,
PanelBody
} = wp.components;
const { Component } = wp.element;

Expand Down Expand Up @@ -50,6 +54,24 @@ export default registerBlockType(
category: 'layout',
icon: 'grid-view',
parent: [ 'amp/amp-story-page' ],

attributes: {
animationType: {
type: 'string',
source: 'attribute',
selector: 'amp-story-grid-layer',
attribute: 'animate-in'
},
animationDuration: {
type: 'number',
default: 0
},
animationDelay: {
type: 'number',
default: 0
}
},

inserter: false,

/*
Expand Down Expand Up @@ -99,9 +121,16 @@ export default registerBlockType(
<Notice status="error" isDismissible={ false }>{ __( 'Multiple CTA Layers are not allowed. Please remove all but one.', 'amp' ) }</Notice>
);
}
return (
return [
<InspectorControls key='controls'>
<PanelBody key='animation' title={ __( 'CTA Layer Animation' ) }>
Copy link
Member

Choose a reason for hiding this comment

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

Missing amp text domain.

{
getAmpStoryAnimationControls( this.props.setAttributes, this.props.attributes )
}
</PanelBody>
</InspectorControls>,
<InnerBlocks key='contents' allowedBlocks={ ALLOWED_BLOCKS } />
);
];
}

hasMoreThanOneCtaBlock() {
Expand All @@ -120,8 +149,19 @@ export default registerBlockType(
},

save( { attributes } ) {
let layerProps = {};
if ( attributes.animationType ) {
layerProps[ 'animate-in' ] = attributes.animationType;

if ( attributes.animationDelay ) {
layerProps[ 'animate-in-delay' ] = attributes.animationDelay + 'ms';
}
if ( attributes.animationDuration ) {
layerProps[ 'animate-in-duration' ] = attributes.animationDuration + 'ms';
}
}
return (
<amp-story-cta-layer template={ attributes.template }>
<amp-story-cta-layer { ...layerProps }>
<InnerBlocks.Content />
</amp-story-cta-layer>
);
Expand Down
44 changes: 40 additions & 4 deletions blocks/amp-story/amp-story-grid-layer.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getAmpStoryAnimationControls } from '../utils';

const { __ } = wp.i18n;
const {
registerBlockType
Expand All @@ -7,7 +9,8 @@ const {
InnerBlocks
} = wp.editor;
const {
SelectControl
SelectControl,
PanelBody
} = wp.components;

const ALLOWED_BLOCKS = [
Expand Down Expand Up @@ -59,6 +62,20 @@ export default registerBlockType(
selector: 'amp-story-grid-layer',
attribute: 'template',
default: 'vertical'
},
animationType: {
type: 'string',
source: 'attribute',
selector: 'amp-story-grid-layer',
attribute: 'animate-in'
},
animationDuration: {
type: 'number',
default: 0
},
animationDelay: {
type: 'number',
default: 0
}
},

Expand All @@ -73,13 +90,13 @@ export default registerBlockType(
*/

edit( props ) {
const { setAttributes } = props;
const { setAttributes, attributes } = props;
return [
<InspectorControls key='inspector'>
<SelectControl
key="template"
label={ __( 'Template', 'amp' ) }
value={ props.attributes.template }
value={ attributes.template }
options={ [
{
value: 'vertical',
Expand All @@ -100,6 +117,11 @@ export default registerBlockType(
] }
onChange={ value => ( setAttributes( { template: value } ) ) }
/>
<PanelBody key='animation' title={ __( 'Grid Layer Animation' ) }>
Copy link
Member

Choose a reason for hiding this comment

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

Missing amp text domain.

{
getAmpStoryAnimationControls( setAttributes, attributes )
}
</PanelBody>
</InspectorControls>,
<div key='contents' className={ 'amp-grid-template amp-grid-template-' + props.attributes.template }>
<InnerBlocks allowedBlocks={ ALLOWED_BLOCKS } />
Expand All @@ -108,8 +130,22 @@ export default registerBlockType(
},

save( { attributes } ) {
let layerProps = {
template: attributes.template
};
if ( attributes.animationType ) {
layerProps[ 'animate-in' ] = attributes.animationType;

if ( attributes.animationDelay ) {
layerProps[ 'animate-in-delay' ] = attributes.animationDelay + 'ms';
}
if ( attributes.animationDuration ) {
layerProps[ 'animate-in-duration' ] = attributes.animationDuration + 'ms';
}
}

return (
<amp-story-grid-layer template={ attributes.template }>
<amp-story-grid-layer { ...layerProps }>
<InnerBlocks.Content />
</amp-story-grid-layer>
);
Expand Down
Loading