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

Blocks: Shortcode: Support multi-line shortcodes #4942

Closed
wants to merge 4 commits into from
Closed

Blocks: Shortcode: Support multi-line shortcodes #4942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df46f26
Blocks: Shortcode: Support multi-line shortcodes
mcsf Feb 7, 2018
501c5b5
Revert "Blocks: Shortcode: Support multi-line shortcodes"
mcsf Apr 27, 2018
dab2bcd
Support multi-line shortcodes via hasBlockSupport( _, 'rawContent' )
mcsf Apr 27, 2018
15a3736
Support multi-line shortcodes via addRawContentBlockName
mcsf Apr 27, 2018
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
2 changes: 2 additions & 0 deletions blocks/api/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export {
getUnknownTypeHandlerName,
setDefaultBlockName,
getDefaultBlockName,
addRawContentBlockName,
isRawContentBlockName,
getDefaultBlockForPostFormat,
getBlockType,
getBlockTypes,
Expand Down
20 changes: 20 additions & 0 deletions blocks/api/registration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ let unknownTypeHandlerName;
*/
let defaultBlockName;

/**
* A set of block names representing block types whose content the serializer
* should never attempt to beautify.
*
* @type {?string}
*/
const rawContentBlockNames = new Set();

/**
* Constant mapping post formats to the expected default block.
*
Expand Down Expand Up @@ -221,6 +229,18 @@ export function getDefaultBlockName() {
return defaultBlockName;
}

export function addRawContentBlockName( name ) {
rawContentBlockNames.add( name );
}

export function isRawContentBlockName( name ) {
return rawContentBlockNames.has( name );
}

export function clearRawContentBlockNames() {
rawContentBlockNames.clear();
}

/**
* Retrieves the expected default block for the post format.
*
Expand Down
8 changes: 6 additions & 2 deletions blocks/api/serializer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { hasFilter, applyFilters } from '@wordpress/hooks';
/**
* Internal dependencies
*/
import { getBlockType, getUnknownTypeHandlerName } from './registration';
import { getBlockType, getUnknownTypeHandlerName, isRawContentBlockName } from './registration';
import BlockContentProvider from '../block-content-provider';

/**
Expand Down Expand Up @@ -196,7 +196,11 @@ export function getBlockContent( block ) {
} catch ( error ) {}
}

return getUnknownTypeHandlerName() === block.name || ! saveContent ? saveContent : getBeautifulContent( saveContent );
return (
getUnknownTypeHandlerName() === block.name ||
isRawContentBlockName( block.name ) ||
! saveContent
) ? saveContent : getBeautifulContent( saveContent );
}

/**
Expand Down
15 changes: 15 additions & 0 deletions blocks/api/test/registration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import {
getUnknownTypeHandlerName,
setDefaultBlockName,
getDefaultBlockName,
addRawContentBlockName,
isRawContentBlockName,
clearRawContentBlockNames,
getBlockType,
getBlockTypes,
getBlockSupport,
Expand All @@ -39,6 +42,7 @@ describe( 'blocks', () => {
} );
setUnknownTypeHandlerName( undefined );
setDefaultBlockName( undefined );
clearRawContentBlockNames();
window._wpBlocks = {};
} );

Expand Down Expand Up @@ -306,6 +310,17 @@ describe( 'blocks', () => {
} );
} );

describe( 'isRawContentBlockName', () => {
it( 'adds block names to a set', () => {
addRawContentBlockName( 'core/test-block-1' );
addRawContentBlockName( 'core/test-block-2' );
addRawContentBlockName( 'core/test-block-1' );
expect( isRawContentBlockName( 'core/test-block-1' ) ).toBe( true );
expect( isRawContentBlockName( 'core/test-block-2' ) ).toBe( true );
expect( isRawContentBlockName( 'core/test-block-3' ) ).toBe( false );
} );
} );

describe( 'getBlockType()', () => {
it( 'should return { name, save } for blocks with minimum settings', () => {
registerBlockType( 'core/test-block', defaultBlockSettings );
Expand Down
2 changes: 2 additions & 0 deletions core-blocks/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
registerBlockType,
setDefaultBlockName,
setUnknownTypeHandlerName,
addRawContentBlockName,
} from '@wordpress/blocks';
import { deprecated } from '@wordpress/utils';

Expand Down Expand Up @@ -83,6 +84,7 @@ export const registerCoreBlocks = () => {

setDefaultBlockName( paragraph.name );
setUnknownTypeHandlerName( freeform.name );
addRawContentBlockName( shortcode.name );
};

// Backwards compatibility
Expand Down
5 changes: 2 additions & 3 deletions core-blocks/shortcode/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* WordPress dependencies
*/
import { RawHTML } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { withInstanceId, Dashicon } from '@wordpress/components';
import { PlainText } from '@wordpress/blocks';
Expand Down Expand Up @@ -45,7 +44,7 @@ export const settings = {
text: {
type: 'string',
shortcode: ( attrs, { content } ) => {
return content;
return content.replace( /<br \/>/g, '' );
Copy link
Member

Choose a reason for hiding this comment

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

How do we know if these are added by the browser? Guess we would need to check if it's plain text, and only remove then?

Btw, the current editor also destroys the following.

[bwcsv]One,Two,Three
A,B,C
D,E,F
[/bwcsv]

becomes

<p>[bwcsv]One,Two,Three A,B,C D,E,F [/bwcsv]</p>

Copy link
Member

Choose a reason for hiding this comment

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

And if pasted in TinyMCE it becomes:

<p>[bw_csv]One,Two,Three<br />A,B,C<br />D,E,F<br />[/bw_csv]</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.

Btw, the current editor also destroys the following.

Not if manually entered, right?

Regardless, a block whose purpose is to hold shortcodes should be capable of preserving arbitrary formatting, IMO.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it should preserve it, agreed. But this line here is about input. Now I just realised this is actually the Markdown converter inserting these line break elements? With the simpleLineBreaks option. 😰

},
},
},
Expand Down Expand Up @@ -83,6 +82,6 @@ export const settings = {
),

save( { attributes } ) {
return <RawHTML>{ attributes.text }</RawHTML>;
return attributes.text;
},
};