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

Fix enter and copy past behaviour not respects allowedBlocks and child blocks restrictions #7230

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Jun 8, 2018

Description

Fixes part of #6569
Part of a general polishing to get #6993.

When pasting or pressing enter the allowedBlocks property was not enforced.
E.g., pressing enter most of the times created a paragraph even if the paragraph block was not allowed.

This PR makes two main changes:

  • In onInsertBlocks, we now verify if it is possible to insert the blocks passed as the argument. Not allowed blocks are ignored. This change handles pasting unallowed blocks and makes pressing enter on placeholders and similar areas not create a paragraph block if the paragraph is not allowed.

  • onSplit now checks if the split can occur (a block of a given type e.g: a paragraph can be inserted) if the division cannot happen we try to add the pasted blocks without splitting the original block.

Test block: https://gist.github.com/jorgefilipecosta/be3c487d10af57838f90e3227225c6d7 allows nesting of lists and quotes. Contains a template that inserts other blocks so we can test.

Child blocks test:
https://gist.github.com/noisysocks/9b9503bd6489ffa51088d35c7a2a8ba0

How has this been tested?

I used the block in the gist and verified that if I press enter on the placeholder inside the block, a paragraph block is not created, because the paragraph is not supported.

I verified that's it is not possible to split paragraphs and headings by pressing enter (another paragraph block cannot be inserted).

I added an image and a quote outside the block, multi-selected them and copied (command +c ). "Pasted" the content in the heading, paragraph, and list inside the test block. In all the cases the image was ignored, and the quote added. In the paragraph and heading because the split was not possible the quote was pasted after the blocks. In the list, the split was possible (because we can insert another list) so the list block was divided, and the quote was added in the middle.

I added a Product block (from Child blocks test), with two Buy buttons inside, multi-selected them and copied. I checked that they could not be pasted in paragraphs outside the Product block.

In master, it was always possible to split unless a template lock existed. And it was always possible to paste blocks even if they were not allowed in the place they are pasted.

@jorgefilipecosta jorgefilipecosta added the [Type] Bug An existing feature does not function as intended label Jun 8, 2018
@jorgefilipecosta jorgefilipecosta self-assigned this Jun 8, 2018
@jorgefilipecosta jorgefilipecosta changed the title Fix enter and copy past behaviour not respects allowedBlocks Fix enter and copy past behaviour not respects allowedBlocks and child blocks restrictions Jun 8, 2018
@@ -651,6 +659,7 @@ const applyWithSelect = withSelect( ( select, { uid, rootUID } ) => {
block,
isSelected,
hasFixedToolbar,
canInsertBlockType,
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 passing here the selector, because we need to use it, with parameters that we don't know yet here. I think this has no implications because the reference should always be the same but it is something we should keep an eye on. I'm also open to suggestions to avoid passing the selector directly.

@@ -66,6 +67,13 @@ export default function HeadingEdit( {
onSplit={
insertBlocksAfter ?
( before, after, ...blocks ) => {
if ( ! canInsertSibling( 'core/paragraph' ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

Would there be any way to move this to block-list inside insertBlocksAfter to avoid having the block implementor care about this? I feel like this logic should be part of core, not the block.

@jorgefilipecosta jorgefilipecosta added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Jun 18, 2018
@jorgefilipecosta
Copy link
Member Author

Closing in favor of #14003.

@youknowriad youknowriad deleted the fix/respect-allowed-blocks-locking-copy-paste-enter branch March 20, 2019 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Blocked Used to indicate that a current effort isn't able to move forward [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants