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] Add amp-fit-text support to text blocks #1151

Merged
merged 14 commits into from
May 25, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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: 0 additions & 1 deletion .jshintrc

This file was deleted.

27 changes: 27 additions & 0 deletions .jshintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"boss": true,
"curly": true,
"eqeqeq": true,
"eqnull": true,
"es3": true,
"esversion": 6,
"expr": true,
"immed": true,
"noarg": true,
"nonbsp": true,
"onevar": true,
"quotmark": "single",
"trailing": true,
"undef": true,
"unused": true,

"browser": true,

"globals": {
"_": false,
"Backbone": false,
"jQuery": false,
"JSON": false,
"wp": false
}
}
1 change: 1 addition & 0 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ function amp_init() {
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_editor_core_blocks' );
add_action( 'wp_loaded', 'amp_add_options_menu' );
add_action( 'parse_query', 'amp_correct_query_when_is_front_page' );

Expand Down
10 changes: 10 additions & 0 deletions assets/css/amp-default.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,13 @@
/** Force the image into a box of fixed dimensions and use object-fit to scale. **/
object-fit: contain;
}

.entry__content amp-fit-text blockquote,
amp-fit-text h1,
amp-fit-text h2,
amp-fit-text h3,
amp-fit-text h4,
amp-fit-text h5,
amp-fit-text h6 {
font-size: inherit;
}
122 changes: 120 additions & 2 deletions assets/js/amp-editor-blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,15 @@ var ampEditorBlocks = ( function() {
'core/image',
'core/video',
'core/audio'
]
],
textBlocks: [
'core/paragraph',
'core/heading',
'core/code',
'core/quote',
'core/subhead'
],
ampSettingsLabel: __( 'AMP Settings' )
}
};

Expand Down Expand Up @@ -178,6 +186,27 @@ var ampEditorBlocks = ( function() {
};
}

// Fit-text for text blocks.
if ( -1 !== component.data.textBlocks.indexOf( name ) ) {
if ( ! settings.attributes ) {
settings.attributes = {};
}
settings.attributes.ampFitText = {
type: 'boolean',
default: false
};
settings.attributes.minFont = {
type: 'number'
};
settings.attributes.maxFont = {
type: 'number'
};
settings.attributes.height = {
type: 'number',
default: 50
};
}

// Layout settings for embeds and media blocks.
if ( 0 === name.indexOf( 'core-embed' ) || -1 !== component.data.mediaBlocks.indexOf( name ) ) {
if ( ! settings.attributes ) {
Expand Down Expand Up @@ -222,6 +251,8 @@ var ampEditorBlocks = ( function() {
}
} else if ( -1 !== component.data.mediaBlocks.indexOf( name ) || 0 === name.indexOf( 'core-embed/' ) ) {
inspectorControls = component.setUpInspectorControls( props );
} else if ( -1 !== component.data.textBlocks.indexOf( name ) ) {
inspectorControls = component.setUpTextBlocksInspectorControls( props );
}

// Return just inspector controls in case of 'nodisplay'.
Expand Down Expand Up @@ -314,6 +345,77 @@ var ampEditorBlocks = ( function() {
);
};

/**
* Setup inspector controls for text blocks.
*
* @param {Object} props Props.
* @return {Object|Element|*|{$$typeof, type, key, ref, props, _owner}} Inspector Controls.
*/
component.setUpTextBlocksInspectorControls = function setUpInspectorControls( props ) {
var ampFitText = props.attributes.ampFitText,
minFont = props.attributes.minFont,
maxFont = props.attributes.maxFont,
height = props.attributes.height,
isSelected = props.isSelected,
el = wp.element.createElement,
InspectorControls = wp.blocks.InspectorControls,
TextControl = wp.components.TextControl,
ToggleControl = wp.components.ToggleControl,
PanelBody = wp.components.PanelBody,
label = 'Use AMP Fit Text';
Copy link
Member

Choose a reason for hiding this comment

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

Needs translation


if ( ampFitText ) {
return isSelected && (
el( InspectorControls, { key: 'inspector' },
el( PanelBody, { title: component.data.ampSettingsLabel },
el( ToggleControl, {
label: label,
checked: ampFitText,
onChange: function() {
props.setAttributes( { ampFitText: ! ampFitText } );
Copy link
Member

Choose a reason for hiding this comment

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

Should ampFitText be replaced with event.checked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the core blocks seem to use negating the current value for toggle controls (as far as I've seen), thought of staying consistent with the core blocks when using that. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Can this also result in the font-size-picker and range-control under Text Settings to be hidden when ampFitText is true?

}
} ),
el( TextControl, {
Copy link
Member

Choose a reason for hiding this comment

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

Can the font-size-picker control be used instead?

label: __( 'Height (px)' ),
value: height,
onChange: function( nextHeight ) {
props.setAttributes( { height: nextHeight } );
}
} ),
el( TextControl, {
label: __( 'Min font (px)' ),
value: minFont,
onChange: function( nextMinFont ) {
props.setAttributes( { minFont: nextMinFont } );
Copy link
Member

Choose a reason for hiding this comment

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

Abort if nextMinFont is larger than maxFont

}
} ),
el( TextControl, {
label: __( 'Max font (px)' ),
value: maxFont,
onChange: function( nextMaxFont ) {
props.setAttributes( { maxFont: nextMaxFont } );
Copy link
Member

Choose a reason for hiding this comment

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

Make sure the height is set to be the Math.max() of the nextMaxFont and its current value.

}
} )
)
)
);
}

return isSelected && (
el( InspectorControls, { key: 'inspector' },
el( PanelBody, { title: component.data.ampSettingsLabel },
el( ToggleControl, {
label: label,
checked: ampFitText,
onChange: function() {
props.setAttributes( { ampFitText: ! ampFitText } );
}
} )
)
)
);
};

/**
* Set up inspector controls for shortcode block.
* Adds ampCarousel attribute in case of gallery shortcode.
Expand Down Expand Up @@ -359,7 +461,12 @@ var ampEditorBlocks = ( function() {
* @return {*} Output element.
*/
component.filterBlocksSave = function filterBlocksSave( element, blockType, attributes ) {
var text;
var text,
fitTextProps = {
layout: 'fixed-height',
children: element
};

if ( 'core/shortcode' === blockType.name && component.isGalleryShortcode( attributes ) ) {
if ( attributes.ampCarousel ) {
// If the text contains amp-carousel, lets remove it.
Expand Down Expand Up @@ -390,6 +497,17 @@ var ampEditorBlocks = ( function() {
{},
text
);
} else if ( -1 !== component.data.textBlocks.indexOf( blockType.name ) && attributes.ampFitText ) {
if ( attributes.minFont ) {
fitTextProps[ 'min-font-size' ] = attributes.minFont;
}
if ( attributes.maxFont ) {
fitTextProps[ 'max-font-size' ] = attributes.maxFont;
}
if ( attributes.height ) {
fitTextProps.height = attributes.height;
}
return wp.element.createElement( 'amp-fit-text', fitTextProps );
}
return element;
};
Expand Down