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

[amp-stories sub-task] Grid layers layout #1347

Merged
merged 18 commits into from
Aug 29, 2018
Merged

[amp-stories sub-task] Grid layers layout #1347

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
de8a1c5
Modify CSS for having the layer on top of each other and hiding the t…
miina Aug 20, 2018
954c14e
Merge remote-tracking branch 'origin/amp-stories' into amp-story/layouts
miina Aug 20, 2018
87cca23
Merge remote-tracking branch 'origin/develop' into amp-story/layouts
miina Aug 20, 2018
9d79e01
Add initial take on BlockSelector for selecting layers.
miina Aug 21, 2018
6ec83eb
Merge branch 'amp-stories' into amp-story/layouts
westonruter Aug 21, 2018
83d2c34
Use Button instead of links. Make strings translatable.
miina Aug 21, 2018
2774a1a
Merge branch 'amp-story/layouts' of github.com:Automattic/amp-wp into…
miina Aug 21, 2018
c7951bd
Add grid system to the grid layers. Place layers exactly on top of ea…
miina Aug 22, 2018
4fa18d3
Add inserter for grid layers.
miina Aug 22, 2018
bfdefb9
Fix style for fill template. Remove importing lodash since it breaks …
miina Aug 22, 2018
cd4d801
Fix eslint.
miina Aug 22, 2018
b9b410c
Add CTA Layer to the block inserter dropdown.
miina Aug 22, 2018
e5c84d0
Extend blocks to allow configuring the position in thirds template.
miina Aug 23, 2018
912439d
Merge branch 'amp-stories' into amp-story/layouts
miina Aug 29, 2018
6ff0bf6
Add a partly working workaround for selectBlock issue.
miina Aug 29, 2018
b486847
Merge amp-stories and resolve conflicts.
miina Aug 29, 2018
b7afb23
Ensure a singular AMP story always is_amp_endpoint
westonruter Aug 29, 2018
dc87187
Ensure amp-runtime and Gutenberg block styles are included on story t…
westonruter Aug 29, 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
58 changes: 50 additions & 8 deletions assets/css/amp-editor-story-blocks.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,56 @@
div[data-type="amp/amp-story-page"] {
border: 1px solid #ffdddd;
}

