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

Hide insertion point if it is not possible to insert the default block #7226

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Jun 8, 2018

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

Insertion point was violating the allowedBlocks restriction of the parent block. The insertion point always inserts the default block and it may not be possible to insert the default block.

This PR makes sure insertion point only appears if it is possible to insert the default block.

Test block (transforms): gist.github.com/jorgefilipecosta/b958239761a24664685d5efc7ab48fa6

How has this been tested?

I used the "Test Transforms" block ( does not allow paragraphs ) and I verified that the insertion point does not appear inside the block.
I added a Column block, and I verified insertion point appears there.

@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 requested a review from a team June 11, 2018 09:50
@jasmussen
Copy link
Contributor

fix/dont-allow-sibling-inserter-in-blocks-that-dontsupport-defaultblock-as-child

🤘

Hard to test without the Test Transforms block, but I can verify that the appender appears in the columns block! 👍 👍

@ZebulanStanphill
Copy link
Member

See the discussion at #7168, particularly this comment by @chrisvanpatten. It would be ideal if the sibling inserter could still be used in nested contexts where Paragraph blocks are not allowed, though the sibling inserter would have to be changed to open the inserter menu directly, or the placeholder block would need to be modified as per the aforementioned comment.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Looks good 👍

…ble to insert the default block

Insertion point has violating the allowedBlocks restriction of the parent block.
@jorgefilipecosta jorgefilipecosta force-pushed the fix/dont-allow-sibling-inserter-in-blocks-that-dontsupport-defaultblock-as-child branch from 39f819c to a918ece Compare July 10, 2018 16:40
@jorgefilipecosta jorgefilipecosta merged commit 20d82de into master Jul 10, 2018
@jorgefilipecosta jorgefilipecosta deleted the fix/dont-allow-sibling-inserter-in-blocks-that-dontsupport-defaultblock-as-child branch July 10, 2018 17:06
@jorgefilipecosta
Copy link
Member Author

Merging this as it is better than what we had now where we may have blocks in an inconsistent state.
@SuperGeniusZeb I agree that showing the inserter may be a nice feature for us to try in the future and it goes inline with what we are trying with the child block placeholder in #7414.

