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

Inserter: Restrict unallowed block types more broadly #4097

Closed
wants to merge 2 commits into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Dec 19, 2017

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
Member Author

aduth commented Dec 19, 2017

cc @greatislander

@greatislander
Copy link
Contributor

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

@gziolo gziolo added the [Feature] Extensibility The ability to extend blocks or the editing experience label Jan 31, 2018
@gziolo
Copy link
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
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
Member Author

aduth commented Jan 31, 2018

Will rebase the branch shortly to resolve conflicts.

@aduth
Copy link
Member Author

aduth commented Jan 31, 2018

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

@gziolo
Copy link
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
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
Member

noisysocks commented Feb 1, 2018

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
Member

Wrong button 😅

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

gziolo commented Feb 1, 2018

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
Member

gziolo commented Feb 1, 2018

Related: #3992, #3990.

@aduth
Copy link
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
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
Development

Successfully merging this pull request may close these issues.

4 participants