Skip to content

Inserter: Restrict unallowed block types more broadly#4097

Closed
aduth wants to merge 2 commits intomasterfrom
fix/allowed-types-inserter
Closed

Inserter: Restrict unallowed block types more broadly#4097
aduth wants to merge 2 commits intomasterfrom
fix/allowed-types-inserter

Conversation

@aduth
Copy link
Copy Markdown
Member

@aduth aduth commented Dec 19, 2017 •
edited by gziolo
Loading

Closes: #3990.
Related: #4091

This pull request seeks to broaden the application of the allowed_block_types to apply more universally. Prior to these changes, if a block type was not allowed, it would still be surfaced in:

  • Slash autocomplete
  • Frequently used blocks inserter
  • Drop zone transforms

Implementation notes:

I debated between the approach here (where each component is responsible for checking allowed block types) and something more low-level where the block types themselves would not be registered. I decided in this direction because "allowed types" is arguably an editor setting, selecting from the totality of block types registered.

Testing instructions:

Ensure tests pass:

npm test

Verify that restricting allowed block types is reflected in the above-listed behaviors.

Example change:

diff --git a/lib/client-assets.php b/lib/client-assets.php
index 31507acc..b1558757 100644
--- a/lib/client-assets.php
+++ b/lib/client-assets.php
@@ -811,7 +811,7 @@ function gutenberg_editor_scripts_and_styles( $hook ) {
         * @param bool|array $allowed_block_types Array of block type slugs, or
         *                                        boolean to enable/disable all.
         */
-       $allowed_block_types = apply_filters( 'allowed_block_types', true );
+       $allowed_block_types = apply_filters( 'allowed_block_types', [ 'core/paragraph' ] );
 
        $editor_settings = array(
                'wideImages'       => ! empty( $gutenberg_theme_support[0]['wide-images'] ),

@aduth aduth added the [Feature] Inserter The main way to insert blocks using the + button in the editing interface label Dec 19, 2017
@aduth
Copy link
Copy Markdown
Member Author

aduth commented Dec 19, 2017

cc @greatislander

@greatislander
Copy link
Copy Markdown
Contributor

greatislander commented Dec 20, 2017

@aduth Thanks for this! I believe it will also fix #3990 which I reported earlier.

@aduth aduth mentioned this pull request Jan 9, 2018
Merged
3 tasks
@gziolo gziolo added the [Feature] Extensibility The ability to extend blocks or the editing experience label Jan 31, 2018
@gziolo
Copy link
Copy Markdown
Member

gziolo commented Jan 31, 2018

@aduth what is the current status of this one? Should we also consider filtering based on the category name?

@aduth
Copy link
Copy Markdown
Member Author

aduth commented Jan 31, 2018

The status of this is that it lacks reviews 😄

Should we also consider filtering based on the category name?

Can you clarify what you mean by this?

@aduth
Copy link
Copy Markdown
Member Author

aduth commented Jan 31, 2018

Will rebase the branch shortly to resolve conflicts.

@aduth
Copy link
Copy Markdown
Member Author

aduth commented Jan 31, 2018

References to a few commits which are planned to be dropped in rebase:

@aduth aduth force-pushed the fix/allowed-types-inserter branch from b88af3a to 40bb47b Compare January 31, 2018 17:04
@gziolo
Copy link
Copy Markdown
Member

gziolo commented Jan 31, 2018

We recently introduced this new function which registers all core block. Can we move now filtering there?

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Jan 31, 2018

The status of this is that it lacks reviews 😄

It should be better since I discovered it :)

Should we also consider filtering based on the category name?

Can you clarify what you mean by this?

Yes, so every block has this category type specified. There is another PR open when they propose to make it possible to add custom categories to group 3rd party blocks. I was wondering if we could allow to limit blocks registered based on this value. Let’s say I want to have all blocks that have category embeds or widgets. It’s similar to this filtering but based on a different block’s field. I hope it’s better explained now.

@noisysocks
Copy link
Copy Markdown
Member

noisysocks commented Feb 1, 2018 •
edited
Loading

If we move ahead with something like #4769 then we won't need the changes that this PR makes to blocksAutocompleter.

We recently introduced this new function which registers all core block. Can we move now filtering there?

This is a good idea. We could just never register a block if it's disabled.

@noisysocks noisysocks closed this Feb 1, 2018
@noisysocks
Copy link
Copy Markdown
Member

noisysocks commented Feb 1, 2018

Wrong button 😅

@noisysocks noisysocks reopened this Feb 1, 2018
@gziolo
Copy link
Copy Markdown
Member

gziolo commented Feb 1, 2018 •
edited
Loading

I debated between the approach here (where each component is responsible for checking allowed block types) and something more low-level where the block types themselves would not be registered. I decided in this direction because "allowed types" is arguably an editor setting, selecting from the totality of block types registered.

I should read this earlier :P Sorry about that. However, I still feel that we should discuss if it makes sense to register all blocks and only filter them in UI. Unless there is a reason to register all blocks even in the case when they can't be selected. /cc @mtias

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Feb 1, 2018

Related: #3992, #3990.

@aduth aduth mentioned this pull request Feb 2, 2018
Merged
3 tasks
@gziolo gziolo mentioned this pull request Feb 5, 2018
Closed
@aduth aduth mentioned this pull request Mar 7, 2018
Closed
@jasonbahl jasonbahl mentioned this pull request Apr 25, 2018
Closed
2 tasks
@aduth
Copy link
Copy Markdown
Member Author

aduth commented May 1, 2018

We'll need to address this, though ideally not in a way which is scattered across the various implementations. Some ideation at #6067 (comment) and #6067 (comment) .

@aduth aduth closed this May 1, 2018
@gziolo gziolo deleted the fix/allowed-types-inserter branch May 7, 2018 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

[Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Inserter The main way to insert blocks using the + button in the editing interface

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@aduth @greatislander @gziolo @noisysocks