Skip to content

Commit

Permalink
Fixes from feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
miina committed May 21, 2018
1 parent 7954b23 commit dbc1c7c
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 112 deletions.
138 changes: 68 additions & 70 deletions assets/js/amp-editor-blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,39 +16,68 @@ var ampEditorBlocks = ( function() {
ampLayoutOptions: [
{
value: 'nodisplay',
label: __( 'No Display' )
label: __( 'No Display' ),
notAvailable: [
'core-embed/vimeo',
'core-embed/dailymotion',
'core-embed/hulu',
'core-embed/reddit',
'core-embed/soundcloud'
]
},
{
// Not supported by amp-audio and amp-pixel.
value: 'fixed',
label: __( 'Fixed' )
label: __( 'Fixed' ),
notAvailable: [
'core-embed/soundcloud'
]
},
{
// To ensure your AMP element displays, you must specify a width and height for the containing element.
value: 'responsive',
label: __( 'Responsive' )
label: __( 'Responsive' ),
notAvailable: [
'core/audio',
'core-embed/soundcloud'
]
},
{
value: 'fixed-height',
label: __( 'Fixed height' )
label: __( 'Fixed height' ),
notAvailable: []
},
{
value: 'fill',
label: __( 'Fill' )
},
{
// Not supported by img and video.
value: 'container',
label: __( 'Container' )
label: __( 'Fill' ),
notAvailable: [
'core/audio',
'core-embed/soundcloud'
]
},
{
value: 'flex-item',
label: __( 'Flex Item' )
label: __( 'Flex Item' ),
notAvailable: [
'core/audio',
'core-embed/soundcloud'
]
},
{
// Not supported by video.
value: 'intrinsic',
label: __( 'Intrinsic' )
label: __( 'Intrinsic' ),
notAvailable: [
'core/audio',
'core-embed/youtube',
'core-embed/facebook',
'core-embed/instagram',
'core-embed/vimeo',
'core-embed/dailymotion',
'core-embed/hulu',
'core-embed/reddit',
'core-embed/soundcloud'
]
}
],
defaultWidth: 608, // Max-width in the editor.
Expand All @@ -75,59 +104,38 @@ var ampEditorBlocks = ( function() {
wp.hooks.addFilter( 'blocks.getSaveContent.extraProps', 'ampEditorBlocks/addExtraAttributes', component.addAMPExtraProps );
};

/**
* Check if layout is available for the block.
*
* @param {string} blockName Block name.
* @param {Object} option Layout option object.
* @return {boolean} If is available.
*/
component.isLayoutAvailable = function isLayoutAvailable( blockName, option ) {
return -1 === option.notAvailable.indexOf( blockName );
};

/**
* Get layout options depending on the block.
*
* @param {string} blockName Block name.
* @return {[*]} Options.
*/
component.getLayoutOptions = function getLayoutOptions( blockName ) {
var layoutOptions, embedBlocks;
layoutOptions = [
var layoutOptions = [
{
value: '',
label: __( 'Default' )
}
];
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 ) {
if ( 'container' === option.value ) {
return;
}
} else if ( 'core/audio' === blockName ) {
if ( -1 !== [ 'responsive', 'fill', 'container', 'flex-item', 'intrinsic' ].indexOf( option.value ) ) {
return;
}
} else if ( -1 !== embedBlocks.indexOf( blockName ) ) {
if ( 'container' === option.value || 'intrinsic' === option.value ) {
return;
}
} 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;
}
} else if ( 'core-embed/soundcloud' === blockName ) {
if ( 'fixed-height' !== option.value ) {
return;
}
if ( component.isLayoutAvailable( blockName, option ) ) {
layoutOptions.push( {
value: option.value,
label: option.label
} );
}

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

return layoutOptions;
Expand Down Expand Up @@ -221,23 +229,17 @@ var ampEditorBlocks = ( function() {
inspectorControls = component.setUpInspectorControls( props );
}

if ( attributes.ampLayout ) {
if ( 'core/image' === name ) {
component.setImageBlockLayoutAttributes( props, attributes.ampLayout, inspectorControls );
} else if ( 'nodisplay' === attributes.ampLayout ) {
return [
inspectorControls
];
}
// Return just inspector controls in case of 'nodisplay'.
if ( ampLayout && 'nodisplay' === ampLayout ) {
return [
inspectorControls
];
}

// Return original.
return [
inspectorControls,
el( BlockEdit, _.extend( {
key: 'original',
'data-amp-layout': ampLayout,
style: 'height:100px;'
key: 'original'
}, props ) )
];
};
Expand All @@ -248,10 +250,8 @@ var ampEditorBlocks = ( function() {
*
* @param {Object} props Props.
* @param {string} layout Layout.
* @param {Object} inspectorControls Inspector controls.
* @return {[*]} Void or block edit element.
*/
component.setImageBlockLayoutAttributes = function setImageBlockLayoutAttributes( props, layout, inspectorControls ) {
component.setImageBlockLayoutAttributes = function setImageBlockLayoutAttributes( props, layout ) {
var attributes = props.attributes;
switch ( layout ) {
case 'fixed-height':
Expand All @@ -268,11 +268,6 @@ var ampEditorBlocks = ( function() {
props.setAttributes( { width: component.data.defaultWidth } );
}
break;

case 'nodisplay':
return [
inspectorControls
];
}
};

Expand Down Expand Up @@ -307,6 +302,9 @@ var ampEditorBlocks = ( function() {
options: component.getLayoutOptions( name ),
onChange: function( value ) {
props.setAttributes( { ampLayout: value } );
if ( 'core/image' === props.name ) {
component.setImageBlockLayoutAttributes( props, value );
}
}
} ),
el( ToggleControl, {
Expand Down
8 changes: 4 additions & 4 deletions includes/admin/class-amp-editor-blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ public function init() {
/**
* Whitelist used data-amp-* attributes.
*
* @param array $context Array of contexts.
* @param array $tags Array of allowed post tags.
* @return mixed Modified array.
*/
public function whitelist_layout_in_wp_kses_allowed_html( $context ) {
foreach ( $context as $tag ) {
public function whitelist_layout_in_wp_kses_allowed_html( $tags ) {
foreach ( $tags as &$tag ) {
$tag['data-amp-layout'] = true;
$tag['data-amp-noloading'] = true;
}
return $context;
return $tags;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion includes/sanitizers/class-amp-audio-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function sanitize() {
$node = $nodes->item( $i );
$amp_data = $this->get_data_amp_attributes( $node );
$old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node );
$old_attributes = $this->set_data_amp_attributes( $old_attributes, $amp_data );
$old_attributes = $this->filter_data_amp_attributes( $old_attributes, $amp_data );

$new_attributes = $this->filter_attributes( $old_attributes );

Expand Down
30 changes: 2 additions & 28 deletions includes/sanitizers/class-amp-base-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ public function get_data_amp_attributes( $node ) {
* @param array $amp_data Array of AMP attributes.
* @return array Updated attributes.
*/
public function set_data_amp_attributes( $attributes, $amp_data ) {
public function filter_data_amp_attributes( $attributes, $amp_data ) {
if ( isset( $amp_data['layout'] ) ) {
$attributes['data-amp-layout'] = $amp_data['layout'];
}
Expand All @@ -395,33 +395,7 @@ public function set_data_amp_attributes( $attributes, $amp_data ) {
* @param string $layout Layout.
* @return array New attributes.
*/
public function set_attachment_layout_attributes( $node, $new_attributes, $layout ) {

// 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'];
}
}
}
public function filter_attachment_layout_attributes( $node, $new_attributes, $layout ) {

// The width has to be unset / auto in case of fixed-height.
if ( 'fixed-height' === $layout ) {
Expand Down
4 changes: 2 additions & 2 deletions includes/sanitizers/class-amp-img-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,11 @@ private function adjust_and_replace_node( $node ) {

$amp_data = $this->get_data_amp_attributes( $node );
$old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node );
$old_attributes = $this->set_data_amp_attributes( $old_attributes, $amp_data );
$old_attributes = $this->filter_data_amp_attributes( $old_attributes, $amp_data );

$new_attributes = $this->filter_attributes( $old_attributes );
$layout = isset( $amp_data['layout'] ) ? $amp_data['layout'] : false;
$new_attributes = $this->set_attachment_layout_attributes( $node, $new_attributes, $layout );
$new_attributes = $this->filter_attachment_layout_attributes( $node, $new_attributes, $layout );

$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'] ) ) {
Expand Down
39 changes: 37 additions & 2 deletions includes/sanitizers/class-amp-video-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@ public function sanitize() {
$node = $nodes->item( $i );
$amp_data = $this->get_data_amp_attributes( $node );
$old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node );
$old_attributes = $this->set_data_amp_attributes( $old_attributes, $amp_data );
$old_attributes = $this->filter_data_amp_attributes( $old_attributes, $amp_data );

$new_attributes = $this->filter_attributes( $old_attributes );
$layout = isset( $amp_data['layout'] ) ? $amp_data['layout'] : false;
$new_attributes = $this->set_attachment_layout_attributes( $node, $new_attributes, $layout );
$new_attributes = $this->filter_video_dimensions( $new_attributes );
$new_attributes = $this->filter_attachment_layout_attributes( $node, $new_attributes, $layout );
$new_attributes = $this->set_layout( $new_attributes );
if ( empty( $new_attributes['layout'] ) && ! empty( $new_attributes['width'] ) && ! empty( $new_attributes['height'] ) ) {
$new_attributes['layout'] = 'responsive';
Expand Down Expand Up @@ -108,6 +109,40 @@ public function sanitize() {
}
}

/**
* Filter video dimensions, try to get width and height from original file if missing.
*
* @param array $new_attributes Attributes.
* @return array Modified attributes.
*/
protected function filter_video_dimensions( $new_attributes ) {
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'];
}
}
}
return $new_attributes;
}

/**
* "Filter" HTML attributes for <amp-audio> elements.
*
Expand Down
Loading

0 comments on commit dbc1c7c

Please sign in to comment.