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

Quote Block: Adding alignment controls to the quote block #511

Closed
wants to merge 5 commits into from
Closed

Quote Block: Adding alignment controls to the quote block #511

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
76f8742
Quote Block: Adding alignment controls to the quote block
youknowriad Apr 26, 2017
39abb03
Controls: Take into account mixed controls use-case
youknowriad Apr 26, 2017
95723f3
Document toolbar control normalization
aduth Apr 26, 2017
e5ba308
Simplify the toolbar computing
youknowriad Apr 27, 2017
be27f94
Restore truthiness check to avoid errors on blocks without controls
youknowriad Apr 27, 2017
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
63 changes: 49 additions & 14 deletions blocks/library/quote/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import './style.scss';
import { registerBlock, query as hpq } from 'api';
import Editable from 'components/editable';

const { children, query, attr } = hpq;
const { children, query, attr, prop } = hpq;

registerBlock( 'core/quote', {
title: wp.i18n.__( 'Quote' ),
Expand All @@ -15,6 +15,7 @@ registerBlock( 'core/quote', {
attributes: {
value: query( 'blockquote > p', children() ),
citation: children( 'footer' ),
align: prop( 'blockquote', 'style.textAlign' ),
style: ( node ) => {
const value = attr( 'blockquote', 'class' )( node );
if ( ! value ) {
Expand All @@ -30,21 +31,52 @@ registerBlock( 'core/quote', {
}
},

controls: [ 1, 2 ].map( ( variation ) => ( {
icon: 'format-quote',
title: wp.i18n.sprintf( wp.i18n.__( 'Quote style %d' ), variation ),
isActive: ( { style = 1 } ) => style === variation,
onClick( attributes, setAttributes ) {
setAttributes( { style: variation } );
},
subscript: variation
} ) ),
controls: [
[
{
icon: 'editor-alignleft',
title: wp.i18n.__( 'Align left' ),
isActive: ( { align } ) => ! align || 'left' === align,
onClick( attributes, setAttributes ) {
setAttributes( { align: undefined } );
}
},
{
icon: 'editor-aligncenter',
title: wp.i18n.__( 'Align center' ),
isActive: ( { align } ) => 'center' === align,
onClick( attributes, setAttributes ) {
setAttributes( { align: 'center' } );
}
},
{
icon: 'editor-alignright',
title: wp.i18n.__( 'Align right' ),
isActive: ( { align } ) => 'right' === align,
onClick( attributes, setAttributes ) {
setAttributes( { align: 'right' } );
}
}
],
[ 1, 2 ].map( ( variation ) => ( {
icon: 'format-quote',
title: wp.i18n.sprintf( wp.i18n.__( 'Quote style %d' ), variation ),
isActive: ( { style = 1 } ) => style === variation,
onClick( attributes, setAttributes ) {
setAttributes( { style: variation } );
},
subscript: variation
} ) )
],

edit( { attributes, setAttributes, focus, setFocus } ) {
const { value, citation, style = 1 } = attributes;
const { align, value, citation, style = 1 } = attributes;

return (
<blockquote className={ `blocks-quote blocks-quote-style-${ style }` }>
<blockquote
className={ `blocks-quote blocks-quote-style-${ style }` }
style={ align ? { textAlign: align } : null }
Copy link
Member

Choose a reason for hiding this comment

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

Do we want the text alignment to apply to the entire quote (including citation), or just the paragraphs? cc @jasmussen

Copy link

@joyously joyously Apr 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The text alignment only applies to the selected text, not a block. Float alignment applies to the whole block.

Copy link
Contributor Author

@youknowriad youknowriad Apr 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Good question @joyously and @jasmussen might help to answer here. For me considering the alignment as inline controls is relevant for the continuous Text block #447.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I missed this. Yes it would be preferrable if we were able to have the alignments be per paragraph. Let me know if you'd like a mockup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I ask for a clarification? Is this for the current quote block or for the continuous Text block? Yep, some mockups could be good :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the midstream correction on this one. Here's a mockup of a multi paragraph quote:

quote left aligned

Note how the caret is in text that is left aligned, therefore the toolbar shows the left alignment highlighted.

Now the first paragraph is right aligned:

quote right aligned

Note how the caret is inside the first paragraph.

Technically the behavior here would be the same as that of the "Continuous text" block.

>
<Editable
value={ value }
onChange={
Expand Down Expand Up @@ -74,10 +106,13 @@ registerBlock( 'core/quote', {
},

save( attributes ) {
const { value, citation, style = 1 } = attributes;
const { align, value, citation, style = 1 } = attributes;

return (
<blockquote className={ `blocks-quote-style-${ style }` }>
<blockquote
className={ `blocks-quote-style-${ style }` }
style={ align ? { textAlign: align } : null }
>
{ value && value.map( ( paragraph, i ) => (
<p key={ i }>{ paragraph }</p>
) ) }
Expand Down
12 changes: 9 additions & 3 deletions editor/modes/visual-editor/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ class VisualEditorBlock extends wp.element.Component {
wrapperProps = settings.getEditWrapperProps( block.attributes );
}

// Normalize controls as an array of arrays (toolbars of controls)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe something like <div className="editor-toolbar__group"> should move to a separate component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, but this component will have the following props : content, attributes, setAttributes. Or we could pass a block uid instead and duplicate setAttributes. Having attributes and setAttributes as props for a GroupedToolbars might not be a good idea.

const toolbars = settings.controls && settings.controls.length && ! Array.isArray( settings.controls[ 0 ] )
? [ settings.controls ]
: settings.controls;

// Disable reason: Each block can receive focus but must be able to contain
// block children. Tab keyboard navigation enabled by tabIndex assignment.

Expand All @@ -159,14 +164,15 @@ class VisualEditorBlock extends wp.element.Component {
{ isSelected && ! isTyping &&
<div className="editor-visual-editor__block-controls">
<BlockSwitcher uid={ block.uid } />
{ !! settings.controls && (
{ toolbars.map( ( controls, index ) => (
Copy link
Member

Choose a reason for hiding this comment

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

This will error if a block doesn't define controls because toolbars will be undefined. We'll want to restore the truthy test or use Lodash's map.

<Toolbar
controls={ settings.controls.map( ( control ) => ( {
key={ index }
controls={ controls.map( ( control ) => ( {
...control,
onClick: () => control.onClick( block.attributes, this.setAttributes ),
isActive: control.isActive( block.attributes )
} ) ) } />
) }
) ) }
<Slot name="Formatting.Toolbar" />
</div>
}
Expand Down
5 changes: 4 additions & 1 deletion languages/gutenberg.pot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,21 @@ msgstr ""

#: blocks/library/image/index.js:41
#: blocks/library/list/index.js:25
#: blocks/library/quote/index.js:38
#: blocks/library/text/index.js:23
msgid "Align left"
msgstr ""

#: blocks/library/image/index.js:47
#: blocks/library/list/index.js:33
#: blocks/library/quote/index.js:46
#: blocks/library/text/index.js:31
msgid "Align center"
msgstr ""

#: blocks/library/image/index.js:53
#: blocks/library/list/index.js:41
#: blocks/library/quote/index.js:54
#: blocks/library/text/index.js:39
msgid "Align right"
msgstr ""
Expand All @@ -74,7 +77,7 @@ msgstr ""
msgid "Quote"
msgstr ""

#: blocks/library/quote/index.js:35
#: blocks/library/quote/index.js:63
msgid "Quote style %d"
msgstr ""

Expand Down