Keep styling after spliting a block

[Jackie6]

Description

Fix #13740

How has this been tested?

• Local
• unit tests
• Local e2e tests
• Browser testing

Screenshots

Types of changes

Update the splitBlock function such that it will make the styling of the new block same as the current one.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[mapk]

@Jackie6 Thanks for taking this on! I really appreciate you jumping in and contributing. 🎉

I didn't even think about what should happen if the word or paragraph is broken in the middle. Great exploration.

Your PR looks fantastic with one exception: when I hit Enter at the beginning of the sentence, the block above still retained the old styling. I think that block should have all styling removed if there's no content remaining in it. You've got the first part, having the styling follow the block content, which is awesome. We just need that other part now.

[Jackie6]

@mapk Hi Mark! Thanks for your comments!
I have thought about the case you mention before I commit. I think when I hit Enter at the beginning of the sentence, the block above should retain the old styling. This is because when you hit Enter which results in split block, it seems that we just clone the styling of the current block and create a new one. No matter whether the before has content or not, we should keep the styling unchanged.

Anyway, I can make another commit to fix the issue if needed. That is not hard.

[mapk]

@Jackie6 I hear what you're saying. When the block retains some of the content (it splits) I'm fine with it keeping the styling, but when it's void of content, let's remove the styling as well. The reason I'm pushing for this is that I've seen people use this method to move a block down b/c they want a fresh block above it.

Thanks so much for your time on this!

[mapk]

This is working really well. There's just one last thing before merging it. When moving the paragraph down by pressing enter the styling follows which is great, but when I want to move that paragraph back up by pressing delete I lose the styling again.

The first keyboard press is enter.
The second keyboard press is delete.

@Jackie6 can you make this last change? Just keep the styling when moving the paragraph back up to the original position. Is this possible?

[Jackie6]

Hi @mapk Thanks for your ideas. I have modified the code to keep styling after pressing delete

[dmsnell]

Oops, looks like we might have gotten some merge commits pollute the diff. Any chance you might be able to rebase this against the latest master @Jackie6?

[Jackie6]

Hi @dmsnell I have rebased my code. Is this what you expect?

[dmsnell]

much better @Jackie6 ! thanks

[mapk]

I just tested everything from a functional/design perspective and it works as expected. It's great! LGTM

[Jackie6]

Hi @aduth , thanks for your review, it is really helpful. You are right,

mergeBlocks is not specific to paragraph. We can't assume paragraph attributes here.

But I don't think the correct wat to apply the attributes of blockB into blockA is to check the mergeblocks method in packages/block-editor/src/store/effects.js . In fact, a better way would be change blockType.merge(attributes, attributesToMerge) method. If the blockType is core/paragrph, then the merge mehod should be updated.

[aduth]

@ellatrix Could you advise here given it appears it could directly conflict with your proposed changes at #14765 ? Likewise @Jackie6 , perhaps you might have some feedback for the proposal at #14765 which seems like it could impact your intended goal.

[ellatrix]

Yes, some of it overlaps with #14765.

[Jackie6]

Hi @aduth @ellatrix I have reviewed the changed files in #14765 and #14464. I think Ella's pr is better since it fixed more issues. I do not have the permission and make a commit on Ella's pr. But I would advise Ella to change

gutenberg/packages/block-library/src/paragraph/edit.js

Line 201 in 3ceec4d

onSplit={ ( value ) => createBlock( name, { content: value } ) }

to

onSplit={ ( value ) => createBlock( name, value.length ? { ...attributes, content: value } : { content: value } ) }

also change

gutenberg/packages/block-library/src/paragraph/index.js

Lines 147 to 151 in 3ceec4d

	merge( attributes, attributesToMerge ) {
	return {
	content: ( attributes.content || '' ) + ( attributesToMerge.content || '' ),
	};
	},

to

	merge( attributes, attributesToMerge ) {
		return {
			backgroundColor: attributes.content.length === 0 ? attributesToMerge.backgroundColor : attributes.backgroundColor,
			textColor: attributes.content.length === 0 ? attributesToMerge.textColor : attributes.textColor,
			fontSize: attributes.content.length === 0 ? attributesToMerge.fontSize : attributes.fontSize,
			content: ( attributes.content || '' ) + ( attributesToMerge.content || '' ),
		};
	},

Then, the issue #13740 will also be fixed and this pr can be closed.

[ellatrix]

@Jackie6 Thanks for looking into it. I think it's fine to scope #14765 to what it currently fixes (hopefully that PR is merged soon). After merging that one, we can follow-up with fixing this.

[ellatrix]

Ok, I actually took it over in e740fcf. Seemed like a trivial fix. :)