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

REST API: Apply allowed_block_types filter to Block Types endpoint #24076

Closed

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Jul 20, 2020

Description

This is more of an RFC -- this code would currently produce an error.

In #21065, a new endpoint was introduced to return registered blocks:

  • __experimental/block-types List of all blocks
  • __experimental/block-types/<namespace>/<block_type> ie __experimental/block-types/core/post-author". Get details on a single block type
  • __experimental/block-types/<namespace> ie __experimental/block-types/core . Get all blocks in a single name space.

By comparison, there's an allowed_block_types hook in Core that's applied upon loading the Block Editor, allowing filtering of the blocks that are available to the user in the inserter.

I think it would make sense to apply that same filter to the block-types endpoint. However, that is not straight-forward, as the filter currently accepts a second argument: the current post object (which is, of course, readily available in a Block Editor context).

Somewhat related: WordPress/wordpress-develop#419

Questions

  • Do people agree that it makes sense to apply the filter to the endpoint?
  • If yes, how do we get hold of a post ID? Do we mandate passing one to the endpoint? That'd mean a significant API change; but maybe it is arguable that a list of blocks needs a post context? (Think e.g. Site Editor (wp_template CPTs) vs Post Editor.)

cc/ @spacedmonkey @TimothyBJacobs for opinions 🙂

How has this been tested?

Hasn't yet 😬

Types of changes

RFC

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@ockham ockham added the [Type] Question Questions about the design or development of the editor. label Jul 20, 2020
@ockham ockham self-assigned this Jul 20, 2020
@github-actions
Copy link

Size Change: 0 B

Total Size: 1.15 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.min.js 1.14 kB 0 B
build/annotations/index.min.js 3.67 kB 0 B
build/api-fetch/index.min.js 3.43 kB 0 B
build/autop/index.min.js 2.82 kB 0 B
build/blob/index.min.js 620 B 0 B
build/block-directory/index.min.js 7.63 kB 0 B
build/block-directory/style-rtl.css 944 B 0 B
build/block-directory/style.css 945 B 0 B
build/block-editor/index.min.js 125 kB 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.61 kB 0 B
build/block-library/editor.css 7.61 kB 0 B
build/block-library/index.min.js 132 kB 0 B
build/block-library/style-rtl.css 7.82 kB 0 B
build/block-library/style.css 7.83 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/block-serialization-default-parser/index.min.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.min.js 3.1 kB 0 B
build/blocks/index.min.js 48.3 kB 0 B
build/components/index.min.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.6 kB 0 B
build/compose/index.min.js 9.68 kB 0 B
build/core-data/index.min.js 11.5 kB 0 B
build/data-controls/index.min.js 1.29 kB 0 B
build/data/index.min.js 8.45 kB 0 B
build/date/index.min.js 5.38 kB 0 B
build/deprecated/index.min.js 772 B 0 B
build/dom-ready/index.min.js 568 B 0 B
build/dom/index.min.js 3.23 kB 0 B
build/edit-navigation/index.min.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.min.js 304 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.min.js 16.9 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/index.min.js 9.37 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.min.js 45.3 kB 0 B
build/editor/style-rtl.css 3.78 kB 0 B
build/editor/style.css 3.78 kB 0 B
build/element/index.min.js 4.65 kB 0 B
build/escape-html/index.min.js 733 B 0 B
build/format-library/index.min.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.min.js 2.13 kB 0 B
build/html-entities/index.min.js 621 B 0 B
build/i18n/index.min.js 3.56 kB 0 B
build/is-shallow-equal/index.min.js 711 B 0 B
build/keyboard-shortcuts/index.min.js 2.51 kB 0 B
build/keycodes/index.min.js 1.94 kB 0 B
build/list-reusable-blocks/index.min.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.min.js 5.32 kB 0 B
build/notices/index.min.js 1.79 kB 0 B
build/nux/index.min.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.min.js 2.56 kB 0 B
build/primitives/index.min.js 1.4 kB 0 B
build/priority-queue/index.min.js 789 B 0 B
build/redux-routine/index.min.js 2.85 kB 0 B
build/rich-text/index.min.js 13.9 kB 0 B
build/server-side-render/index.min.js 2.71 kB 0 B
build/shortcode/index.min.js 1.7 kB 0 B
build/token-list/index.min.js 1.27 kB 0 B
build/url/index.min.js 4.06 kB 0 B
build/viewport/index.min.js 1.85 kB 0 B
build/warning/index.min.js 1.14 kB 0 B
build/wordcount/index.min.js 1.17 kB 0 B

compressed-size-action

@TimothyBJacobs
Copy link
Member

In my opinion, the block types endpoint should return the full list of registered blocks.

We could add a for_post=<id> parameter to the block types endpoint, but I could see that list being filtered for other scenarios that aren't posts and I'd prefer not to add a bunch of those filters, particularly if . For me, the list of allowed block types is a property of the object being edited.

So I would implement this as a property on the posts endpoint that would be the list of allowed block names. It would only be available in the edit context. This might also be a time to look at implementing the proposed block-editor context.

@ockham
Copy link
Contributor Author

ockham commented Jul 21, 2020

In my opinion, the block types endpoint should return the full list of registered blocks.

I don't have a strong preference for either option, but a point can be made that the only place were blocks are currently made available in the editor does apply that filter (which is contextual). This is my main reason for wondering if the same pattern might make sense to carry over to the endpoint.

We could add a for_post=<id> parameter to the block types endpoint, but I could see that list being filtered for other scenarios that aren't posts and I'd prefer not to add a bunch of those filters, particularly if . For me, the list of allowed block types is a property of the object being edited.

Right. By comparison, what would be a typical use case for the block types endpoint? 🤔

So I would implement this as a property on the posts endpoint that would be the list of allowed block names. It would only be available in the edit context. This might also be a time to look at implementing the proposed block-editor context.

Ah, thanks for the link, wasn't aware of that conversation!

@TimothyBJacobs
Copy link
Member

I don't have a strong preference for either option, but a point can be made that the only place were blocks are currently made available in the editor does apply that filter (which is contextual).

Yeah that's definitely true. For me though, I think that list could grow so it wouldn't be the most forward compatible interface.

Right. By comparison, what would be a typical use case for the block types endpoint?

As a developer, I want to be able to limit the blocks that can be used in the widgets screen, navigation screen, and full site editing. I think those would be different filtering contexts. For those, they could perhaps be done with the same arguments because they are each backed by posts. But what about term descriptions or author archives?

@ockham
Copy link
Contributor Author

ockham commented Jul 22, 2020

I don't have a strong preference for either option, but a point can be made that the only place were blocks are currently made available in the editor does apply that filter (which is contextual).

Yeah that's definitely true. For me though, I think that list could grow so it wouldn't be the most forward compatible interface.

Right. By comparison, what would be a typical use case for the block types endpoint?

As a developer, I want to be able to limit the blocks that can be used in the widgets screen, navigation screen, and full site editing. I think those would be different filtering contexts. For those, they could perhaps be done with the same arguments because they are each backed by posts. But what about term descriptions or author archives?

Hmm, what would it mean to insert a block in an author archive? Wouldn't one just be editing the corresponding template? And for term descriptions, those are currently plain text, AFAICT 🤔

Not trying to hair-split, but it seems like a post object based filter might give us pretty good coverage of most of the use cases you're listing, and it'd be nice if

  1. we could re-use the existing filter
  2. wouldn't have to introduce a ton of contexts.

@ockham
Copy link
Contributor Author

ockham commented Jul 22, 2020

I'll add that in contexts like the Site Editor, it's possible to switch between the currently edited template (i.e. wp_template post object) on the client side (without a server side reload). This means that we can't simply apply this filter in lib/edit-site-page.php, since it'd only apply to the initially loaded template. Instead, we'd need to fetch the allowed blocks for the currently loaded template somehow from the REST API. (Just to document a use case.)

@TimothyBJacobs
Copy link
Member

Hmm, what would it mean to insert a block in an author archive? Wouldn't one just be editing the corresponding template?

The template would, but the users themselves also have a description field that often is shown in the archive. Also plain text, but I think in an ideal world it'd be a block editor. So that belongs to the user object.

And for term descriptions, those are currently plain text, AFAICT

Yeah those are currently a plain text input, I thought the idea was that we'd be able to edit those with the block editor at some point?

Instead, we'd need to fetch the allowed blocks for the currently loaded template somehow from the REST API. (Just to document a use case.)

That seems like a strong argument for making it a property of the post being edited :) Then you wouldn't need to refetch the block types definition, you'd just be getting the list of block names from the post object you've selected.

@ockham
Copy link
Contributor Author

ockham commented Jul 27, 2020

Some related discussion (about we should filter by context rather than post object): #24006 (comment)

@ockham
Copy link
Contributor Author

ockham commented Oct 12, 2020

Closing. We might opt to make allowed blocks a property of the post object (as returned by the posts/pages/post types endpoint), see discussion above.

@ockham ockham closed this Oct 12, 2020
@ockham ockham deleted the update/apply-allowed-block-types-filter-to-rest-api branch October 12, 2020 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Question Questions about the design or development of the editor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants