Make WordPress Core

Opened 12 months ago

Closed 8 months ago

Last modified 6 months ago

#59572 closed enhancement (fixed)

Block Hooks: Allow passing block definitions

Reported by: bernhard-reiter's profile Bernhard Reiter Owned by: bernhard-reiter's profile Bernhard Reiter
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch has-unit-tests add-to-field-guide
Focuses: Cc:

Description (last modified by Bernhard Reiter)

Currently, it is only possible to specify a block name for automatic insertion via Block Hooks; this means the block will be inserted in its "default" state (i.e. with no way of specifying custom attributes or inner blocks). This is true for both registering a hooked block via the blockHooks field block.json, or via the hooked_block_types filter (introduced in [56673]).

We might want to allow providing a full, parsed-block style, block definition, at least for the filter:

array( 'mycommerce/mini-cart', array(
        'isPreview'    => true,
        'miniCartIcon' => 'bag',
    ) )

Props @ndiego for pointing out this shortcoming to me.

Change History (30)

#1 @Bernhard Reiter
12 months ago

  • Description modified (diff)

#2 @Bernhard Reiter
12 months ago

Tentative idea:

diff --git a/src/wp-includes/blocks.php b/src/wp-includes/blocks.php
index 00d9fdae3e..90cf58ab33 100644
--- a/src/wp-includes/blocks.php
+++ b/src/wp-includes/blocks.php
@@ -804,6 +804,10 @@ function make_before_block_visitor( $hooked_blocks, $context ) {
                         */
                        $hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
                        foreach ( $hooked_block_types as $hooked_block_type ) {
+                               if ( is_array( $hooked_block_type ) && isset( $hooked_block_type['blockName'] ) && isset( $hooked_block_type['attrs'] ) ) {
+                                       $markup .= get_comment_delimited_block_content( $hooked_block_type['blockName'], $hooked_block_type['attrs'], '' );
+                                       continue;
+                               }
                                $markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
                        }
                }

(Needs repeating the same for the other three relative positions.)

Not super pretty, but at least somewhat consistent. One downside is that we called the filter hooked_block_types; but here, they'd become actual hooked block instances.

#3 @gziolo
12 months ago

That's an interesting idea. The syntax used should probably follow how templates are shaped in PHP:

https://developer.wordpress.org/block-editor/reference-guides/block-api/block-templates/#custom-post-types

It looks like it aligns with the example shared in the description.

This ticket was mentioned in Slack in #core-editor by gziolo. View the logs.


11 months ago

#5 @Bernhard Reiter
11 months ago

It's too late to get this into 6.4.0, but we might want to consider it for 6.4.1.

This ticket was mentioned in Slack in #core-editor by colorful-tones. View the logs.


11 months ago

#7 @jorbin
11 months ago

we might want to consider it for 6.4.1.

As an enhancement, I think it's more likely that it would make more sense for 6.5.0. The one exception I see is if this was critical to functionality introduced in 6.4 in which case I think we would need to make sure there is high confidence that introducing it wouldn't have any side effects

#8 @gziolo
10 months ago

  • Component changed from General to Editor

Alternatively, we could consider doing some prototyping with block variations to mirror how the block inserter works, so that would be more like mycommerce/mini-cart/special-version where special-version is the block variation name.

Regardless, let's wait a little bit on this to get a sense of the use cases extenders have. Ideally, Block Hooks should be used with blocks that provide good defaults out of the box, similar to how you use the inserter.

This ticket was mentioned in PR #5811 on WordPress/wordpress-develop by @Bernhard Reiter.


9 months ago
#9

  • Keywords has-patch added

This is a proof-of-concept to explore setting a hooked block's layout block support attribute to the same value as its anchor block, if the hooked block is inserted before or after the anchor block.

The motivation for this are scenarios like inserting a 'Like' button block after the Post Content block. It's been brought to my attention that this currently doesn't work well for e.g. the TT3 and TT4 Block Themes. The reason for this is that those Block Themes use layout block-support on their Post Content block and post meta Group block (i.e. the Group block that typically contains the post date, byline, categories, and sometimes also the Featured Image block) to set their layout to constrained -- which sets a certain margin and padding to center those blocks horizontally.

