| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Terms Query inspector controls revamp + add subterms option #71633
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
Terms Query inspector controls revamp + add subterms option #71633
Conversation
kmanijak
requested review from
ajitbohra,
fabiankaegy and
juanmaguitar
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. |
|
@kmanijak awesome :-) that's the exact need. kudos |
mikachan
added
[Type] Enhancement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left some minor comments, but otherwise this is looking good. Thanks so much for working on it, it's a great refactor!
I'll give this another test once it's been rebased after #71656 being merged.
packages/block-library/src/terms-query/inspector-controls/max-terms-controls.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/terms-query/inspector-controls/display-options.js
Show resolved
Hide resolved
|
Thanks for initial review @mikachan! Conflict has been resolved and also I addressed your comments, it's ready for another round of review! 🙌 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is testing as expected following the testing instructions 🎉
- Non-taxonomy archive template ✅
- Taxonomy archive template ✅
- Played around and changed the settings to see if they default to correct values ✅
I've left one more inline comment, but otherwise I think this is good to bring in ✨
packages/block-library/src/terms-query/inspector-controls/max-terms-controls.js
Outdated
Show resolved
Hide resolved
packages/block-library/src/terms-query/inspector-controls/max-terms-controls.js
Outdated
Show resolved
Hide resolved
… the rest of inspector controls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noticed a potential bug when using the "Show hierarchy" option in a post. It looks like the subterms are duplicated below the main query on the front end, but they display as expected in the editor:
| Editor | Front end |
|---|---|
 |
 |
@mikachan, thank you for spotting it! Eventually it's a oneliner fix but took me a moment to spot a problem 😅 734b35a. I didn't set parent properly, as I didn't realise the children terms are fetched separately in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like that fixes it!
I think we could optimise it and avoid additional queries and just build the structure ourselves after one query. But I don't think it's immediate need.
Yep, sounds like a good follow-up.
…s#71633) * Split controls into separate files * Add subterms control * Extract Empty Terms control * Change the default parent to false * Extract hierarchy control and make it render conditionally * Make the terms to show a radio * Modify the query depends on high level filters * Default to all terms when changing some attributes * Pass terms to show via context * Filter only children terms when subtree is chosen * Assign default query for subterms option * Add explanation to subterms option * Add a guard if terms is null * Add support for subtree on the frontend * Fix lint * Fix merge conflicts * Make code more defensive * Simplify the condition * Add end dot to comment * Fix lint * Rename function to match file name * Rename MaxTermsControls to MaxTermsControl to make it consistent with the rest of inspector controls * Fix duplicated taxonomies when showing all and hierarchical * Fix lint Co-authored-by: kmanijak <karolmanijak@git.wordpress.org> Co-authored-by: mikachan <mikachan@git.wordpress.org> Co-authored-by: Marc-pi <mdxfr@git.wordpress.org>
…s#71633) * Split controls into separate files * Add subterms control * Extract Empty Terms control * Change the default parent to false * Extract hierarchy control and make it render conditionally * Make the terms to show a radio * Modify the query depends on high level filters * Default to all terms when changing some attributes * Pass terms to show via context * Filter only children terms when subtree is chosen * Assign default query for subterms option * Add explanation to subterms option * Add a guard if terms is null * Add support for subtree on the frontend * Fix lint * Fix merge conflicts * Make code more defensive * Simplify the condition * Add end dot to comment * Fix lint * Rename function to match file name * Rename MaxTermsControls to MaxTermsControl to make it consistent with the rest of inspector controls * Fix duplicated taxonomies when showing all and hierarchical * Fix lint Co-authored-by: kmanijak <karolmanijak@git.wordpress.org> Co-authored-by: mikachan <mikachan@git.wordpress.org> Co-authored-by: Marc-pi <mdxfr@git.wordpress.org>
What?
Closes
termToShowfor high-level, predefined set of "filters"Why?
Subterms option is highly demanded. It should be easy to include Terms Query into taxonomy template and have it automatically display subterms of currently viewed taxonomy term.
This dictated restructure so there's no conflict between "Show only top level" vs "Show hierarchial", etc.
How?
New Inspector Controls logic is as follows with "Terms to Show" being radio button so options are mutually exclusive which removes the confusion where we had options conflicting with each other:
Examples:
And it works fine with custom taxonomies like Product Categories from WooCommerce, showing subcategories in Products by Category page:
Testing Instructions
Not taxonomy archive template
Taxonomy archive template
Other
Testing Instructions for Keyboard
Screenshots or screencast