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

Parser: Remove specific support for <!--more--> tag #5061

Merged
merged 2 commits into from
Feb 24, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
26 changes: 1 addition & 25 deletions blocks/api/post.pegjs
Original file line number Diff line number Diff line change
Expand Up @@ -169,33 +169,9 @@ Block_List
}

Token
= Tag_More
/ Block_Void
= Block_Void
/ Block_Balanced

Tag_More
= "<!--" WS* "more" customText:(WS+ text:$((!(WS* "-->") .)+) { /** <?php return $text; ?> **/ return text })? WS* "-->" noTeaser:(WS* "<!--noteaser-->")?
{ /** <?php
$attrs = array( 'noTeaser' => (bool) $noTeaser );
if ( ! empty( $customText ) ) {
$attrs['customText'] = $customText;
}
return array(
'blockName' => 'core/more',
'attrs' => $attrs,
'innerHTML' => ''
);
?> **/
return {
blockName: 'core/more',
attrs: {
customText: customText || undefined,
noTeaser: !! noTeaser
},
innerHTML: ''
}
}

Block_Void
= "<!--" WS+ "wp:" blockName:Block_Name WS+ attrs:(a:Block_Attributes WS+ {
/** <?php return $a; ?> **/
Expand Down
3 changes: 3 additions & 0 deletions blocks/api/raw-handling/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { getBlockTypes, getUnknownTypeHandlerName } from '../registration';
import { getBlockAttributes, parseWithGrammar } from '../parser';
import normaliseBlocks from './normalise-blocks';
import stripAttributes from './strip-attributes';
import specialCommentConverter from './special-comment-converter';
import commentRemover from './comment-remover';
import createUnwrapper from './create-unwrapper';
import isInlineContent from './is-inline-content';
Expand Down Expand Up @@ -96,6 +97,7 @@ export default function rawHandler( { HTML, plainText = '', mode = 'AUTO', tagNa
// Add semantic formatting before attributes are stripped.
formattingTransformer,
stripAttributes,
specialCommentConverter,
commentRemover,
createUnwrapper( ( node ) => ! isInline( node, tagName ) ),
] );
Expand Down Expand Up @@ -124,6 +126,7 @@ export default function rawHandler( { HTML, plainText = '', mode = 'AUTO', tagNa
// Add semantic formatting before attributes are stripped.
formattingTransformer,
stripAttributes,
specialCommentConverter,
commentRemover,
! canUserUseUnfilteredHTML && createUnwrapper( ( element ) => element.nodeName === 'IFRAME' ),
embeddedContentReducer,
Expand Down
73 changes: 73 additions & 0 deletions blocks/api/raw-handling/special-comment-converter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/**
* Browser dependencies
*/
const { COMMENT_NODE } = window.Node;

/**
* Looks for `<!--more-->` comments, as well as the `<!--more Some text-->`
* variant and its `<!--noteaser-->` companion, and replaces them with a custom
* element representing a future block.
*
* The custom element is a way to bypass the rest of the `raw-handling`
* transforms, which would eliminate other kinds of node with which to carry
* `<!--more-->`'s data: nodes with `data` attributes, empty paragraphs, etc.
*
* The custom element is then expected to be recognized by any registered
* block's `raw` transform.
*
* @param {Node} node The node to be processed.
* @return {void}
*/
export default function( node ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we should unit test this or is it already covered elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think some tests are in order 👍

if (
node.nodeType !== COMMENT_NODE ||
node.nodeValue.indexOf( 'more' ) !== 0
) {
// We don't specificially look for `noteaser`, meaning that if one is
// found on its own (and not adjacent to `more`), it will be lost.
return;
}

// Grab any custom text in the comment
const customText = node.nodeValue.slice( 4 ).trim();
Copy link
Member

Choose a reason for hiding this comment

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

Amusing side note: this fixes a bug in the more handling currently in the parser, which requires a space between "more" and the custom text. Core doesn't require that space. 🙂

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I missed the previous discussion wherein @dmsnell learned yet another wonderful weird thing about Core. 🙃


// When a `<!--more-->` comment is found, we need to look for any
// `<!--noteaser-->` sibling, but it may not be a direct sibling
// (whitespace typically lies in between)
let sibling = node;
let noTeaser = false;
while ( ( sibling = sibling.nextSibling ) ) {
if (
sibling.nodeType === COMMENT_NODE &&
sibling.nodeValue === 'noteaser'
) {
noTeaser = true;
sibling.parentNode.removeChild( sibling );
break;
}
}

// Conjure up a custom More element
const more = createMore( customText, noTeaser );

// Append it to the top level for later conversion to blocks
let parent = node.parentNode;
while ( parent.nodeName !== 'BODY' ) {
parent = parent.parentNode;
}
parent.appendChild( more );
node.parentNode.removeChild( node );
}

function createMore( customText, noTeaser ) {
const node = document.createElement( 'wp-block' );
Copy link
Member

Choose a reason for hiding this comment

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

Interesting thing :) I wonder if we can also do this for shortcakes... Might enable us to get rid of the "pieces" of HTML and parse it all in one go.

node.dataset.block = 'core/more';
if ( customText ) {
node.dataset.customText = customText;
}
if ( noTeaser ) {
// "Boolean" data attribute
node.dataset.noTeaser = '';
}
return node;
}
49 changes: 49 additions & 0 deletions blocks/api/raw-handling/test/special-comment-converter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* External dependencies
*/
import { equal } from 'assert';

/**
* Internal dependencies
*/
import specialCommentConverter from '../special-comment-converter';
import { deepFilterHTML } from '../utils';

describe( 'specialCommentConverter', () => {
it( 'should convert a single comment into a basic block', () => {
equal(
deepFilterHTML( '<!--more-->', [ specialCommentConverter ] ),
'<wp-block data-block="core/more"></wp-block>'
);
} );
it( 'should convert two comments into a block', () => {
equal(
deepFilterHTML( '<!--more--><!--noteaser-->', [ specialCommentConverter ] ),
'<wp-block data-block="core/more" data-no-teaser=""></wp-block>'
);
} );
it( 'should pass custom text to the block', () => {
equal(
deepFilterHTML(
'<!--more Read all about it!--><!--noteaser-->',
[ specialCommentConverter ]
),
'<wp-block data-block="core/more" data-custom-text="Read all about it!" data-no-teaser=""></wp-block>'
);
} );
it( 'should handle reformatted content', () => {
const output = deepFilterHTML(
`<p>
<!--more-->
<!--noteaser-->
</p>`,
[ specialCommentConverter ]
);
// Skip the empty paragraph, which other transforms would eliminate
const start = output.indexOf( '</p>' ) + '</p>'.length;
equal(
output.substr( start ),
'<wp-block data-block="core/more" data-no-teaser=""></wp-block>'
);
} );
} );
15 changes: 1 addition & 14 deletions blocks/api/serializer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { isEmpty, reduce, isObject, castArray, compact, startsWith } from 'lodash';
import { isEmpty, reduce, isObject, castArray, startsWith } from 'lodash';
import { html as beautifyHtml } from 'js-beautify';
import isEqualShallow from 'is-equal-shallow';

Expand Down Expand Up @@ -239,19 +239,6 @@ export function serializeBlock( block ) {
const saveAttributes = getCommentAttributes( block.attributes, blockType );

switch ( blockName ) {
case 'core/more':
const { customText, noTeaser } = saveAttributes;

const moreTag = customText ?
`<!--more ${ customText }-->` :
'<!--more-->';

const noTeaserTag = noTeaser ?
'<!--noteaser-->' :
'';

return compact( [ moreTag, noTeaserTag ] ).join( '\n' );

case getUnknownTypeHandlerName():
return saveContent;

Expand Down
48 changes: 0 additions & 48 deletions blocks/api/test/serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,54 +283,6 @@ describe( 'block serializer', () => {
} );

describe( 'serializeBlock()', () => {
describe( '"more" block', () => {
beforeEach( () => {
registerBlockType( 'core/more', {
category: 'layout',
title: 'more',
attributes: {
customText: {
type: 'string',
},
noTeaser: {
type: 'boolean',
default: false,
},
},

save: () => null,
} );
} );

it( 'serializes without text', () => {
const block = createBlock( 'core/more', {} );

const content = serializeBlock( block );

expect( content ).toBe( '<!--more-->' );
} );

it( 'serializes with text', () => {
const block = createBlock( 'core/more', {
customText: 'Read more!',
} );

const content = serializeBlock( block );

expect( content ).toBe( '<!--more Read more!-->' );
} );

it( 'serializes with no teaser', () => {
const block = createBlock( 'core/more', {
noTeaser: true,
} );

const content = serializeBlock( block );

expect( content ).toBe( '<!--more-->\n<!--noteaser-->' );
} );
} );

it( 'serializes the fallback block without comment delimiters', () => {
registerBlockType( 'core/unknown-block', {
category: 'common',
Expand Down
47 changes: 45 additions & 2 deletions blocks/library/more/index.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
/**
* External dependencies
*/
import { compact } from 'lodash';

/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { ToggleControl } from '@wordpress/components';
import { RawHTML } from '@wordpress/element';

/**
* Internal dependencies
*/
import './editor.scss';
import { createBlock } from '../../api';
import InspectorControls from '../../inspector-controls';

export const name = 'core/more';
Expand Down Expand Up @@ -39,6 +46,28 @@ export const settings = {
},
},

transforms: {
from: [
{
type: 'raw',
isMatch: ( node ) => node.dataset && node.dataset.block === 'core/more',
transform( node ) {
const { customText, noTeaser } = node.dataset;
const attrs = {};
// Don't copy unless defined and not an empty string
if ( customText ) {
attrs.customText = customText;
}
// Special handling for boolean
if ( noTeaser === '' ) {
attrs.noTeaser = true;
}
return createBlock( 'core/more', attrs );
},
},
],
},

edit( { attributes, setAttributes, isSelected } ) {
const { customText, noTeaser } = attributes;

Expand Down Expand Up @@ -68,7 +97,21 @@ export const settings = {
];
},

save() {
return null;
save( { attributes } ) {
const { customText, noTeaser } = attributes;

const moreTag = customText ?
`<!--more ${ customText }-->` :
'<!--more-->';

const noTeaserTag = noTeaser ?
'<!--noteaser-->' :
'';

return (
<RawHTML>
{ compact( [ moreTag, noTeaserTag ] ).join( '\n' ) }
</RawHTML>
);
},
};
2 changes: 2 additions & 0 deletions blocks/test/fixtures/core__more.html
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
<!-- wp:core/more -->
<!--more-->
<!-- /wp:core/more -->
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core__more.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
"noTeaser": false
},
"innerBlocks": [],
"originalContent": ""
"originalContent": "<!--more-->"
}
]
7 changes: 3 additions & 4 deletions blocks/test/fixtures/core__more.parsed.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
[
{
"blockName": "core/more",
"attrs": {
"noTeaser": false
},
"innerHTML": ""
"attrs": null,
"innerBlocks": [],
"innerHTML": "\n<!--more-->\n"
},
{
"attrs": {},
Expand Down
2 changes: 2 additions & 0 deletions blocks/test/fixtures/core__more.serialized.html
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
<!-- wp:more -->
<!--more-->
<!-- /wp:more -->
2 changes: 2 additions & 0 deletions blocks/test/fixtures/core__more__custom-text-teaser.html
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
<!-- wp:core/more {"customText":"Continue Reading","noTeaser":true} -->
<!--more Continue Reading-->
<!--noteaser-->
<!-- /wp:core/more -->
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core__more__custom-text-teaser.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
"noTeaser": true
},
"innerBlocks": [],
"originalContent": ""
"originalContent": "<!--more Continue Reading-->\n<!--noteaser-->"
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"customText": "Continue Reading",
"noTeaser": true
},
"innerHTML": ""
"innerBlocks": [],
"innerHTML": "\n<!--more Continue Reading-->\n<!--noteaser-->\n"
},
{
"attrs": {},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
<!-- wp:more {"customText":"Continue Reading","noTeaser":true} -->
<!--more Continue Reading-->
<!--noteaser-->
<!-- /wp:more -->
Loading