OTOH, an automatically inserted (hooked) Like button block -- with no attributes set -- will sit at the very left of the viewport.

To solve that issue, this fix makes it so that a hooked block that has opted into layout block support will get its layout attribute set to the same value as its anchor block's.

Before After
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/96308/64bf3e81-3ab8-4d10-9ff3-1295dddaa21c https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/96308/193b01a7-85b9-4744-bbf8-8e5ef066302d

### Testing instructions

Use the latest version (0.6.0) of my demo Like button block plugin for testing. Try both with trunk, and with this PR. (Use the WordPress Playground for easy testing.)

### Notes

It seems like the concept of different layouts (as used in this context) is somewhat incompatible with Block Hooks, as it means that a hooked block needs to have an attribute set -- or needs to be wrapped in another block with that attribute -- neither of which is currently supported by Block Hooks; otherwise it won't be displayed with the desired margins.

This is probably not going to be the final version of the fix for this issue. While it tries to be fairly cautious when setting the layout attribute, I still find that it might make too many assumptions about the hooked block.

Instead, it probably makes sense to expose the necessary information (i.e. the anchor block's attributes), and allow setting the hooked block's attributes (see `#59572`), so that extenders can implement the required logic in "user space", i.e. inside of the `hooked_block_types` filter.

Trac ticket: TBD

@Bernhard Reiter commented on PR #5811:


9 months ago
#10

FYI @yansern @TimBroddin @andrewserong @tellthemachines @gziolo @tjcafferkey

Highlighting this important bit from the PR desc:

This is probably not going to be the final version of the fix for this issue. While it tries to be fairly cautious when setting the layout attribute, I still find that it might make too many assumptions about the hooked block.

Instead, it probably makes sense to expose the necessary information (i.e. the anchor block's attributes), and allow setting the hooked block's attributes (see `#59572`), so that extenders can implement the required logic in "user space", i.e. inside of the `hooked_block_types` filter.

@yansern commented on PR #5811:


9 months ago
#11

I still find that it might make too many assumptions about the hooked block.

+1.

Instead, it probably makes sense to expose the necessary information (i.e. the anchor block's attributes), and allow setting the hooked block's attributes (see #59572), so that extenders can implement the required logic in "user space", i.e. inside of the hooked_block_types filter.

I like that. I wonder if it makes sense to provide the entire $anchor_block itself, and possibly, the $parent_block to the hooked block filter. Who knows, perhaps there are other properties that the extenders might find useful from these blocks.

Side-topic: I was considering calling get_block_templates() ahead of time to scan the template structure, e.g. "does it have header, does it have footer, does it have navigation?" to determine where to best insert the block. It's probably not in this filter's interest to pass in the entire block tree, but hey, just sharing what I was thinking as an extender. :)

@Bernhard Reiter commented on PR #5811:


9 months ago
#12

Thanks a lot for weighing in, folks.
_Long comment ahead; tl;dr: I'm considering proceeding with this PR after all._

I'm still a bit torn on how much of the heavy lifting should be done by the Block Hooks mechanism itself, vs delegating it to user space.

To frame this a bit more: IMO, Block Hooks is a fairly "low-level" concept; the mechanism is supposed to insert a block in a given position in the block tree. This -- fairly straight-forward -- concept is arguably everything one needs to understand in order to use it (maybe with the limitation that blocks cannot be inserted as first or last children into certain blocks).

FWIW, during the early stages of development, I was considering adding both support for specifying attributes and for allowing insertion of patterns. Eventually -- and in the spirit of keeping things as simple as possible -- we didn't add either. The way that @mtias framed the problem for me was that _a hooked block should be conceptually equal to one that has just been manually inserted in the editor_: At that stage, no attributes have been set, nor have any inner blocks been added. This became the baseline for Block Hooks; it put the burden on block authors to choose sensible attribute default values that would work well when used with Block Hooks, which seemed reasonable. Only once a really compelling use case that required those other features had been demonstrated would we add them.

Having a hooked block aligned correctly with its sibling block(s) is certainly a compelling use case. However, in keeping with the idea of minimalism, I'd rather avoid increasing the API surface all too much. (I'm particularly skeptical of allowing pattern insertion for the sake of wrapping a third-party block in a Group block with the right attributes set to make it align properly with its sibling blocks. As for allowing attributes to be set, see `#59572`, and further below.)

IMO, the problem with layout block-support is that it is an (arguably) higher-level concept (as it goes beyond the abstract notion of a block tree and introduces concepts such as content width, which IIUC is owed to using certain blocks both in the post and in the site editor) that the Block Hooks mechanism -- or hooked blocks -- need to be aware of in order to be displayed correctly. (AFAIK, it's also the only such concept.)

That said, I'm glad that the problem can be at least partially solved by opting the hooked block into layout block-support. It still raises the question whose responsibility it is to set that attribute correctly: The Block Hooks mechanism's (i.e. Core's) or the hooked block's?

I'm still torn on that question. While I was indicating earlier that I was leaning towards making it the hooked block's (and having Core expose the information it needs to do so), I'm not 100% convinced it's the right choice. It feels wrong to require a block author to add a bunch of code simply to make their block "just work" as expected; it's the kind of decision that I think would lead to WP.org forum posts and SO threads sharing the same snippet over and over.

If a block author already opted their block into layout block-supports and hooked it after e.g. core/post-content, it seems like they stated their intention clearly enough; there's a point to be made that it should then be on Core to render the block the way they intended it, when there really seems to be only one sensible way of doing so. (Unless I'm missing an equally probable intention that could be expressed that way; please LMK!)

So I'm starting to consider proceeding with a solution as demonstrated in this PR's code. I'd still want to allow people to override the layout attribute that the mechanism would set, so I'll need to expose it; and since it would be arbitrary to just expose _one_, I'd expose them all. Thinking of adding a new filter such as

$hooked_block = apply_filters( 'hooked_block', $hooked_block, $relative_position, $anchor_block, $context );

where $hooked_block is a "parsed block array" (e.g. fields like blockName, attrs, and all that).

---

BTW I realize that if I manually insert a block with layout block-support right after another block that has layout set to constrained will currently also _not_ set the newly inserted block's layout attribute to constrained (i.e. the Block Hooks mechanism is indeed consistent with this behavior). FWIW, I'd opt to change that, too; we have some precedent where we already carry over attributes from an existing sibling block in the editor.

@Bernhard Reiter commented on PR #5811:


9 months ago
#13

One follow-up question that just occurred to me: Would it make sense to set the hooked block's default layout to constrained? If I apply the following patch to my Like button block, it's displayed the way it should be with Core trunk:

  • src/block.json

    diff --git a/src/block.json b/src/block.json
    index 99660d4..f475cf2 100644
    a b  
    99       "description": "Example block scaffolded with Create Block tool.",
    1010       "supports": {
    1111               "html": false,
    12                "layout": true
     12               "layout": {
     13                       "default": {
     14                               "type": "constrained"
     15                       }
     16               }
    1317       },
    1418       "textdomain": "like-button",
    1519       "editorScript": "file:./index.js",

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/96308/60e2c228-8661-4a27-b39e-315bc13846b0

I guess the question is how confidently can a block author say that their block only makes sense to be automatically inserted into a constrained (or flow, or flex) layout? 🤔

I'm trying to come up with scenarios where we wouldn't want to set a default layout. I guess there might be themes that use the Post Content block differently (i.e. with the layout attribute set to a different value than constrained), in which case we wouldn't want the Like button block's layout to be set accordingly. Or if we'd like to use the Like button as part of a post 🤔

Maybe @tellthemachines @andrewserong can help me build a better intuition for lumping blocks into buckets where different layout attribute settings do or don't make sense, respectively? 😊🙏

@andrewserong commented on PR #5811:


9 months ago
#14

Thanks for the continued thoughts @ockham, it's certainly an interesting problem!

Having a hooked block aligned correctly with its sibling block(s) is certainly a compelling use case. However, in keeping with the idea of minimalism, I'd rather avoid increasing the API surface all too much.

That's a good point to keep in mind. One concern that comes to me from looking over the code again is that I imagine that a very large number of 3rd party blocks are not going to be container blocks for other blocks, but will be individual blocks for a single use case, rather than wrapping other things. Therefore, those blocks are unlikely to be good candidates for opting-in to the layout block support, and so wouldn't line up correctly with the adjacent block.

That brings me back to thinking that the ideal state if we're attempting to match against another block that uses layout is to use a primitive for wrapping the block that we know does have layout (i.e. Group), or to see if there's a way to append as last child to the post content block.

I think my main concern is that the layout block support is already fairly complex and not suited to all blocks, so I can imagine us running into further issues if folks wind up needing to opt their 3rd party blocks in to layout to ensure they line up correctly with other blocks, which they shouldn't need to. And then we might be back to the forums and SO issue when they run into issues with the layout support when they're not using useInnerBlocksProps for non-container blocks 🤔

Would it make sense to set the hooked block's default layout to constrained? If I apply the following patch to my Like button block, it's displayed the way it should be with Core trunk:

I suppose a problem here is the assumptions we make about what a template is doing with the adjacent block. The adjacent block might be full width (default) layout, or use a custom content or wide size, so it's difficult to predict what would be needed there. If we're trying to line up an inserted block next to an existing post content block, I'm not sure we can reliably make it line up without copying the layout attributes.

I'm not sure if my comments here have been very helpful, I'm sorry! Very happy to continue discussing 🙂

This ticket was mentioned in PR #5837 on WordPress/wordpress-develop by @Bernhard Reiter.


9 months ago
#15

  • Keywords has-unit-tests added

Another alternative to https://github.com/WordPress/wordpress-develop/pull/5811 and https://github.com/WordPress/wordpress-develop/pull/5835. Uses some code from https://github.com/WordPress/wordpress-develop/pull/5609.

Implements an idea originally conceived of in https://core.trac.wordpress.org/ticket/59572:

We might want to allow providing a full, parsed-block style, block definition, at least for the filter:

array( 'mycommerce/mini-cart', array(
        'isPreview'    => true,
        'miniCartIcon' => 'bag',
) )

In addition, this PR seeks to also explore a possible solution for https://core.trac.wordpress.org/ticket/60126: With the ability to pass attributes for hooked blocks, we can now hook patterns (by giving core/pattern as the block type, and setting the slug attribute to the desired pattern).

To demonstrate the latter, apply the following patch to the Like Button block code:

  • like-button.php

    diff --git a/like-button.php b/like-button.php
    index 65acbc3..08a54c1 100644
    a b  
    2222 */
    2323function create_block_like_button_block_init() {
    2424        register_block_type( __DIR__ . '/build' );
     25
     26        register_block_pattern(
     27                'ockham/like-button-wrapper',
     28                array(
     29                        'title'       => __( 'Like Button', 'like-button' ),
     30                        'description' => _x( 'A button to like content.', 'Block pattern description', 'like-button' ),
     31                        'content'     => '<div class="wp-block-group"></div>',
     32                        'inserter'    => false
     33                )
     34        );
    2535}
    2636add_action( 'init', 'create_block_like_button_block_init' );
     37
     38function insert_like_button_pattern_after_post_content( $hooked_blocks, $position, $anchor_block ) {
     39        if ( 'after' !== $position ) {
     40                return $hooked_blocks;
     41        }
     42
     43        if ( 'core/post-content' !== $anchor_block['blockName'] ) {
     44                return $hooked_blocks;
     45        }
     46
     47        $hooked_blocks[] = array(
     48                'blockName' => 'core/pattern',
     49                'attrs'     => array(
     50                        'slug' => 'ockham/like-button-wrapper',
     51                ),
     52        );
     53
     54        return $hooked_blocks;
     55}
     56add_filter( 'hooked_blocks', 'insert_like_button_pattern_after_post_content', 10, 3 );
  • src/block.json

    diff --git a/src/block.json b/src/block.json
    index 99660d4..1cd5685 100644
    a b  
    1919        "viewScript": "file:./view.js",
    2020        "usesContext": [ "postType", "postId", "commentId" ],
    2121        "blockHooks": {
    22                 "core/comment-template": "lastChild",
    23                 "core/post-content": "after"
     22                "core/comment-template": "lastChild"
    2423        }
    2524}

Trac ticket: https://core.trac.wordpress.org/ticket/59572

@Bernhard Reiter commented on PR #5837:


9 months ago
#16

FYI @andrewserong @tellthemachines @yansern @TimBroddin
(Note that this is an early-stage exploration.)

@Bernhard Reiter commented on PR #5837:


9 months ago
#18

## Advantages of this approach

  • Couples addition/removal of hooked blocks with specifying attributes, thus allowing e.g. using a core/pattern block with its slug attribute specified (as demonstrated by the code in the PR desc), which isn't really possible with the hooked_block filter approach in https://github.com/WordPress/wordpress-develop/pull/5835, as explained here.

## Downsides of this approach

  • By allowing parsed block arrays as entries of the array in the first arg, we're bending the notion of hooked block _types_ quite a bit 😅
  • While it might be appealing to be able to use a pattern (and specify its slug) as a hooked block, that's not quite enough to solve https://core.trac.wordpress.org/ticket/60126, as we cannot dynamically set the containing group block's layout attribute.
  • What's more is that even if we don't use a pattern but e.g. use a Group block wrapper (and set its inner block to be a Like button), we don't know wha the adjacent anchor block's layout attribute is set to (if any) as we only have access to anchor_block_type (but not the actual anchor block instance that includes attributes etc).
    • To remediate, we would either need to add yet another argument to the filter (the full $anchor_block instance), which would be very ugly; or alternatively, to change the existing $anchor_block_type to implement PHP's ArrayAccess trait, _and_ __toString. But that's a bit too much magic _on top of allowing $hooked_block_types's entries to arrays of parsed block arrays.
  • If a filter wants to _remove_ a hooked block, lookup in the $hooked_block_types array becomes harder; rather than just searching a string, it now also needs to account for the possibility that an array entry could be an parsed block array, and would need to compare the search string to that array's blockName field.

@Bernhard Reiter commented on PR #5811:


9 months ago
#20

One concern that comes to me from looking over the code again is that I imagine that a very large number of 3rd party blocks are not going to be container blocks for other blocks, but will be individual blocks for a single use case, rather than wrapping other things. Therefore, those blocks are unlikely to be good candidates for opting-in to the layout block support, and so wouldn't line up correctly with the adjacent block.

Indeed. What got me somewhat excited about opting non-container blocks into layout block support was that the Post Content block seemed to set some precedent for that. Granted, it's a bit special (as it typically does contain blocks -- the ones from the relevant post's content), but it's not an "actual" container block (i.e. with inner blocks) in the true sense of the word; plus layout block-support seemed to work pretty straight-forward for the Like button. My takeaway was that the "actual" requirement for a constrained layout to work wasn't necessarily that it was applied to a container block but to a block that had at least one direct child (HTML) element, to which the .is-layout-constrained > :where(:not(.alignleft):not(.alignright):not(.alignfull)) selector would then apply the relevant margin settings.

I'd wager that a lot of blocks already fulfill that criterion, and it might be easy enough to rewrite a block that doesn't yet, so I was wondering if we could go as far as to standardize that requirement for the constrained layout setting to work? 🤔

That brings me back to thinking that the ideal state if we're attempting to match against another block that uses layout is to use a primitive for wrapping the block that we know does have layout (i.e. Group), or to see if there's a way to append as last child to the post content block.

Yeah, and apologies for not having discussed those options more yet. FWIW, I'm not opposed to using a wrapper Group block per se, but I have some concerns about that option.

Wrapping a third-party block in a Group block is a bit tricky in terms of how we could make it work with existing Block Hooks semantics. Currently, both the block.json field and the hooked_block_types filter accept the block _type_ of the hooked block as an argument. This shouldn't be core/group, as it would make it indistinguishable to the Block Hooks mechanism from any other hooked block that _also_ wants to wrap itself in a Group block; it would most likely lead to only one of them being actually inserted.

I've started experimenting with two different approaches that are both somewhat promising. I've listed the pros and cons for both approaches. Neither might be a perfect fit for using a Group wrapper block, but with #5835, it _might_ be possible. (I'll try it out these days to verify.)

As for last-child insertion into Post Content, I've also given it some more thought, and I don't think it's viable. I experimented a bit with this in https://github.com/WordPress/gutenberg/pull/56972, and I think it'd have a number of serious shortcomings -- first and foremost, the hooked block wouldn't show up in the editor, which is kind of a no-go.

I think my main concern is that the layout block support is already fairly complex and not suited to all blocks, so I can imagine us running into further issues if folks wind up needing to opt their 3rd party blocks in to layout to ensure they line up correctly with other blocks, which they shouldn't need to. And then we might be back to the forums and SO issue when they run into issues with the layout support when they're not using useInnerBlocksProps for non-container blocks 🤔

This might be a detail, but FWIW, I didn't use useInnerBlockProps but `__unstableLayoutClassNames` in combination with `useBlockProps` -- as does the Post Content block.

Would it make sense to set the hooked block's default layout to constrained? If I apply the following patch to my Like button block, it's displayed the way it should be with Core trunk:

I suppose a problem here is the assumptions we make about what a template is doing with the adjacent block. The adjacent block might be full width (default) layout, or use a custom content or wide size, so it's difficult to predict what would be needed there. If we're trying to line up an inserted block next to an existing post content block, I'm not sure we can reliably make it line up without copying the layout attributes.

Yeah, assuming that we can invariably set the default to one specific layout setting seems too risky, even to me 😅

I'm not sure if my comments here have been very helpful, I'm sorry! Very happy to continue discussing 🙂

Quite the opposite! I really appreciate your thoughts, they've helped shape the potential solutions I've been working on, and have made me reconsider some of my assumptions.

---

In the spirit of the aforementioned minimalism, I'm now leaning towards https://github.com/WordPress/wordpress-develop/pull/5835. This will give extenders enough flexibility and information to set the hooked block's layout attribute to match the anchor block's; and _possibly_ also to wrap the hooked block in a Group block (TBD). I'd like to see if that solves a large enough percentage of cases, and if it doesn't, we can iterate and give even more power and information to extenders.

@andrewserong commented on PR #5811:


9 months ago
#21

Glad the discussion is useful for you @ockham, I'm very much getting a lot out of it, too!

In the spirit of the aforementioned minimalism, I'm now leaning towards https://github.com/WordPress/wordpress-develop/pull/5835. This will give extenders enough flexibility and information to set the hooked block's layout attribute to match the anchor block's; and possibly also to wrap the hooked block in a Group block (TBD).

Being able to handle both cases sounds like a good idea to me. There'll be some blocks that naturally work well with layout and where copying the layout attribute will work nicely, and others that won't be a suitable candidate, so I like the idea that extenders can have flexibility in how they might implement this.

I'd wager that a lot of blocks already fulfill that criterion, and it might be easy enough to rewrite a block that doesn't yet, so I was wondering if we could go as far as to standardize that requirement for the constrained layout setting to work? 🤔

It's a very good question. If you have an idea about how it should ideally work, it'd make for a good Gutenberg issue if you have time to open one? If not I'm happy to open an issue for it and link to this discussion 🙂. If we're considering including non-container blocks, then I think there'll be a few wrinkles to iron out, as there's a bit more going on than just the wrapper > child hierarchy as the UI currently assumes that it really is a container block (i.e. in the help text, etc). Also, features that are only available on the child block will be unavailable, i.e. if someone wanted the direct child to have "wide" alignment to align with a preceding constrained block where someone has used "wide" alignment, that won't be possible on a block that isn't a true container for other blocks. So if it's formally allowed for non-container blocks, there'd likely need to be a guardrail or two (or just docs) to cover the limitations. It'd also be good to get other contributors' feedback on the idea if we're considering proposing it, since there could be other considerations we mightn't have thought of. But very well worth discussing further! It could also allow more blocks to use blockGap / block spacing, which would be cool.

This might be a detail, but FWIW, I didn't use useInnerBlockProps but __unstableLayoutClassNames in combination with useBlockProps -- as does the Post Content block.

The post content block is a really interesting case as it's _not_ a real container block, but it's doing everything it can to pretend to be one! 😄 I.e. in the editable form of the block, it uses useInnerBlocksProps here with a controlled set of blocks, and useInnerBlocksProps gets the layout classnames internally here. So it's kind of trying to do the best of both worlds and use as much of layout as it can in preview mode, and then it "really" uses it when it's in edit mode. Hopefully most 3rd party blocks won't need to jump through so many hoops to work with layout! 😅

---

From my perspective, some of the points I'm getting from the current discussion is:

  • For blocks that work nicely with layout, it'd be great if the block hooks can allow extenders to copy over layout from the anchor block
  • For blocks that aren't a good candidate for layout, there should be another option for extenders to use in order to get things to line up properly (e.g. the wrapping in a Group block idea, if it's possible)
  • There are some blocks that could be a good candidate for _some_ of the features of layout, and it could be worth seeing if it's possible to "partially" opt-in to layout support for non-container blocks, that are laying out content that is not separate child blocks. For this one, let's open an issue for it if we think it's worth pursuing further 🙂

@isabel_brison commented on PR #5837:


8 months ago
#26

I just reported the spam comments above to GH, not sure if I should delete them now or leave them here as evidence 🤔

@andrewserong commented on PR #5837:


8 months ago
#27

I just did the same 😄
Do you have an option to hide the comments in the ellipsis menu on each comment? (I don't have access to in this repo)

@isabel_brison commented on PR #5837:


8 months ago
#28

Great idea @andrewserong! Done.

@Bernhard Reiter commented on PR #5837:


8 months ago
#29

Thanks, folks!

I wonder what keeps attracting spambots to this PR; I already reported another one about a week ago 😕

@Bernhard Reiter commented on PR #5811:


8 months ago
#31

Per discussion on https://github.com/WordPress/wordpress-develop/pull/5811, https://github.com/WordPress/wordpress-develop/pull/5835#issuecomment-1877355408, and https://github.com/WordPress/wordpress-develop/pull/5837#issuecomment-1877463590, I've now opened #5835 for review.

I'll address remaining feedback on this PR soon (hopefully today or tomorrow) and will subsequently close this PR.

#32 @Bernhard Reiter
8 months ago

  • Milestone changed from Awaiting Review to 6.5

#33 @Bernhard Reiter
8 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 57354:

Block Hooks: Introduce a new hooked_block_{$block_type} filter.

Add a new hooked_block_{$block_type} filter that allows modifying a hooked block (in parsed block format) prior to insertion, while providing read access to its anchor block (in the same format).

This allows block authors to e.g. set a hooked block's attributes, or its inner blocks; the filter can peruse information about the anchor block when doing so. As such, this filter provides a solution to both #59572 and #60126.

The new filter is designed to strike a good balance and separation of concerns with regard to the existing `hooked_block_types` filter, which allows addition or removal of a block to the list of hooked blocks for a given anchor block -- all of which are identified only by their block types. This new filter, on the other hand, only applies to one hooked block at a time, and allows modifying the entire (parsed) hooked block; it also gives (read) access to the parsed anchor block.

Props gziolo, tomjcafferkey, andrewserong, isabel_brison, timbroddin, yansern.
Fixes #59572, #60126.

#34 @Bernhard Reiter
8 months ago

In 57355:

Block Hooks: Amend PHPDoc for hooked_block_{$hooked_block_type} filter.

Add missing explanation of the dynamic part of the hook name.

Follow-up [57354].
Props swissspidy.
See #59572, #60126.

#35 @stevenlinx
7 months ago

  • Keywords add-to-field-guide added

@Bernhard Reiter commented on PR #5811:


6 months ago
#36

I never got around to replying to all feedback in detail; apologies for that!

Since it's been almost two months and https://github.com/WordPress/wordpress-develop/pull/5835 has been merged (and will be part of WP 6.5), it doesn't make sense to keep this PR open any longer.

Thanks again for the valuable discussion, folks! It helped shape the hooked_block_{$block_type} filter introduced in https://github.com/WordPress/wordpress-develop/pull/5835, which doesn't make any assumptions about the presence of the layout attribute; it's up to the block author to make the relevant checks, opt their block into layout block-support, or use the filter to wrap their hooked block in a container block that has layout block-supports.

Note: See TracTickets for help on using tickets.