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

Jump to bottom

Writing Flow/Quote: allow splitting #17121

Merged
merged 2 commits into from
Aug 21, 2019
Merged

Writing Flow/Quote: allow splitting #17121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8f01df9
Writing Flow/Quote: allow splitting
ellatrix Aug 21, 2019
edfeb43
Add extra merge e2e test
ellatrix Aug 21, 2019
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
11 changes: 11 additions & 0 deletions packages/block-library/src/quote/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import classnames from 'classnames';
import { __ } from '@wordpress/i18n';
import { AlignmentToolbar, BlockControls, RichText } from '@wordpress/block-editor';
import { BlockQuotation } from '@wordpress/components';
import { createBlock } from '@wordpress/blocks';

export default function QuoteEdit( { attributes, setAttributes, isSelected, mergeBlocks, onReplace, className } ) {
const { align, value, citation } = attributes;
Expand Down Expand Up @@ -48,6 +49,16 @@ export default function QuoteEdit( { attributes, setAttributes, isSelected, merg
// translators: placeholder text used for the quote
__( 'Write quote…' )
}
onReplace={ onReplace }
onSplit={ ( piece ) =>
createBlock( 'core/quote', {
...attributes,
value: piece,
} )
}
__unstableOnSplitMiddle={ () =>
createBlock( 'core/paragraph' )
}
/>
{ ( ! RichText.isEmpty( citation ) || isSelected ) && (
<RichText
Expand Down
10 changes: 8 additions & 2 deletions packages/block-library/src/quote/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,23 @@ export const settings = {
edit,
save,
merge( attributes, { value, citation } ) {
// Quote citations cannot be merged. Pick the second one unless it's
// empty.
if ( ! citation ) {
citation = attributes.citation;
}

if ( ! value || value === '<p></p>' ) {
return {
...attributes,
citation: attributes.citation + citation,
citation,
};
}

return {
...attributes,
value: attributes.value + value,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we append the first citation between the two values here?

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 feels a bit strange to me. :) If you split a quote into two quotes and merge them, you end up with a cite as quote content? In that case I would expect them to get merged in the original form. But maybe that's ok, then it requires a few more backspaces to restore. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I'm not sure either, I guess I "just" wanted to avoid losing that text but it probably doesn't make a lot of sense.

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 agree with trying to avoid data loss. If the merge wasn't good, the user could always revert though. Something else we could do is ask the user which citation to use in case there are two. Kind of like a merge conflict in git. :) This pattern might also be useful to other blocks.

citation: attributes.citation + citation,
citation,
};
},
deprecated,
Expand Down
62 changes: 62 additions & 0 deletions packages/e2e-tests/specs/blocks/__snapshots__/quote.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,68 @@ exports[`Quote can be merged into from a paragraph 1`] = `
<!-- /wp:quote -->"
`;

exports[`Quote can be split at the end and merged back 1`] = `
"<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><p>1</p></blockquote>
<!-- /wp:quote -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->"
`;

exports[`Quote can be split at the end and merged back 2`] = `
"<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><p>1</p><p></p></blockquote>
<!-- /wp:quote -->"
`;

exports[`Quote can be split at the end and merged back 3`] = `
"<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><p>1</p></blockquote>
<!-- /wp:quote -->"
`;

exports[`Quote can be split in the middle and merged back 1`] = `
"<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><p>1</p><cite>c</cite></blockquote>
<!-- /wp:quote -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><p>2</p><cite>c</cite></blockquote>
<!-- /wp:quote -->"
`;

exports[`Quote can be split in the middle and merged back 2`] = `
"<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><p>1</p><p></p><cite>c</cite></blockquote>
<!-- /wp:quote -->

<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><p>2</p><cite>c</cite></blockquote>
<!-- /wp:quote -->"
`;

exports[`Quote can be split in the middle and merged back 3`] = `
"<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><p>1</p><cite>c</cite></blockquote>
<!-- /wp:quote -->

<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><p>2</p><cite>c</cite></blockquote>
<!-- /wp:quote -->"
`;

exports[`Quote can be split in the middle and merged back 4`] = `
"<!-- wp:quote -->
<blockquote class=\\"wp-block-quote\\"><p>1</p><p>2</p><cite>c</cite></blockquote>
<!-- /wp:quote -->"
`;

exports[`Quote is transformed to a heading and a quote if the quote contains a citation 1`] = `
"<!-- wp:heading -->
<h2>one</h2>
Expand Down
53 changes: 53 additions & 0 deletions packages/e2e-tests/specs/blocks/quote.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,57 @@ describe( 'Quote', () => {
await page.keyboard.press( 'Backspace' );
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'can be split at the end and merged back', async () => {
await insertBlock( 'Quote' );
await page.keyboard.type( '1' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );

// Expect empty paragraph outside quote block.
expect( await getEditedPostContent() ).toMatchSnapshot();

await page.keyboard.press( 'Backspace' );

// Expect empty paragraph inside quote block.
expect( await getEditedPostContent() ).toMatchSnapshot();

await page.keyboard.press( 'Backspace' );

// Expect quote without empty paragraphs.
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'can be split in the middle and merged back', async () => {
await insertBlock( 'Quote' );
await page.keyboard.type( '1' );
await page.keyboard.press( 'Enter' );
await page.keyboard.type( '2' );
await page.keyboard.press( 'Tab' );
await page.keyboard.type( 'c' );
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );

// Expect two quote blocks and empty paragraph in the middle.
expect( await getEditedPostContent() ).toMatchSnapshot();

await page.keyboard.press( 'Backspace' );

// Expect two quote blocks and empty paragraph in the first quote.
expect( await getEditedPostContent() ).toMatchSnapshot();

await page.keyboard.press( 'Backspace' );

// Expect two quote blocks.
expect( await getEditedPostContent() ).toMatchSnapshot();

await page.keyboard.press( 'ArrowLeft' );
await page.keyboard.press( 'ArrowDown' );
await page.keyboard.press( 'ArrowDown' );
await page.keyboard.press( 'Backspace' );

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