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

Jump to bottom

Fix using the classic block in nested contexts #16477

Merged
merged 3 commits into from Jul 9, 2019
Merged

Fix using the classic block in nested contexts #16477

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0496727
Fix using the classic block in nested contexts
youknowriad Jul 9, 2019
82d4c21
update docs
youknowriad Jul 9, 2019
354b027
Changes per review
youknowriad Jul 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 11 additions & 7 deletions packages/blocks/src/api/serializer.js
View file
Open in desktop
Expand Up @@ -255,17 +255,18 @@ export function getCommentDelimitedContent( rawBlockName, attributes, content )
* Returns the content of a block, including comment delimiters, determining
* serialized attributes and content form from the current state of the block.
*
* @param {Object} block Block instance.
* @param {Object} block Block instance.
* @param {Object} options Serialization options.
*
* @return {string} Serialized block.
*/
export function serializeBlock( block ) {
export function serializeBlock( block, { isNested = false } = {} ) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I think we should try to consolidate our vocabulary around "inner blocks". Do we have many other references of "nested" in code? I know there's a tendency to refer to them this way in casual conversation. Is isInnerBlock reasonable as an alternative here?

const blockName = block.name;
const saveContent = getBlockContent( block );

switch ( blockName ) {
case getFreeformContentHandlerName():
case getUnregisteredTypeHandlerName():
switch ( true ) {
Copy link
Member

Choose a reason for hiding this comment

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

At this point, I think we'd just not want to bother with a switch and use if instead. Alternatively, we could intentionally use fallthrough to allow the nested case to fall through to default.

switch ( blockName ) {
	case getUnregisteredTypeHandlerName():
		return saveContent;

	case getFreeformContentHandlerName():
		if ( ! isNested ) {
			return saveContent;
		}

	default: {
		const blockType = getBlockType( blockName );
		const saveAttributes = getCommentAttributes( blockType, block.attributes );
		return getCommentDelimitedContent( blockName, saveAttributes, saveContent );
	}
}

It's pretty confusing though. Maybe less-so if default was just code following the switch instead of a case, but still confusing I think.

case ! isNested && blockName === getFreeformContentHandlerName():
case blockName === getUnregisteredTypeHandlerName():
return saveContent;

default: {
Expand All @@ -280,9 +281,12 @@ export function serializeBlock( block ) {
* Takes a block or set of blocks and returns the serialized post content.
*
* @param {Array} blocks Block(s) to serialize.
* @param {Object} options Serialization options.
Copy link
Member

Choose a reason for hiding this comment

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

We should document these options. I think the "best" way would to define a JSDoc typedef of something like WPBlockSerializationOptions with each property described. Unfortunately docgen won't yet output these, but it seems prime for a future enhancement.

*
* @return {string} The post content.
*/
export default function serialize( blocks ) {
return castArray( blocks ).map( serializeBlock ).join( '\n\n' );
export default function serialize( blocks, options ) {
return castArray( blocks )
.map( ( block ) => serializeBlock( block, options ) )
.join( '\n\n' );
}
22 changes: 22 additions & 0 deletions packages/blocks/src/api/test/serializer.js
View file
Open in desktop
Expand Up @@ -243,6 +243,28 @@ describe( 'block serializer', () => {

expect( content ).toBe( 'Bananas' );
} );
it( 'serializes the freeform content fallback block with comment delimiters in nested context', () => {
registerBlockType( 'core/freeform-block', {
category: 'common',
title: 'freeform block',
attributes: {
fruit: {
type: 'string',
},
},
save: ( { attributes } ) => attributes.fruit,
} );
setFreeformContentHandlerName( 'core/freeform-block' );
const block = createBlock( 'core/freeform-block', { fruit: 'Bananas' } );

const content = serializeBlock( block, { isNested: true } );

expect( content ).toBe(
'<!-- wp:freeform-block {\"fruit\":\"Bananas\"} -->\n' +
'Bananas\n' +
'<!-- /wp:freeform-block -->'
);
} );
it( 'serializes the unregistered fallback block without comment delimiters', () => {
registerBlockType( 'core/unregistered-block', {
category: 'common',
Expand Down
2 changes: 1 addition & 1 deletion packages/blocks/src/block-content-provider/index.js
View file
Open in desktop
Expand Up @@ -31,7 +31,7 @@ const { Consumer, Provider } = createContext( () => {} );
const BlockContentProvider = ( { children, innerBlocks } ) => {
const BlockContent = () => {
// Value is an array of blocks, so defer to block serializer
const html = serialize( innerBlocks );
const html = serialize( innerBlocks, { isNested: true } );

// Use special-cased raw HTML tag to avoid default escaping
return <RawHTML>{ html }</RawHTML>;
Expand Down