.editor-block-list__layout {
background-color: #ffdddd;
div[data-type="amp/amp-story-page"],
div[data-type="amp/amp-story-page"] .editor-inner-blocks .editor-block-list__layout {
min-height: 450px;
Copy link
Member

Choose a reason for hiding this comment

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

On desktop an AMP story has the following style rule:

[desktop]>amp-story-page {
    margin: auto!important;
    max-height: 75vh!important;
    max-width: 45vh!important;
    min-width: 320px!important;
    min-height: 533px!important;
}

Would that ensure that the page is displayed with the right aspect ratio and dimensions?

}
.editor-block-list__layout .editor-block-list__layout {
background-color: white;
div[data-type="amp/amp-story-page"] .editor-inner-blocks .editor-block-list__layout .editor-block-list__layout {
background-color: initial;
min-height: initial;
}
div[data-type="amp/amp-story-page"] {
background-color: #ddffdd;

.editor-block-list__layout div[data-type="amp/amp-story-grid-layer"] {
position: absolute;
height: 100%;
width: 100%;
}

.editor-block-list__layout div[data-type="amp/amp-story-cta-layer"] {
position: absolute;
height: 20%;
width: 100%;
bottom: 0;
}

.editor-block-list__layout div[data-type="amp/amp-story-grid-layer"],
.editor-block-list__layout div[data-type="amp/amp-story-cta-layer"] {
border: 1px solid #ddffdd;
}
div[data-type="amp/amp-story-grid-layer"] {
background-color: #ddddff;

.editor-block-list__layout div[data-type="amp/amp-story-grid-layer"].is-selected ~ div[data-type="amp/amp-story-grid-layer"],
.editor-block-list__layout div[data-type="amp/amp-story-grid-layer"].is-selected-parent ~ div[data-type="amp/amp-story-grid-layer"]{
display: none;
}

.editor-selectors {
position: absolute;
right: -150px;
bottom: 0px;
width: 100px;
z-index: 80;
}

.editor-selectors .component-editor__selector {
margin-bottom: 2px;
}

.editor-selectors .component-editor__selector a {
color: #656565;
font-size: 13px;
}

.editor-selectors .is-selected.component-editor__selector a {
text-decoration: underline;
}
7 changes: 4 additions & 3 deletions blocks/amp-story/amp-story-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import memoize from 'memize';
import uuid from 'uuid/v4';
import BlockSelector from './block-selector';

const { __ } = wp.i18n;
const {
Expand Down Expand Up @@ -27,7 +28,6 @@ const ALLOWED_BLOCKS = [
const getStoryPageTemplate = memoize( ( grids, hasCTA ) => {
let template = _.times( grids, () => [
'amp/amp-story-grid-layer',
[],
[
[
'core/paragraph',
Expand Down Expand Up @@ -101,10 +101,11 @@ export default registerBlockType(
grids = 1;
}

return (
return [
<BlockSelector key="selectors" rootClientId={ props.clientId } />,
// Get the template dynamically.
<InnerBlocks key='contents' template={ getStoryPageTemplate( grids, hasCTALayer ) } allowedBlocks={ ALLOWED_BLOCKS } />
);
];
},

save( { attributes } ) {
Expand Down
73 changes: 73 additions & 0 deletions blocks/amp-story/block-selector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import forEachRight from 'lodash'; // eslint-disable-line no-unused-vars

const { Component } = wp.element;
const { getBlockType } = wp.blocks;
const {
dispatch,
select
} = wp.data;
const {
getBlock,
isBlockSelected,
hasSelectedInnerBlock,
getSelectedBlock
} = select( 'core/editor' );
const {
selectBlock
} = dispatch( 'core/editor' );

class BlockSelector extends Component {
render() {
if ( ! this.props.rootClientId ) {
return null;
}

const rootBlock = getBlock( this.props.rootClientId );

if ( ! rootBlock.innerBlocks.length ) {
return null;
}

let links = [];

_.forEachRight( rootBlock.innerBlocks, function( block, index ) {
let className = 'component-editor__selector';
if ( isBlockSelected( block.clientId ) || hasSelectedInnerBlock( block.clientId ) ) {
className += ' is-selected';
}
let blockType = getBlockType( block.name );
links.push(
<li className={ className } key={ 'selector-' + index }>
<a onClick={ ( e ) => {
Copy link
Member

Choose a reason for hiding this comment

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

Let's have this be a button so that it can be focused and accessible.

e.stopPropagation();
if ( getSelectedBlock.clientId !== block.clientId ) {
// @todo This selects the first inner child instead for some reason.
selectBlock( block.clientId );
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 For some reason selectBlock here is triggering selectBlock for its first inner block, too, and ends up selecting the first inner block of the layer (e.g. the first paragraph within the layer block) instead of the layer itself. Any thoughts on why this might happen? (If not, I'll just continue looking into it, perhaps it's an issue to report.)

Copy link
Member

Choose a reason for hiding this comment

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

I don't know why. Is this why I am unable to insert a grid layer upon selecting the page?

}
}}>{ blockType.title }</a>
Copy link
Member

Choose a reason for hiding this comment

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

This can just be “Layer” perhaps. In the end, I don't think that we'll be displaying the label and that we'd just have an icon instead, with perhaps a tooltip.

</li>
);
} );

let className = 'component-editor__selector';
if ( isBlockSelected( this.props.rootClientId ) ) {
className += ' is-selected';
}

links.push(
<li className={ className } key='page-selector'>
<a onClick={ () => {
Copy link
Member

Choose a reason for hiding this comment

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

Per above, this should be button.

selectBlock( this.props.rootClientId );
}}>Page</a>
Copy link
Member

Choose a reason for hiding this comment

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

“Page” here needs to be translatable.

</li>
);

return (
<ul className="editor-selectors">
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be an ol since the layers are ordered.

{ links }
</ul>
);
}
}

export default BlockSelector;
7 changes: 7 additions & 0 deletions includes/sanitizers/class-amp-base-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,13 @@ public function prepare_validation_error( array $error = array(), array $data =
if ( 'script' === $node->nodeName && ! $node->hasAttribute( 'src' ) ) {
$error['text'] = $node->textContent;
}

// Suppress 'ver' param from enqueued scripts and styles.
if ( 'script' === $node->nodeName && isset( $error['node_attributes']['src'] ) && false !== strpos( $error['node_attributes']['src'], 'ver=' ) ) {
$error['node_attributes']['src'] = add_query_arg( 'ver', '__normalized__', $error['node_attributes']['src'] );
} elseif ( 'link' === $node->nodeName && isset( $error['node_attributes']['href'] ) && false !== strpos( $error['node_attributes']['href'], 'ver=' ) ) {
$error['node_attributes']['href'] = add_query_arg( 'ver', '__normalized__', $error['node_attributes']['href'] );
}
} elseif ( $node instanceof DOMAttr ) {
if ( ! isset( $error['code'] ) ) {
$error['code'] = AMP_Validation_Error_Taxonomy::INVALID_ATTRIBUTE_CODE;
Expand Down
12 changes: 10 additions & 2 deletions tests/test-class-amp-base-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,16 +206,18 @@ public function test_remove_invalid_child() {
$parent_tag_name = 'div';
$dom_document = new DOMDocument( '1.0', 'utf-8' );
$parent = $dom_document->createElement( $parent_tag_name );
$child = $dom_document->createElement( 'h1' );
$child = $dom_document->createElement( 'script' );
$child->setAttribute( 'id', 'foo' );
$child->setAttribute( 'src', 'http://example.com/bad.js?ver=123' );
$parent->appendChild( $child );

$expected_error = array(
'code' => AMP_Validation_Error_Taxonomy::INVALID_ELEMENT_CODE,
'node_name' => $child->nodeName,
'parent_name' => $parent_tag_name,
'node_attributes' => array(
'id' => 'foo',
'id' => 'foo',
'src' => 'http://example.com/bad.js?ver=__normalized__',
),
'foo' => 'bar',
'sources' => null,
Expand All @@ -242,6 +244,12 @@ public function test_remove_invalid_child() {
remove_filter( 'amp_validation_error_sanitized', '__return_true' );

// Test unsanitized.
$child = $dom_document->createElement( 'link' );
$child->setAttribute( 'id', 'foo' );
$child->setAttribute( 'href', 'http://example.com/bad.css?ver=123' );
$expected_error['node_name'] = 'link';
unset( $expected_error['node_attributes']['src'] );
$expected_error['node_attributes']['href'] = 'http://example.com/bad.css?ver=__normalized__';
add_filter( 'amp_validation_error_sanitized', '__return_false' );
AMP_Validation_Manager::reset_validation_results();
$parent->appendChild( $child );
Expand Down
2 changes: 1 addition & 1 deletion tests/validation/test-class-amp-validation-manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ public function get_block_data() {
),
'latest_posts' => array(
'<!-- wp:latest-posts /-->',
'<!--amp-source-stack {"block_name":"core\/latest-posts","post_id":{{post_id}},"block_content_index":0,"type":"plugin","name":"gutenberg","function":"render_block_core_latest_posts"}--><ul class="wp-block-latest-posts aligncenter"><li><a href="{{url}}">{{title}}</a></li></ul><!--/amp-source-stack {"block_name":"core\/latest-posts","post_id":{{post_id}},"type":"plugin","name":"gutenberg","function":"render_block_core_latest_posts"}-->',
'<!--amp-source-stack {"block_name":"core\/latest-posts","post_id":{{post_id}},"block_content_index":0,"type":"plugin","name":"gutenberg","function":"render_block_core_latest_posts"}--><ul class="wp-block-latest-posts"><li><a href="{{url}}">{{title}}</a></li></ul><!--/amp-source-stack {"block_name":"core\/latest-posts","post_id":{{post_id}},"type":"plugin","name":"gutenberg","function":"render_block_core_latest_posts"}-->',
array(
'element' => 'ul',
'blocks' => array( 'core/latest-posts' ),
Expand Down