Skip to content

Conversation

@cr0ybot
Copy link
Contributor

@cr0ybot cr0ybot commented Sep 5, 2025

What?

Addresses differences between the Query block's pattern and variation picker and the Terms Query block.

This PR removes variations from the term-template block, adds variations more similar to the query block to the terms-query block, and includes an example pattern that can be chosen.

Why?

This was discussed in the main PR as one of the issues to address before merging the terms-query block.

How?

  • Removes variations from the term-template block
  • Adds variations to the terms-query block that are similar to the query block
  • Adds an example pattern registration to the terms-query block in order to show the pattern chooser
  • Reorganizes the terms-query edit component into separate files

Testing Instructions

  1. Edit a page or template
  2. Insert the Terms Query block
  3. Confirm that you are presented with the option to choose a pattern or start blank
  4. Confirm that a pattern is available and selecting it loads the pattern into the editor
  5. Repeat “Select All” behavior #3 and confirm that choosing "start blank" presents the option to select from a few variations
  6. Confirm that choosing a variation loads the variation into the editor

Testing Instructions for Keyboard

All UI patterns are copied from the query block and should work the same

  • The initial placeholder for selecting between "choose" and "start blank" should be navigable by tab and arrow keys and selectable with space or enter
  • The pattern modal should be navigable in the same way and dismissable with escape
  • The variation picker should be navigable in the same way

Screenshots or screencast

Before After
480134543-9f76280d-3eb3-4213-a823-b28ee6cd2c60 Screenshot 2025-09-05 at 5 31 27 PM

Additional Screenshots:

Screenshot 2025-09-09 at 11 48 20 AM Screenshot 2025-09-05 at 5 38 02 PM

I am open to any feedback, and in particular am looking for thoughts about default patterns and variations.

@github-actions
Copy link

github-actions bot commented Sep 5, 2025

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: cr0ybot <cr0ybot@git.wordpress.org>
Co-authored-by: MaggieCabrera <onemaggie@git.wordpress.org>
Co-authored-by: mikachan <mikachan@git.wordpress.org>

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

This was referenced Sep 5, 2025
@skorasaurus skorasaurus added the [Block] Post Terms Affects the Post Terms Block label Sep 6, 2025
@cr0ybot
Copy link
Contributor Author

cr0ybot commented Sep 7, 2025

Need to fix some tests, apparently getByRole('option', { name: 'Standard' }) is picking up the new Standard Terms pattern I added, strange that the selector is so loose...

@mikachan mikachan added the [Type] Enhancement A suggestion for improvement. label Sep 9, 2025
<!-- wp:buttons -->
<div class="wp-block-buttons">
<!-- wp:button {"metadata":{"bindings":{"url":{"source":"core/term-data","args":{"key":"link"}}}},"className":"is-style-fill"} -->
<div class="wp-block-button is-style-fill"><a class="wp-block-button__link wp-element-button">View posts</a></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to translate text like this

<div class="wp-block-group">
<!-- wp:buttons {"layout":{"type":"flex","verticalAlignment":"center"}} -->
<div class="wp-block-buttons">
<!-- wp:button {"metadata":{"bindings":{"url":{"source":"core/term-data","args":{"key":"link"}},"text":{"source":"core/term-data","args":{"key":"name"}}}},"className":"is-style-outline","style":{"spacing":{"padding":{"left":"0","right":"0","top":"0","bottom":"0"}},"border":{"color":"#00000000","radius":{"topLeft":"0px","topRight":"0px","bottomLeft":"0px","bottomRight":"0px"}}}} -->
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to avoid as much of this styling as we can. Colors won't work on all themes, we should allow the attern to inherit whatever the theme provides. I would also avoid using button variations like outline (the other pattern has an is-style-fill class too that we can probably remove too)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, you're totally right. Since we don't have a compatible Term Title block there's no great way to provide a list of links unless they're all buttons. I could recreate the original pattern with paragraphs but there would be no links... Either way you're right that this isn't great and needs to be changed.

<!-- wp:term-template {"style":{"spacing":{"blockGap":"var:preset|spacing|20"}},"layout":{"type":"flex","orientation":"horizontal","justifyContent":"left","flexWrap":"wrap"}} -->
<!-- wp:buttons -->
<div class="wp-block-buttons">
<!-- wp:button {"metadata":{"bindings":{"url":{"source":"core/term-data","args":{"key":"link"}},"text":{"source":"core/term-data","args":{"key":"name"}}}},"className":"is-style-outline","style":{"spacing":{"padding":{"left":"var:preset|spacing|30","right":"var:preset|spacing|30","top":"var:preset|spacing|20","bottom":"var:preset|spacing|20"}}},"fontSize":"small"} -->
Copy link
Contributor

Choose a reason for hiding this comment

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

same here with the font size, I tested in a theme where this looks really strange

Copy link
Contributor

Choose a reason for hiding this comment

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

Archeo:

Screenshot 2025-09-09 at 17 33 00

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. I was referencing some of the core query patterns and noticed the use of a few font size slugs but perhaps we should just use pixel values here.

Copy link
Contributor

Choose a reason for hiding this comment

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

mmmh, I don't think pixel values are better. Maybe we need design input here, but we either remove this type of opinionated styling altogether and leave it to the theme or use the preset values for both texts (the term name and the count number) so they are related to one another. As a last resort we could go for em values if we really had to before going for pixels

*/
import { Path, SVG } from '@wordpress/components';

export const link = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are these from? if we add them to the library we need to support them forever

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're from me, meant to mimic the style of the icons used in the query block variation picker.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, these need to be approved by designers before we land them then /cc @jameskoster @fcoveram

<!-- /wp:button -->
</div>
<!-- /wp:buttons -->
<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/term-data","args":{"key":"count"}}}},"fontSize":"medium"} -->
Copy link
Member

Choose a reason for hiding this comment

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

This fontSize is being inserted as large when I use this pattern. I'm not sure why, as the theme I'm testing with does have a medium font size preset:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol well as @MaggieCabrera pointed out we probably shouldn't be using font size slugs anyways.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, this is the same issue that @MaggieCabrera mentioned here. I think it looks so strange because the wrong font size slug is being pulled through, but I'm not sure why. It's probably best to just remove the font size entirely and let the block use the default paragraph size.

@cr0ybot
Copy link
Contributor Author

cr0ybot commented Sep 9, 2025

@MaggieCabrera @mikachan Thanks for taking a look. I've pushed some updates that should address unnecessary styles in patterns, translating text, and some issues with termQuery props being overridden on pattern insertion. FYI I've removed the buttons from the category list pattern, which means that there are no links. I'm not particularly satisfied with that but I've got nothing other than buttons to work with when it comes to block bindings...

</SVG>
);

export const imageTitleLink = (
Copy link
Member

Choose a reason for hiding this comment

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

It looks like imageTitleLink and imageTitleDescriptionLink are not used. Should we remove these until we need them?

* @param {IHasNameAndId[]} entities The entities to extract of helper object.
* @return {QueryEntitiesInfo} The object with the entities information.
*/
export const getEntitiesInfo = ( entities ) => {
Copy link
Member

Choose a reason for hiding this comment

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

Is this utils file copied from the Query Loop block? I can't see where many of these functions are used for the Terms Query and Term Template blocks.

These functions don't seem to be used: getEntitiesInfo, getValueFromObjectPath, mapToIHasNameAndId, useAllowedControls, isControlAllowed, useTaxonomies, useAllowedTaxonomies, isTaxonomyAllowed, useUnsupportedBlocks.

And then perhaps these could be extracted to a shared util file rather than dupicated: isControlAllowed, usePatterns, useUnsupportedBlocks

That should leave four new functions that are using the Terms Query blocks: useAllowedControls, getTransformedBlocksFromPattern, useBlockNameForPatterns, useScopedBlockVariations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mikachan Well that's embarrassing. It was initially copied from the query block and then customized for the plugin I built, then copied over to this PR without cleaning it up. I'll remove the unused functions and look for ways to extract shared utils. I've been hesitant to touch the query block code, I'm assuming packages/block-library/utils/ is the right place for these?

Copy link
Member

Choose a reason for hiding this comment

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

No worries 😄 That sounds good.

Yes, I understand being hesitant to touch the Query block code, especially as this will currently be merged into a much larger PR, which makes it more difficult to isolate these changes. Maybe we could leave the shared utils for a follow-up PR, and just duplicate those functions for the time-being.

@draganescu draganescu deleted the branch WordPress:add/terms-query-block-container September 10, 2025 14:41
@draganescu draganescu closed this Sep 10, 2025
@mikachan
Copy link
Member

Looks like this PR was closed automatically because #70720 was merged. That's not intentional! I thought it would re-target the base branch to trunk. Maybe it's because this is from a fork. @cr0ybot Please re-open this against trunk when you get a chance, and we can continue the review from there 🙇

@cr0ybot
Copy link
Contributor Author

cr0ybot commented Sep 10, 2025

@mikachan No worries, will do.

@t-hamano t-hamano added [Block] Terms Query Affects the Terms Query Block and removed [Block] Post Terms Affects the Post Terms Block labels Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Terms Query Affects the Terms Query Block [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants