Update Terms Query block to support nesting & inheritance #71916
Conversation
cr0ybot
requested review from
ajitbohra,
fabiankaegy,
juanmaguitar and
spacedmonkey
as code owners
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Working on fixing some weirdness with the inherited taxonomy not being reflected in the control, as alluded to in this comment from @kmanijak: #71596 (comment) While the control could just be hidden, I think I'd prefer to display it |
cr0ybot
changed the title
t-hamano
added
[Type] Enhancement
There was a problem hiding this comment.
@cr0ybot thanks for taking time to extract it from bigger PR, appreciate it! 🙌
As I'm kind of representing WooCommerce interest for this block, I want to make sure Terms Query works with taxonomies provided by plugin. I'm happy to help if needed but don't want to duplicate effort. Are you planning on addressing the comments regarding template/taxonomy recognition? If not, do you want me to create PR targeting this branch?
Even though we can do followups I want to make sure block in trunk is in usable state hence the question.
The current iteration of the Terms Query and Term Template blocks do not account for nesting because they render a nested list of terms if you select the hierarchical option. This does not allow for control over the layout and design of different levels of the hierarchy, and if a full hierarchical term list is desired that can already be accomplished with the Term List block.
You convinced me to the nesting idea, especially that, yeah, Term List block just does render terms list and allows for hierarchical rendering. No point it duplicating that but in blockified manner.
I'm still not convinced to Query Type control though! 😅 But I must agree that with nesting approach the declarative way is not that good either. I'm planning on adding include/exclude controls. With the Query Type how would you see them working:
- Exclude should be available for both: Default and Custom query
- Include doesn't make much sense for Default query I think (we cannot always determine what terms are gonna be displayed on the frontend).
I actually envisioned "Include" option more like "Handpicked terms" option, maybe that should be just third option to Query Type:
- Default - automatically adjusting to context
- Custom - adjust the query
- Handpicked - choose specific terms
WDYT?
FYI: @mikachan
This is all handled in this PR! There were some bugs in the initial PR that prevented it from working, and I apologize for not testing thoroughly enough before asking for review. While working on this block I have been ensuring that WooCommerce is installed in order to test with custom taxonomies, and I mentioned doing the same in the testing instructions. I did test that this PR works specifically for showing child terms on a product category template. If you've found a situation where it doesn't work, please do let me know and I can fix it or merge in a PR. The most important part of this PR for WooCommerce is the addition of a filter for the
Understood! I think this PR is a better place to iterate on that control if you don't mind repeating some of that discussion. Is the problem primarily the "Default" label? I know the Query Loop block used to specify "inherit"... Ok so I just did a git blame to figure out what the discussion was behind that change and I discovered that you authored that change? Sorry to be blunt but I'm a bit confused, what gives?
I think that there are use cases for both "sticky" terms at the top of the list, and "handpicked" terms only, but I think both can be accomplished by the former. That is, if we provide a sticky/include control and limit the number of terms to the amount of chosen terms, it would function the same as "handpicked" without introducing a third "query type". I suppose if really needed we could add a conditional control that appears if the user has selected sticky terms to show only sticky terms, which might also hide the "max terms" slider. I just don't think that included terms counts as a query type different from "custom". Exclude is a bit more difficult, and just like |
|
Side note here before I start making more changes, but having the default The downside to this approach is that blocks in the wild could be affected by updates to those fallback values. The only alternative I could think of is to detect if we're inheriting and updating the blocks attributes to remove the |
No need to apologies! And I must have made some mistake rebuilding then 😅
Hahah 😄 Yeah, I (and many others!) considered "Inherit..." option a bad UX choice. I found out an issue suggesting Query Type: Default | Custom and basically implemented in straight away and so it got merged quickly. I see it better than former version but still not great. My idea for Terms Query was to keep the control more declarative and avoid the control completely. Like a radio button in
I wasn't familiar with "Sticky" option in Query Loop. And honestly at first glance, I don't understand what it does and how setting Include Exclude or Ignore impacts the query... and hence I don't consider it intuitive 😅 
But I'll play with it and so I can give my opinion.
While I agree, I think "handpicked terms" may be a common use case. And I think we should be exposing and simplifying such options to users rather than making it achievable by some specific controls combination. Basically, to reduce the cognitive load required to achieve specific use case. But I'm not saying it must be a new Query Type, it was just an idea, some conditional control to "show only", as you suggested, would do as well. |
…xonomy and parent if inheriting
|
PR updated with a better approach to inheritance. This involves stripping out the Edit: shoot, need to fix the tests. |
lib/load.php
Outdated
| require __DIR__ . '/compat/wordpress-6.9/term-data-block-bindings.php'; | ||
| require __DIR__ . '/compat/wordpress-6.9/terms-query-block.php'; |
There was a problem hiding this comment.
Furthermore, these two PHP files should have an additional check for the experimental settings:
if ( gutenberg_is_experiment_enabled( 'gutenberg-block-experiments' ) ) {
require __DIR__ . '/experimental/term-data-block-bindings.php';
require __DIR__ . '/experimental/terms-query-block.php';
}There was a problem hiding this comment.
It also looks like these were improperly located within the WP_REST_Controller check if I'm reading the comment These files only need to be loaded if within a rest server instance correctly, so I've moved them both outside. @t-hamano Let me know if I've done that incorrectly.
There was a problem hiding this comment.
Thank you, all files and code seem to be placed correctly.
There was a problem hiding this comment.
I'm pretty sure that as a result, this PR could be marked as No Core Sync Required? There is already a core backport PR open for the original Terms Query block PR #70720 but since it's been marked as experimental I assume that PR will stay open until the experimental flag is removed.
There was a problem hiding this comment.
Yes, it's fine to prepare a core backport PR beforehand, but it should not be committed if the feature is still experimental. For now, this backport changelog file should also be deleted.
There was a problem hiding this comment.
@t-hamano Thanks, I've removed that too.
sunyatasattva
left a comment
sunyatasattva
left a comment
There was a problem hiding this comment.
Really good work on the new inheritance model, it sounds like this is going to be more future-proof, which is exciting! I wanted to jump in to comment on something, though:
I think that there are use cases for both "sticky" terms at the top of the list, and "handpicked" terms only, but I think both can be accomplished by the former. That is, if we provide a sticky/include control and limit the number of terms to the amount of chosen terms, it would function the same as "handpicked" without introducing a third "query type". I suppose if really needed we could add a conditional control that appears if the user has selected sticky terms to show only sticky terms, which might also hide the "max terms" slider. I just don't think that included terms counts as a query type different from "custom".
Maybe I'm misunderstanding something of your proposal here, but I don't think allowing the selection of “sticky” terms would be enough to cover the “handpicked” use-case. Stickies are set once in a central place, which of course has the advantage of being able to change them from a central location to update every block downstream.
However, this is a double-edged sword as there are legitimate use-cases for which users need different hand-picked selection for different blocks. For example:
- Users have a newsletter. On each edition of the newsletter (or to different segments of their audience) they have different landing pages which showcase a different selection of categories.
- Even within the same page, I might want to have two different query blocks. At the top I want to showcase my Clothing, Apparel and Shoes categories. At the bottom, I want to showcase some Branded Merch category. These are examples of product categories, but they can be replaced with publishing categories just as easily (at the top I want my stickied categories, at the bottom a selection of fun articles about meme cultures or sponsored stuff).
Unless I misunderstood your proposal, I wager we really need a hand-picked option.
|
@sunyatasattva Thanks for the feedback! I do think we're getting sidetracked by something that isn't part of this PR at all, but I will say that I used "sticky" and the concept of "hand-picked" interchangeably there when I probably shouldn't have. There is currently no core mechanism for "sticky" terms (or even sticky posts for custom post types, but I digress) so I don't think that's in the cards anyways for the time being. All I was suggesting is that an "include these terms" feature in concert with a max items setting theoretically handles the use case of only showing the terms you select for that particular terms query, but I think I agree that it would be nice to have a "show only selected terms" option. Edit: I think additional discussion about include/exclude terms needs to either happen in it's own issue or in the main Terms Query tracking issue #49094. |
mikachan
added
the
No Core Sync Required
mikachan
left a comment
mikachan
left a comment
There was a problem hiding this comment.
Thanks for splitting this functionality out into its own PR 🙇
I've caught up on all the discussion here and tested lots of different types of queries. Most scenarios are testing well but I did notice an odd bug in the post editor, where the child terms are rendered on the top level, and then are rendered again as a nested list:
| Editor | Front end |
|---|---|
 |
 |
Here's my block markup:
<!-- wp:terms-query {"termQueryId":1,"termQuery":{"perPage":10,"pages":0,"order":"asc","orderBy":"name","hideEmpty":true,"include":[],"exclude":[],"inherit":false}} -->
<div class="wp-block-terms-query"><!-- wp:term-template {"layout":{"type":"default"}} -->
<!-- wp:group {"metadata":{"name":"Term Name with Count"},"style":{"spacing":{"blockGap":"0.5rem"}},"layout":{"type":"flex","orientation":"horizontal"}} -->
<div class="wp-block-group"><!-- wp:paragraph {"metadata":{"name":"Term Name","bindings":{"content":{"source":"core/term-data","args":{"key":"name"}}}}} -->
<p></p>
<!-- /wp:paragraph -->
<!-- wp:paragraph {"placeholder":"(count)","metadata":{"name":"Term Count","bindings":{"content":{"source":"core/term-data","args":{"key":"count"}}}}} -->
<p></p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->
<!-- wp:terms-query {"termQueryId":1,"termQuery":{"perPage":10,"pages":0,"order":"asc","orderBy":"name","hideEmpty":true,"include":[],"exclude":[],"inherit":true}} -->
<div class="wp-block-terms-query"><!-- wp:term-template {"layout":{"type":"default"}} -->
<!-- wp:group {"metadata":{"name":"Term Name with Count"},"style":{"spacing":{"blockGap":"0.5rem"}},"layout":{"type":"flex","orientation":"horizontal"}} -->
<div class="wp-block-group"><!-- wp:paragraph {"metadata":{"name":"Term Name","bindings":{"content":{"source":"core/term-data","args":{"key":"name"}}}}} -->
<p></p>
<!-- /wp:paragraph -->
<!-- wp:paragraph {"placeholder":"(count)","metadata":{"name":"Term Count","bindings":{"content":{"source":"core/term-data","args":{"key":"count"}}}}} -->
<p></p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->
<!-- /wp:term-template --></div>
<!-- /wp:terms-query -->
<!-- /wp:term-template --></div>
<!-- /wp:terms-query -->
I'm also not sure how intuitive it is to insert an inner Terms Query block to render nested terms, although I am a fan of the increased flexibiliy this brings. If we go ahead with this structure, I'm wondering if we could follow-up with some additional functionality that reduces this flow to a single click or a toggle option (no need to block this PR though). For example, a similar toggle option as before, "Show hierarchy", but this now inserts the additional block/s (and removes them when untoggled).
Co-authored-by: Sarah Norris <[email protected]>
Bah, good catch. This seems to be due to the |
@mikachan Definitely needs to be a followup discussion for sure. Not sure how I feel about a toggle for that kind of thing, and I don't know how easily we would be able to detect how many levels of hierarchy there would need to be for any given term. There is another piece to this that I think is worth discussion that could confuse things even more, which is that we could still allow |
This sounds like a nice addition just for increased flexibility, but the controls could definitely become confusing. We can discuss with design what might work best when we start working on it. |
…happen to be on a single page
|
@mikachan I've fixed some issues with nested term queries by setting a "context" for the query at the top-level Terms Query block. This allows nested queries to know whether they should take the |
Nice! This fixed the issue I was seeing in the post editor: 
I think I've spotted another issue. When I'm in the site editor, if I use the default Query Type, I'd expect the query to inherit the current context and show the child terms, but instead I'm seeing all terms (the same as what shows for the default custom query): 
|
|
Thanks @mikachan, I found and fixed a hole in my logic for determining taxonomy and term slug from the current template. We have to handle category and tags differently from other taxonomies because those templates are not prefixed by "taxonomy-", and similarly we have to handle them differently on render because I am now having trouble selecting a taxonomy with inherit on for a single product template, so looking into that next. |
| return { isSingular: true }; | ||
| } | ||
| let isSingular = false; | ||
| let templateType = templateSlug === 'wp' ? 'custom' : templateSlug; |
There was a problem hiding this comment.
When the slug is wp?
Also nit: we don't need to have isSingular as let. We assign a value only once below.
There was a problem hiding this comment.
Fixed the nit. I admittedly don't have the full context for this detail of this utility but I moved it whole-cloth from the query block because the same functionality was required for this block with some additions.
There was a problem hiding this comment.
I think wp is used as the template slug when a Query Loop is inserted into a post/page context, but I can't replicate this now.
| let isSingular = false; | ||
| let templateType = templateSlug === 'wp' ? 'custom' : templateSlug; | ||
| let templateQuery = null; | ||
| const singularTemplates = [ '404', 'blank', 'single', 'page', 'custom' ]; |
There was a problem hiding this comment.
What is the blank template?
There was a problem hiding this comment.
This util is moved from the query block and only altered enough to provide the templateQuery in the return value.
There was a problem hiding this comment.
blank is a type of singular template that can be created in the editor.
|
I'm still reviewing and getting context for this block, but would it be possible to split the PR a bit? |
|
Hello 👋🏻 So here is the thing, while testing the Terms Query block change with this PR, I noticed some unexpected behaviour related to the There are two available options:
When switching from
I believe this might be a regression, as I didn’t observe this issue in Stable Gutenberg V21.7. ✅ Not sure if this is already known, but I wanted to flag it in case it’s related to the recent updates to Terms Query. Environment
ScreencapScreen.Recording.2025-10-07.at.18.19.44.mov |
|
Closing in favor of #72187. |
7 participants
What?
Closes #71911
This PR is the nesting & inheritance features split out from #71596. It removes hierarchical term rendering in favor of handling child terms via nested blocks.
Why?
The current iteration of the Terms Query and Term Template blocks do not account for nesting because they render a nested list of terms if you select the hierarchical option. This does not allow for control over the layout and design of different levels of the hierarchy, and if a full hierarchical term list is desired that can already be accomplished with the Term List block.
There are also discrepancies with how the terms are queried in the editor vs the front end (single query for 100 terms vs multiple queries for each level, respectively). This makes extending the block's query difficult and unpredictable, and no filter is yet available for filtering the query.
Handling this block as a flat query for terms that passes the term ID in context also allows for other inheritance situations to be handled in the same manner, such as inheriting terms from a post (on a single page or within a query loop) or inheriting the term from a taxonomy archive in order to display links to child term archives.
How?
build_query_vars_from_terms_query_block()function to be merged into core alongside the query block's similar functionterms_query_block_query_varsfilter for extending the querytermId(andtaxonomy) from context asparentfor nested Term Query blockstermQuerycontext value in the container Terms Query block instead of passing from attributes directly, because the Term Template block can only see thetermQueryfrom its parent and is unable to reach up a level to detect the parent context's taxonomyinherittotrueon a taxonomy archive template automatically inherits the queried term, so there is no need to explicitly select the taxonomy, which makes copying blocks and duplicating templates simplerTesting Instructions
You must enable "Experimental blocks" in the Gutenberg plugin settings!
It is recommended to install the WooCommerce plugin when testing to ensure custom taxonomies work as intended.
Testing Instructions for Keyboard
The only "new" control is the query type control, copied from the Query Loop block. Focusing on this control via tabbing should allow you to select default or custom via arrow keys.
Screenshots or screencast
Parent Term Query (grid):
Nested Term Query (list):