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

Introduce get_blocks function #9208

Closed
wants to merge 3 commits into from
Closed

Introduce get_blocks function #9208

wants to merge 3 commits into from

Conversation

obenland
Copy link
Member

@obenland obenland commented Aug 21, 2018

See #8352.

Description

A convenient wrapper for gutenberg_parse_blocks() that can be used as a template tag as well, complimentary to has_blocks() and has_block().

How has this been tested?

Through unit tests, as well as checking the output of get_blocks() on 'template_redirect'.

Types of changes

Adds a wrapper function for gutenberg_parse_blocks() that can be used inside and outside of loops.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

A convenient wrapper for `gutenberg_parse_blocks()` that can be used as a template tag as well, complimentary to `has_blocks()` and `has_block()`.

See #8352.
@obenland obenland added [Type] Enhancement A suggestion for improvement. [Feature] Block API API that allows to express the block paradigm. labels Aug 21, 2018
@obenland obenland added this to the 3.7 milestone Aug 21, 2018
@mtias mtias mentioned this pull request Aug 21, 2018
16 tasks
These test both `get_blocks()` and `gutenberg_parse_blocks()`.
@obenland
Copy link
Member Author

I was thinking about a complimentary get_block() function, but given how block type instances are not necessarily unique within a post, using the singular in the function name didn't seem to make much sense.

Maybe a get_blocks( $block_type, $post ) to return an array of all blocks of a specific type? Not sure how useful that would be. It could also be argued that devs could filter the return value of get_blocks() themselves, which is basically what that would do anyway. Thoughts?

@obenland obenland changed the title [WIP] Introduce get_blocks function Introduce get_blocks function Aug 21, 2018
* @since 3.7.0
* @see gutenberg_parse_blocks()
*
* @param int|string|WP_Post|null $post Optional. Post content, post ID, or post object. Defaults to global $post.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a little too magical for me. Accepting four potential variable types that each mean different things is more confusing than it is helpful.

Given gutenberg_parse_blocks() already exists for parsing blocks from a string, I'd suggest get_blocks() should accept a WP_Post object, $post_id Post ID, or null (which would default to the global $post).

@danielbachhuber
Copy link
Member

Maybe a get_blocks( $block_type, $post ) to return an array of all blocks of a specific type? Not sure how useful that would be. It could also be argued that devs could filter the return value of get_blocks() themselves, which is basically what that would do anyway. Thoughts?

What about a gutenberg_parse_block_types( $block_type, $content ) ?

@aduth
Copy link
Member

aduth commented Aug 22, 2018

What about a gutenberg_parse_block_types( $block_type, $content ) ?

I'm a bit hesitant on this since "block type" has a special distinct meaning from the "block"; it's the term we use to describe its abstract definition (register_block_type). That sounds not like what we're proposing to return from a gutenberg_parse_block_types function, which might be more appropriately named something like gutenberg_parse_blocks_of_type

@aduth
Copy link
Member

aduth commented Aug 22, 2018

Re: gutenberg_parse_blocks vs. get_blocks, we should consider what we want the landscape of final available functions to be. We could collapse what we have as gutenberg_parse_blocks (eventually wp_parse_blocks?) into get_blocks if we want it to be an overloaded function returning a parsed result. And if we don't, then I think we should make clearer that get_blocks is specific to certain usages (e.g. get_post_blocks).

@danielbachhuber
Copy link
Member

If we want to have some function that magically references the global $post, I think it should be separate from the function that parses a string for blocks.

Other than that, I don't have strong opinions about naming. PHP and WordPress have inconsistent naming patterns as it is, so I imagine we could find prior art in any direction we look. I'd prefer to delegate naming to one or two people over making it a group decision.

@gziolo
Copy link
Member

gziolo commented Jan 31, 2019

This PR is no longer relevant to Gutenberg plugin now that all the surrounding logic were moved to WordPress core. I'm closing this PR, however, we might still want to open a ticket against WordPress core. What do you think?

@gziolo gziolo closed this Jan 31, 2019
@gziolo gziolo deleted the origin/add/get_blocks branch January 31, 2019 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants