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 Library]: Update the relationship of No results block to ancestor #48348

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Feb 23, 2023

What?

This PR just add a bit more flexibility to the insertion of No results inside a Query Loop block. Previously you could insert them through the inserter only if they were direct parents of Query Loop, but now they can be inserted in nested blocks(ex Group), as long as they have Query Loop as an ancestor.

Similar with: #67657 and #58602

Testing Instructions

  1. Inside a Query Loop block add a Group and then insert through the inserter No results block.
  2. The above block cannot be inserted through the inserter if it doesn't have a Query Loop ancestor.
  3. No regressions are introduced and everything else should work as before.

@ntsekouras ntsekouras added [Package] Block library /packages/block-library [Block] Query Loop Affects the Query Loop Block [Block] Query Pagination Affects the Query Pagination Block - used for pagination within the Query Loop Block [Block] Post Template Affects the Post Template Block labels Feb 23, 2023
@ntsekouras ntsekouras self-assigned this Feb 23, 2023
@gziolo
Copy link
Member

gziolo commented Feb 23, 2023

Was there any discussion about which blocks could benefit from using ancestor instead of parent? Comments block could also get similar updates wherever applicable:

Screenshot 2023-02-23 at 12 23 48

Actually, it's the only block that uses that API, but in a slightly different way than this PR explores.

It also made me wonder whether the Comments block doesn't use ancestor for the Comment Template block to avoid the risk of inserting the block into self.

@ntsekouras
Copy link
Contributor Author

Was there any discussion about which blocks could benefit from using ancestor instead of parent?

In various issues at some point, but not a separate issue. We also introduced a new Query Loop PR that wraps these blocks inside Group blocks. That was my main trigger for this PR right now. I think the main reason for some comment blocks to use this API, is because they cannot work outside of this context at all.

It also made me wonder whether the Comments block doesn't use ancestor for the Comment Template block to avoid the risk of inserting the block into self.

That's a good point and is something we might not want for Post Template... Maybe @ockham or @c4rl0sbr4v0 could share some insights?

@webmandesign
Copy link
Contributor

+1 This would really be great! It would most likely solve my issue #48296 when I want to use core/query-pagination (and also core/comments-pagination) as a Template Part or Pattern.

@ntsekouras ntsekouras force-pushed the update/query-direct-children-to-ancestor branch from 8a1b889 to 063569b Compare December 9, 2024 12:06
@ntsekouras ntsekouras marked this pull request as ready for review December 9, 2024 12:09
Copy link

github-actions bot commented Dec 9, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ntsekouras <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: gziolo <[email protected]>
Co-authored-by: webmandesign <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ntsekouras ntsekouras changed the title [Block Library]: Update the relationship of some posts needed inside Query Loop to ancestor [Block Library]: Update the relationship of No results block to ancestor Dec 9, 2024
@ntsekouras ntsekouras added the [Type] Enhancement A suggestion for improvement. label Dec 9, 2024
@gziolo
Copy link
Member

gziolo commented Dec 9, 2024

Yeah, good idea to move forward with this change 👍🏻

Copy link

github-actions bot commented Dec 9, 2024

Flaky tests detected in 5e67986.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12235468898
📝 Reported issues:

@ntsekouras ntsekouras merged commit 91b130b into trunk Dec 9, 2024
62 of 64 checks passed
@ntsekouras ntsekouras deleted the update/query-direct-children-to-ancestor branch December 9, 2024 12:55
@github-actions github-actions bot added this to the Gutenberg 19.9 milestone Dec 9, 2024
@fabiankaegy fabiankaegy added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Dec 9, 2024
yogeshbhutkar pushed a commit to yogeshbhutkar/gutenberg that referenced this pull request Dec 18, 2024
…cestor` (WordPress#48348)

Co-authored-by: ntsekouras <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: gziolo <[email protected]>
Co-authored-by: webmandesign <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Template Affects the Post Template Block [Block] Query Loop Affects the Query Loop Block [Block] Query Pagination Affects the Query Pagination Block - used for pagination within the Query Loop Block Needs Dev Note Requires a developer note for a major WordPress release cycle [Package] Block library /packages/block-library [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants