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

multiple:false support is not honored for shared and nested blocks #7957

Closed
oandregal opened this issue Jul 13, 2018 · 8 comments
Closed

multiple:false support is not honored for shared and nested blocks #7957

oandregal opened this issue Jul 13, 2018 · 8 comments
Assignees
Labels
[Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended

Comments

@oandregal
Copy link
Member

oandregal commented Jul 13, 2018

Block types that have multiple: false in their supports array are intended to be included only once in a post. Take for example the More block.

This setting is not honored when those kind of blocks are converted to Shared or are nested.

How to reproduce

For shared

  • Add a More block to your post.
  • Open the block settings menu: no Duplicate option is shown.
  • Convert the block to shared.

From this point on, the Duplicate option is shown in the block settings menu for the shared more block. The More block is eligible for adding from the inserter as well.

duplicate-bug

For nested

  • Add the Columns (beta) block.
  • Introduce as many More blocks as you want in any column.

The Duplicate option is not in the block settings menu, but you can add the More from the inserter and publish the post successfully.

duplicate-bug-nested

@oandregal oandregal self-assigned this Jul 13, 2018
@oandregal oandregal added the [Type] Bug An existing feature does not function as intended label Jul 13, 2018
@oandregal
Copy link
Member Author

Tested this with the More button, and the BlockDuplicateButton's canDuplicate function returns false when it is a regular block and true when it's shared.

@oandregal
Copy link
Member Author

Reason for that is that blockType.supports array doesn't include the multiple: false key when the More block is shared, so hasBlockSupport falls to default (which is true).

@designsimply designsimply added the [Feature] Blocks Overall functionality of blocks label Jul 18, 2018
@oandregal
Copy link
Member Author

The reason why the blockType.supports doesn't have the multiple: false prop is: when a block is converted to shared its type is changed to core/block. Any check on the type properties will be checking the ones defined for the core/block (the type of any shared block). Because supports is a type property, when the core/more block is transformed to core/block the multiple prop is no longer false, but the default (which is true).

@oandregal
Copy link
Member Author

cc @noisysocks

@oandregal
Copy link
Member Author

oandregal commented Jul 19, 2018

With my limited gutenknowledge I can only think of two approaches for shared blocks:

  • convert some (or all) the block type properties into block instance properties
  • do a "deep check" when a block is shared: if the block is shared, let's check the multiple prop in the original/referenced block type.

I lean towards the second option, but not sure about the scope (should we only do this for multiple?) and the potential side-effects of doing this.

@oandregal oandregal changed the title Blocks that declare multiple:false shouldn't be duplicatable multiple support is not honored for shared and nested blocks Jul 19, 2018
@oandregal oandregal changed the title multiple support is not honored for shared and nested blocks multiple:false support is not honored for shared and nested blocks Jul 19, 2018
@oandregal
Copy link
Member Author

oandregal commented Jul 19, 2018

I've been looking at what happens with nested blocks. If a multiple:false block is added to the post, then it can't be added in any other context (within nested blocks, for example) because the inserter would show it disabled. If the block is first added in a nested context the inserter won't know and the block will be available to add.

This is what I've got atm: nested blocks use the InnerBlocks component, which ends up using the BlockTypesList component. BlockTypesList controls is responsible to render the list of blocks that can be inserted. The list of blocks is passed as the items prop by using this selector wp.data.select( 'core/editor' ).getInserterItems();. Items on that list have a isDisabled property which is used to determine its disability status.

@oandregal
Copy link
Member Author

The getInserterItems calculates the isDisabled prop based on the top-level blocks. We can fix this by unfolding the top-level list into its components (shared to the referenced blocks, nested into children).

oandregal added a commit that referenced this issue Jul 20, 2018
When deriving whether a block should be disabled or not,
the inserter should take into account the blocks referenced by
the existing top-level shared blocks.
oandregal added a commit that referenced this issue Jul 23, 2018
When deriving whether a block should be disabled or not,
the inserter should take into account the blocks referenced by
the existing top-level shared blocks.
oandregal added a commit that referenced this issue Jul 26, 2018
When deriving whether a block should be disabled or not,
the inserter should take into account the blocks referenced by
the existing top-level shared blocks.
oandregal added a commit that referenced this issue Jul 27, 2018
When deriving whether a block should be disabled or not,
the inserter should take into account the blocks referenced by
the existing top-level shared blocks.
oandregal added a commit that referenced this issue Jul 31, 2018
When deriving whether a block should be disabled or not,
the inserter should take into account the blocks referenced by
the existing top-level shared blocks.
oandregal added a commit that referenced this issue Aug 6, 2018
When deriving whether a block should be disabled or not,
the inserter should take into account the blocks referenced by
the existing top-level shared blocks.
@oandregal
Copy link
Member Author

#8075 has landed support for taking into account nested blocks to disable blocks that can't be added more than once.

We decided to put off SharedBlocks at the moment, see #8075 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

2 participants