Add 'hierarchy' attribute to taxonomy filter - #68
Merged
Merged
Conversation
Adds options for "flat" (default), "collapsed", or "nested" display of hierarchical taxonomies.
Playwright — PHP 8.2 / WP latestDetails
|
kadamwhite
approved these changes
Sep 23, 2026
Contributor
There was a problem hiding this comment.
Looks good to my human eyes, and Claude likes it too. Agent code review in details tag below
Click to expand agentic review notes (none blocking)
Correctness (verified)
- Selecting a parent matches its children. This comes from WordPress's include_children default. The
plugin sets 'operator' => 'IN' for multiple values (inc/namespace.php:123) and the default operator
for a single value. AND would have broken it, because WP expands the children into the term list and
then requires all of them. Nothing in the PR depends on it, but a comment near the tax_query would
stop someone switching to AND later. - Parents with no posts of their own still show. get_terms() keeps empty parents of non-empty children
when hide_empty is on and hierarchical is left at its default of true. The Networking fixture
exercises this correctly. - Collapsed-mode context nesting is correct. Each list below the top level reads expanded from its
enclosing - , and a grandchild's own context shadows its parent's. Server-side directive processing
renders hidden and aria-expanded="false" on first paint, which the tests rely on. - Escaping is fine. In render.php:570, $item_attributes is built only from literal strings and
esc_attr( wp_json_encode() ), so the phpcs ignore is justified.
Design points to discuss
- maxVisibleTerms only counts top-level terms in nested and collapsed modes (render.php:435). With one
parent holding 40 children, the cap never kicks in. This is deliberate and commented, but editors
won't expect it. Either note it in the control's help text, or accept it on the grounds that
collapsed mode covers this case. - Checking a parent and one of its children gives a redundant selection. The query string becomes
networking,sd-wan, which returns the same results as the parent alone. Checking a parent also
doesn't show the children as covered. It isn't a bug, but the checkbox mental model is a bit off.
Radio doesn't have the problem. - Overflow now differs by radio mode. Nested and collapsed radio get the "See all" overflow through
$render_branch, but flat radio (render.php:267-273) still loops over $terms with no overflow
handling, as before. Either close the gap or treat it as out of scope. - The [hidden] CSS rule fixes an existing bug (filters.css:124).
.wp-block-query-filter__checkbox-group label { display: flex } already beat the browser's [hidden]
styling on main. That means the existing checkbox overflow toggle never actually hid anything, and
no e2e test covers overflow. The fix is correct, but it belongs in the PR description, and possibly
in a regression test.
Smaller issues
- The select control shows multiple selections differently now (render.php:618). It used to compare
against the raw $current_value, which selected nothing when the value was a comma list. It now marks
every matching option selected, and the browser shows the last one. Flat radio changed the same way.
This only matters if two filters share a query var, so it's negligible, but it is a behavior change. - Tree helpers are rebuilt on every render (edit.js:96-115). buildTree and flattenTree are pure and
could live at module scope. - The > label locator may not be valid (taxonomy-hierarchy.spec.js:913). I'm not sure Playwright
accepts a leading combinator. :scope > label definitely works. Please confirm this test actually ran
green. - The "starts open" test is incomplete (taxonomy-hierarchy.spec.js:859). It checks aria-expanded and
the checked input but not that the child list is visible. Add toBeVisible(), since visibility is what
the CSS fix makes work. - The inherited-search assertion is weaker than before (inherited-search.spec.js:724). Relaxing it is
reasonable: the new fixture pages now appear in any-post-type search results ahead of the older
posts. Still, the new assertion only proves the search isn't "Alpha only". - Some combinations have no tests: radio with nested, overflow combined with a hierarchy, and
horizontal layout with a hierarchy.
Not flagged
The build_term_tree and flatten_term_tree recursion is O(n²). That doesn't matter here, because
get_terms is capped at 100 terms (namespace.php:210).
| /* The display rules above outrank the browser's own [hidden] styling, so the | ||
| interactivity bindings that hide overflow terms and collapsed branches need | ||
| an explicit rule to take effect. */ | ||
| .wp-block-query-filter [hidden] { |
Contributor
There was a problem hiding this comment.
Nice bugfix, in the midst of this
Co-authored-by: K Adam White <kadamwhite@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds options for "flat" (default), "collapsed", or "nested" display of hierarchical taxonomies.