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

Take into account citation when transform a quote to paragraph #10685

Merged
merged 4 commits into from Oct 17, 2018

Conversation

Projects
None yet
5 participants
@nosolosw
Member

nosolosw commented Oct 17, 2018

When transforming a quote to a paragraph the citation was left out. This PR makes the citation a new paragraph.

@nosolosw nosolosw self-assigned this Oct 17, 2018

@nosolosw nosolosw requested a review from WordPress/gutenberg-core Oct 17, 2018

@nosolosw nosolosw added the [Type] Bug label Oct 17, 2018

@nosolosw nosolosw added this to the 4.1 - UI freeze milestone Oct 17, 2018

@@ -97,8 +97,8 @@ export const settings = {
{
type: 'block',
blocks: [ 'core/paragraph' ],
transform: ( { value } ) =>
split( create( { html: value, multilineTag: 'p' } ), '\u2028' )
transform: ( { value, citation } ) =>

This comment has been minimized.

@iseulde

iseulde Oct 17, 2018

Member

This can be added after split, no? No need to parse it.

@iseulde

This comment has been minimized.

Member

iseulde commented Oct 17, 2018

Could you added an e2e test as well?

nosolosw added some commits Oct 17, 2018

Refactor to take into account blank quote or cite
- quote + cite should render at least two paragraphs (more if quotes contains several paragraphs)
- void quote + cite should render only one paragraph with the cite
- quote + void cite should render only one paragraph with the quote
@nosolosw

This comment has been minimized.

Member

nosolosw commented Oct 17, 2018

@iseulde pushed some changes to make it more robust against void cite, quotes, or both. Also updated the existing e2e tests.

@iseulde

Ok, looks good.

@iseulde

This comment has been minimized.

Member

iseulde commented Oct 17, 2018

@nosolosw Oh, could we do the same for pull quote?

@nosolosw

This comment has been minimized.

Member

nosolosw commented Oct 17, 2018

Cool! Going to merge.

Pullquote can only be transformed to/from a quote since a few hours ago 🎉 Depending on how clean is the queue tomorrow morning I might be able to add the pullquote<->paragraph transform (and take the cite into account).

@nosolosw nosolosw merged commit a22cbbb into master Oct 17, 2018

2 checks passed

codecov/project 49.55% (+0.01%) compared to ae9b363
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nosolosw nosolosw deleted the fix/quote-transformations branch Oct 17, 2018

@paaljoachim

This comment has been minimized.

paaljoachim commented Oct 22, 2018

https://make.wordpress.org/test/
13. Add a quote block with a citation, convert it to a paragraph block, make sure the citation is still there. (10685)

Works. Should the citation come direct below the quote in the same paragraph box? Or should it as it is right now go into another paragraph box below the quote?

@nosolosw

This comment has been minimized.

Member

nosolosw commented Oct 22, 2018

Should the citation come direct below the quote in the same paragraph box? Or should it as it is right now go into another paragraph box below the quote?

The expected behavior is that a new paragraph is created for the cite.

@paaljoachim

This comment has been minimized.

paaljoachim commented Oct 22, 2018

Ok.

@mikelittle

This comment has been minimized.

mikelittle commented Oct 28, 2018

That worked for me too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment