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

Block Hooks: Incorrect placement in the parent container for hooked blocks #54307

Closed
gziolo opened this issue Sep 8, 2023 · 0 comments · Fixed by #54349
Closed

Block Hooks: Incorrect placement in the parent container for hooked blocks #54307

gziolo opened this issue Sep 8, 2023 · 0 comments · Fixed by #54349
Assignees
Labels
[Feature] Block API API that allows to express the block paradigm. [Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@gziolo
Copy link
Member

gziolo commented Sep 8, 2023

Description

Every block that has inner blocks also has some wrapping HTML element. The way hooked blocks are currently implemented is that it puts the injected block at exactly first or last place in the parsed block, which happens to be outside of the wrapping element. A simplified example based on REST API response that shows the issue:

<!-- wp:group {"tagName":"main","style":{"spacing":{"margin":{"top":"var:preset|spacing|50","bottom":"var:preset|spacing|70"}}},"layout":{"type":"constrained"}} -->
    <main class="wp-block-group" style="margin-top:var(--wp--preset--spacing--50);margin-bottom:var(--wp--preset--spacing--70)">
        <!-- wp:pattern {"slug":"twentytwentythree/cta"} /-->
    </main>
    <!-- wp:ockham/like-button /-->
<!-- /wp:group /-->

It should be withing the main tag instead:

<!-- wp:group {"tagName":"main","style":{"spacing":{"margin":{"top":"var:preset|spacing|50","bottom":"var:preset|spacing|70"}}},"layout":{"type":"constrained"}} -->
    <main class="wp-block-group" style="margin-top:var(--wp--preset--spacing--50);margin-bottom:var(--wp--preset--spacing--70)">
        <!-- wp:pattern {"slug":"twentytwentythree/cta"} /-->
        <!-- wp:ockham/like-button /-->
    </main>
<!-- /wp:group /-->

Step-by-step reproduction instructions

  1. Install and activate the plugin with the hooked block. Example block can be downloaded from https://github.com/ockham/like-button/releases/tag/v0.3.1.
  2. Activate Twenty Twenty-Three theme or any other theme that uses core/comment-template block.
  3. Go to the single post page and see where the Like Button gets injected. Ensure that all templates and template parts used on that page don't have any customizations applied.
  4. Check the source of the page and notice that the Like button is after the closing tag for the comment row wrapper.

Screenshots, screen recording, code snippet

The configuration used for the Like Button:

Screenshot 2023-09-07 at 17 10 48

The same issue exists for firstChild and lastChild. Everything works correctly for before and after positions.

The issue was hard to discover because the way it's presented in the block editor is perfectly valid:

Screenshot 2023-09-07 at 17 12 04

The REST API endpoint contains the hooked block as expected, at least when you don't check it with enough attention:

Screenshot 2023-09-07 at 17 12 51

The issue is easier to spot when checking the source of the webpage because the block gets printed after (or before when using firstChild) the closing tag for the target parent block:

Screenshot 2023-09-07 at 17 13 34

It's hard to tell by looking at the page:

Screenshot 2023-09-08 at 17 47 29

Well, until you use source inspector:

Screenshot 2023-09-08 at 17 49 17

Environment info

The latest Gutenberg version.

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@gziolo gziolo added [Type] Bug An existing feature does not function as intended [Feature] Block API API that allows to express the block paradigm. labels Sep 8, 2023
@gziolo gziolo added the [Priority] High Used to indicate top priority items that need quick attention label Sep 11, 2023
@gziolo gziolo moved this to In Progress in WordPress 6.4 Editor Tasks Sep 11, 2023
@gziolo gziolo added the [Status] In Progress Tracking issues with work in progress label Sep 11, 2023
@gziolo gziolo self-assigned this Sep 11, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in WordPress 6.4 Editor Tasks Sep 14, 2023
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. [Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant