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

RichText: add preserveWhiteSpace prop #18656

Merged
merged 3 commits into from
Nov 25, 2019
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
4 changes: 4 additions & 0 deletions packages/block-editor/src/components/rich-text/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ Render a rich [`contenteditable` input](https://developer.mozilla.org/en-US/docs

*Optional.* A list of autocompleters to use instead of the default.

### `preserveWhiteSpace: Boolean`

*Optional.* Whether or not to preserve white space characters in the `value`. Normally tab, newline and space characters are collapsed to a single space. If turned on, soft line breaks will be saved as newline characters, not as line break elements.

## RichText.Content

`RichText.Content` should be used in the `save` function of your block to correctly save rich text content.
Expand Down
2 changes: 2 additions & 0 deletions packages/block-editor/src/components/rich-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ class RichTextWrapper extends Component {
start,
reversed,
style,
preserveWhiteSpace,
// From experimental filter. To do: pick props instead.
...experimentalProps
} = this.props;
Expand Down Expand Up @@ -398,6 +399,7 @@ class RichTextWrapper extends Component {
__unstableDidAutomaticChange={ didAutomaticChange }
__unstableUndo={ undo }
style={ style }
preserveWhiteSpace={ preserveWhiteSpace }
>
{ ( { isSelected, value, onChange, Editable } ) =>
<>
Expand Down
7 changes: 5 additions & 2 deletions packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,14 @@ export function resetBlocks( blocks ) {
}

/**
* @typedef {WPBlockSelection} A block selection object.
* A block selection object.
*
* @typedef {Object} WPBlockSelection
*
* @property {string} clientId A block client ID.
* @property {string} attributeKey A block attribute key.
* @property {number} offset A block attribute offset.
* @property {number} offset An attribute value offset, based on the rich
* text value. See `wp.richText.create`.
*/

/**
Expand Down
11 changes: 10 additions & 1 deletion packages/block-editor/src/store/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,19 @@ export default {
const {
multiline: multilineTag,
__unstableMultilineWrapperTags: multilineWrapperTags,
__unstablePreserveWhiteSpace: preserveWhiteSpace,
} = attributeDefinition;
const value = insert( create( {
html,
multilineTag,
multilineWrapperTags,
preserveWhiteSpace,
} ), START_OF_SELECTED_AREA, offset, offset );

selectedBlock.attributes[ attributeKey ] = toHTMLString( {
value,
multilineTag,
preserveWhiteSpace,
} );
}

Expand Down Expand Up @@ -161,15 +164,21 @@ export default {
const {
multiline: multilineTag,
__unstableMultilineWrapperTags: multilineWrapperTags,
__unstablePreserveWhiteSpace: preserveWhiteSpace,
} = blockAType.attributes[ newAttributeKey ];
const convertedValue = create( {
html: convertedHtml,
multilineTag,
multilineWrapperTags,
preserveWhiteSpace,
} );
const newOffset = convertedValue.text.indexOf( START_OF_SELECTED_AREA );
const newValue = remove( convertedValue, newOffset, newOffset + 1 );
const newHtml = toHTMLString( { value: newValue, multilineTag } );
const newHtml = toHTMLString( {
value: newValue,
multilineTag,
preserveWhiteSpace,
} );

updatedAttributes[ newAttributeKey ] = newHtml;

Expand Down
3 changes: 2 additions & 1 deletion packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ import { SVG, Rect, G, Path } from '@wordpress/components';
*
* @property {string} clientId A block client ID.
* @property {string} attributeKey A block attribute key.
* @property {number} offset A block attribute offset.
* @property {number} offset An attribute value offset, based on the rich
* text value. See `wp.richText.create`.
*/

// Module constants
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/src/preformatted/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"type": "string",
"source": "html",
"selector": "pre",
"default": ""
"default": "",
"__unstablePreserveWhiteSpace": true
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this in the block.json. Isn't this a RichText prop?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is needed for merging the values, which uses create and toHTMLString under the hood. It is similar to __unstableMultilineWrapperTags: https://github.com/WordPress/gutenberg/search?q=__unstableMultilineWrapperTags&unscoped_q=__unstableMultilineWrapperTags. I marked it unstable as maybe there's other ways we want to do this in future.

Copy link
Contributor

Choose a reason for hiding this comment

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

mmm didn't know about that one. not great to be honest. Maybe the root issue is with the "merge" itself. maybe it should move to RichText somehow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we should have a look at it. The problem is that selection is stored based on the rich text value and not based on the HTML.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean we should add information about these flags in the selection reducer?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure... Normally they are set with RichText props. We should document the selection reducers to say that the offsets are based on the rich text values, yes.

Copy link
Member Author

@ellatrix ellatrix Nov 25, 2019

Choose a reason for hiding this comment

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

I've added a bit more documentation for WPBlockSelection. I agree that the merge function could be improved, but this should be looked at separately while this PR fixes an issue. The attribute property has been marked unstable so it could be adjusted in the future.

}
}
}
9 changes: 3 additions & 6 deletions packages/block-library/src/preformatted/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,11 @@ export default function PreformattedEdit( { attributes, mergeBlocks, setAttribut
<RichText
tagName="pre"
identifier="content"
// Ensure line breaks are normalised to HTML.
value={ content.replace( /\n/g, '<br>' ) }
preserveWhiteSpace
value={ content }
onChange={ ( nextContent ) => {
setAttributes( {
// Ensure line breaks are normalised to characters. This
// saves space, is easier to read, and ensures display
// filters work correctly.
content: nextContent.replace( /<br ?\/?>/g, '\n' ),
content: nextContent,
} );
} }
placeholder={ __( 'Write preformatted text…' ) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,11 @@ exports[`Preformatted should preserve character newlines 2`] = `
2</pre>
<!-- /wp:preformatted -->"
`;

exports[`Preformatted should preserve white space when merging 1`] = `
"<!-- wp:preformatted -->
<pre class=\\"wp-block-preformatted\\">1
2
3</pre>
<!-- /wp:preformatted -->"
`;
14 changes: 14 additions & 0 deletions packages/e2e-tests/specs/editor/blocks/preformatted.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,18 @@ describe( 'Preformatted', () => {

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should preserve white space when merging', async () => {
await insertBlock( 'Preformatted' );
await page.keyboard.type( '1' );
await page.keyboard.press( 'Enter' );
await page.keyboard.type( '2' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Tab' );
await page.keyboard.type( '3' );
await page.keyboard.press( 'ArrowLeft' );
await page.keyboard.press( 'Backspace' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );
} );
7 changes: 5 additions & 2 deletions packages/editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1193,11 +1193,14 @@ export function getEditorBlocks( state ) {
}

/**
* @typedef {WPBlockSelection} A block selection object.
* A block selection object.
*
* @typedef {Object} WPBlockSelection
*
* @property {string} clientId A block client ID.
* @property {string} attributeKey A block attribute key.
* @property {number} offset A block attribute offset.
* @property {number} offset An attribute value offset, based on the rich
* text value. See `wp.richText.create`.
*/

/**
Expand Down
2 changes: 2 additions & 0 deletions packages/rich-text/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ _Parameters_
- _$1.range_ `[Range]`: Range to create value from.
- _$1.multilineTag_ `[string]`: Multiline tag if the structure is multiline.
- _$1.multilineWrapperTags_ `[Array]`: Tags where lines can be found if nesting is possible.
- _$1.preserveWhiteSpace_ `[?boolean]`: Whether or not to collapse white space characters.

_Returns_

Expand Down Expand Up @@ -328,6 +329,7 @@ _Parameters_
- _$1_ `Object`: Named argements.
- _$1.value_ `Object`: Rich text value.
- _$1.multilineTag_ `[string]`: Multiline tag.
- _$1.preserveWhiteSpace_ `[?boolean]`: Whether or not to use newline characters for line breaks.

_Returns_

Expand Down
22 changes: 18 additions & 4 deletions packages/rich-text/src/component/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,11 @@ class RichText extends Component {
}

createRecord() {
const { __unstableMultilineTag: multilineTag, forwardedRef } = this.props;
const {
__unstableMultilineTag: multilineTag,
forwardedRef,
preserveWhiteSpace,
} = this.props;
const selection = getSelection();
const range = selection.rangeCount > 0 ? selection.getRangeAt( 0 ) : null;

Expand All @@ -212,6 +216,7 @@ class RichText extends Component {
multilineTag,
multilineWrapperTags: multilineTag === 'li' ? [ 'ul', 'ol' ] : undefined,
__unstableIsEditableTree: true,
preserveWhiteSpace,
} );
}

Expand Down Expand Up @@ -947,7 +952,11 @@ class RichText extends Component {
* @return {Object} An internal rich-text value.
*/
formatToValue( value ) {
const { format, __unstableMultilineTag: multilineTag } = this.props;
const {
format,
__unstableMultilineTag: multilineTag,
preserveWhiteSpace,
} = this.props;

if ( format !== 'string' ) {
return value;
Expand All @@ -959,6 +968,7 @@ class RichText extends Component {
html: value,
multilineTag,
multilineWrapperTags: multilineTag === 'li' ? [ 'ul', 'ol' ] : undefined,
preserveWhiteSpace,
} );
value.formats = prepare( value );

Expand Down Expand Up @@ -992,15 +1002,19 @@ class RichText extends Component {
* @return {*} The external data format, data type depends on props.
*/
valueToFormat( value ) {
const { format, __unstableMultilineTag: multilineTag } = this.props;
const {
format,
__unstableMultilineTag: multilineTag,
preserveWhiteSpace,
} = this.props;

value = this.removeEditorOnlyFormats( value );

if ( format !== 'string' ) {
return;
}

return toHTMLString( { value, multilineTag } );
return toHTMLString( { value, multilineTag, preserveWhiteSpace } );
}

Editable( props ) {
Expand Down
Loading