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: Fix serializing the quote block #583

Merged
merged 1 commit into from
May 1, 2017
Merged

Quote Block: Fix serializing the quote block #583

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions blocks/library/quote/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ registerBlock( 'core/quote', {
);
},

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

return (
<blockquote className={ `blocks-quote-style-${ style }` }>
{ value && value.map( ( paragraph, i ) => (
{ value && wp.element.Children.map( value, ( paragraph, i ) => (
Copy link
Member

Choose a reason for hiding this comment

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

Based on the value attributes definition, we should be certain that value is an array, not one of React's opaque children structure:

query( 'blockquote > p', children() )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not when we insert an empty quote block, if we type inside the editable it'll not produce an array if we have a single paragraph.

Copy link
Member

Choose a reason for hiding this comment

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

It's not when we insert an empty quote block, if we type inside the editable it'll not produce an array if we have a single paragraph.

That's true, but in that case we have a bit of a mismatch between the shape of state on parse, and the value that will be assigned from Editable as described in your example.

Specifically, parse will treat a single paragraph as [ <p>foo</p> ], whereas in a new block, the same content would be assigned as just <p>foo</p>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this really concerning? we could update the onChange callback to match this if you prefer.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe, maybe not. Definitely want to avoid a situation where a developer would need to account for varying shapes of the value, if it ever comes to that (Array.isArray checks, React.Children.map when we know it's just one element, etc).

<p key={ i }>{ paragraph }</p>
) ) }
<footer>{ citation }</footer>
Expand Down