jorgefilipecosta added a commit that referenced this pull request Apr 3, 2019
…cks, locking, and child blocks (#14003)

Supersedes: https://github.com/WordPress/gutenberg/pull/7301/files
Uses the select control implemented in #13924.
This PR changes the action creators to be a generator and have all the required validations before yielding the action.

Fixes: #14568;
Fixes: #10186;
Fixes: #13099;

Missing the update to the unit tests.

## How has this been tested?
Test block: [gist.github.com/jorgefilipecosta/b7194daac3ce26827522694fb4252c1c#file-testallowedblocksmiddleware-js](https://gist.github.com/jorgefilipecosta/b7194daac3ce26827522694fb4252c1c#file-testallowedblocksmiddleware-js)

I pasted the test block referred in the browser console.
I added the Product block.
I verified I cannot move using drag & drop the Buy button out of the Product block. (child block restriction.
I verified I cannot move using drag & drop a paragraph created outside of the Product block inside the product block. (allowed blocks restriction).
I verified that if I press enter in the buy button or image inside the product block, a paragraph is not created (allowed blocks restriction).
I added two galleries outside the product block, multi-selected them, and copied them ( ctrl + c ), I pasted them inside the list block inside the Product block and I verified they were not added (allowed blocks restriction).

I verified that when we have a template lock and we press enter on the title we now do not insert unallowed blocks.
Template lock code:

```
add_filter( 'allowed_block_types', function( $allowed_block_types, $post ) {
	if ( $post->post_type === 'post' ) {
	    return $allowed_block_types;
	}
	return [ 'core/image', 'core/button' ];
}, 10, 2 );
```
I verified that if we disable the paragraph, e.g., with a filter or inside the test block, pressing enter on the title or in placeholders does not add paragraph blocks. Pressing the sibling inserter also does not insert the paragraph (PR #7226).
Code to disable paragraph:

```
function myplugin_register_book_lock_post_type() {
    $args = array(
        'public' => true,
	  	'template_lock' => 'all',
        'label'  => 'Books Lock',
        'show_in_rest' => true,
        'template' => array(
            array( 'core/image', array(
            ) ),
            array( 'core/heading', array(
                'placeholder' => 'Add Author...',
            ) ),
            array( 'core/paragraph', array(
                'placeholder' => 'Add Description...',
            ) ),
        ),
    );
    register_post_type( 'book_lock', $args );
}
add_action( 'init', 'myplugin_register_book_lock_post_type' );
```
I added two image blocks, multi-selected them, copied them and tried to paste them on the list inside the test block. I checked the image blocks were not added inside the test block.

All these actions are possible in master.

Todo: If we accept this approach, end 2 end tests will be created before the merge that replicate this manual tests.
aduth pushed a commit that referenced this pull request Apr 16, 2019
…cks, locking, and child blocks (#14003)

Supersedes: https://github.com/WordPress/gutenberg/pull/7301/files
Uses the select control implemented in #13924.
This PR changes the action creators to be a generator and have all the required validations before yielding the action.

Fixes: #14568;
Fixes: #10186;
Fixes: #13099;

Missing the update to the unit tests.

## How has this been tested?
Test block: [gist.github.com/jorgefilipecosta/b7194daac3ce26827522694fb4252c1c#file-testallowedblocksmiddleware-js](https://gist.github.com/jorgefilipecosta/b7194daac3ce26827522694fb4252c1c#file-testallowedblocksmiddleware-js)

I pasted the test block referred in the browser console.
I added the Product block.
I verified I cannot move using drag & drop the Buy button out of the Product block. (child block restriction.
I verified I cannot move using drag & drop a paragraph created outside of the Product block inside the product block. (allowed blocks restriction).
I verified that if I press enter in the buy button or image inside the product block, a paragraph is not created (allowed blocks restriction).
I added two galleries outside the product block, multi-selected them, and copied them ( ctrl + c ), I pasted them inside the list block inside the Product block and I verified they were not added (allowed blocks restriction).

I verified that when we have a template lock and we press enter on the title we now do not insert unallowed blocks.
Template lock code:

```
add_filter( 'allowed_block_types', function( $allowed_block_types, $post ) {
	if ( $post->post_type === 'post' ) {
	    return $allowed_block_types;
	}
	return [ 'core/image', 'core/button' ];
}, 10, 2 );
```
I verified that if we disable the paragraph, e.g., with a filter or inside the test block, pressing enter on the title or in placeholders does not add paragraph blocks. Pressing the sibling inserter also does not insert the paragraph (PR #7226).
Code to disable paragraph:

```
function myplugin_register_book_lock_post_type() {
    $args = array(
        'public' => true,
	  	'template_lock' => 'all',
        'label'  => 'Books Lock',
        'show_in_rest' => true,
        'template' => array(
            array( 'core/image', array(
            ) ),
            array( 'core/heading', array(
                'placeholder' => 'Add Author...',
            ) ),
            array( 'core/paragraph', array(
                'placeholder' => 'Add Description...',
            ) ),
        ),
    );
    register_post_type( 'book_lock', $args );
}
add_action( 'init', 'myplugin_register_book_lock_post_type' );
```
I added two image blocks, multi-selected them, copied them and tried to paste them on the list inside the test block. I checked the image blocks were not added inside the test block.

All these actions are possible in master.

Todo: If we accept this approach, end 2 end tests will be created before the merge that replicate this manual tests.
aduth pushed a commit that referenced this pull request Apr 16, 2019
…cks, locking, and child blocks (#14003)

Supersedes: https://github.com/WordPress/gutenberg/pull/7301/files
Uses the select control implemented in #13924.
This PR changes the action creators to be a generator and have all the required validations before yielding the action.

Fixes: #14568;
Fixes: #10186;
Fixes: #13099;

Missing the update to the unit tests.

## How has this been tested?
Test block: [gist.github.com/jorgefilipecosta/b7194daac3ce26827522694fb4252c1c#file-testallowedblocksmiddleware-js](https://gist.github.com/jorgefilipecosta/b7194daac3ce26827522694fb4252c1c#file-testallowedblocksmiddleware-js)

I pasted the test block referred in the browser console.
I added the Product block.
I verified I cannot move using drag & drop the Buy button out of the Product block. (child block restriction.
I verified I cannot move using drag & drop a paragraph created outside of the Product block inside the product block. (allowed blocks restriction).
I verified that if I press enter in the buy button or image inside the product block, a paragraph is not created (allowed blocks restriction).
I added two galleries outside the product block, multi-selected them, and copied them ( ctrl + c ), I pasted them inside the list block inside the Product block and I verified they were not added (allowed blocks restriction).

I verified that when we have a template lock and we press enter on the title we now do not insert unallowed blocks.
Template lock code:

```
add_filter( 'allowed_block_types', function( $allowed_block_types, $post ) {
	if ( $post->post_type === 'post' ) {
	    return $allowed_block_types;
	}
	return [ 'core/image', 'core/button' ];
}, 10, 2 );
```
I verified that if we disable the paragraph, e.g., with a filter or inside the test block, pressing enter on the title or in placeholders does not add paragraph blocks. Pressing the sibling inserter also does not insert the paragraph (PR #7226).
Code to disable paragraph:

```
function myplugin_register_book_lock_post_type() {
    $args = array(
        'public' => true,
	  	'template_lock' => 'all',
        'label'  => 'Books Lock',
        'show_in_rest' => true,
        'template' => array(
            array( 'core/image', array(
            ) ),
            array( 'core/heading', array(
                'placeholder' => 'Add Author...',
            ) ),
            array( 'core/paragraph', array(
                'placeholder' => 'Add Description...',
            ) ),
        ),
    );
    register_post_type( 'book_lock', $args );
}
add_action( 'init', 'myplugin_register_book_lock_post_type' );
```
I added two image blocks, multi-selected them, copied them and tried to paste them on the list inside the test block. I checked the image blocks were not added inside the test block.

All these actions are possible in master.

Todo: If we accept this approach, end 2 end tests will be created before the merge that replicate this manual tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants