Skip to content

Add 'hierarchy' attribute to taxonomy filter - #68

Merged
goldenapples merged 2 commits into
mainfrom
hierarchical-taxonomy-filter
Sep 23, 2026
Merged

goldenapples merged 2 commits into
mainfrom
hierarchical-taxonomy-filter

Conversation

@goldenapples

@goldenapples goldenapples commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Adds options for "flat" (default), "collapsed", or "nested" display of hierarchical taxonomies.

Open WordPress Playground Preview

Adds options for "flat" (default), "collapsed", or "nested" display of
hierarchical taxonomies.
@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Playwright — PHP 8.2 / WP latest

passed  34 passed

Details

stats  34 tests across 8 suites
duration  35.8 seconds
commit  23544ca

@kadamwhite kadamwhite left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

  1. 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.
  2. 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.
  3. 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.
  4. 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).

Comment thread inc/namespace.php Outdated
Comment thread src/filters.css
/* 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] {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice bugfix, in the midst of this

Co-authored-by: K Adam White <kadamwhite@users.noreply.github.com>
@goldenapples
goldenapples merged commit 6455782 into main Sep 23, 2026
6 checks passed
@goldenapples
goldenapples deleted the hierarchical-taxonomy-filter branch September 23, 2026 16:29
github-actions Bot added a commit that referenced this pull request Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants