Editor: Speed up the hierarchical term selector for large taxonomies - #81376
Editor: Speed up the hierarchical term selector for large taxonomies#81376Mamaduka wants to merge 2 commits into
Conversation
|
Size Change: +335 B (0%) Total Size: 7.65 MB 📦 View Changed
|
tyxla
left a comment
There was a problem hiding this comment.
Thanks for working on this @Mamaduka.
While this is a perf change, it also changes how the list looks and how it behaves. That mix makes regressions easy to miss.
Should we perhaps split it - tree building and memoization in one PR, CSS and search changes in another?
| ); | ||
| } ); | ||
|
|
||
| function TermRow( { term, selectedTerms, onToggle } ) { |
There was a problem hiding this comment.
I see we're memoizing TermCheckbox here but aren't we still recreating all TermRow elements and their child elements on every render? Is that the intent and why?
There was a problem hiding this comment.
The TermRow memoization had no real effect on performance. The memoed TermCheckbox ensures that only one item re-renders when selection changes.
Manual memoization is easy to break, so I decided to split it into smaller parts.
| debouncedFilterTerms.cancel(); | ||
| setFilteredTermsTree( null ); |
There was a problem hiding this comment.
Are we intentionally missing the a11y announcements here?
There was a problem hiding this comment.
Good question. This was logical to me, no filter value, no announcement. I'll check which patterns we're using elsewhere.
| // Only a few rows show at a time. Without this, every forced layout in the | ||
| // editor costs in proportion to the number of terms. | ||
| content-visibility: auto; | ||
| contain-intrinsic-size: auto $grid-unit-30 + $focus-ring-space * 2; |
There was a problem hiding this comment.
The contain-intrinsic-size: auto 36px is set on every .hierarchical-terms-choice, but some of those rows contain a whole subtree inside .hierarchical-terms-subchoices. So a skipped top-level row with 20 children is reported as 36px instead of ~500px.
The 36px value also may also be off for leaf rows on desktop. It comes from $grid-unit-30 (24px), which is the --checkbox-input-size for small viewports, but above the break-small breakpoint the checkbox is 16px, so a actual leaf row is about 28px.
Should we apply content-visibility only to rows without children?
|
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. |
Makes sense. I got a bit carried away, since the selection interaction improvements required overlapping changes. |
05652b1 to
fe9a53f
Compare
What
PR tries to speed up the Categories panel on sites with thousands of terms. With 5,000 categories, ticking a checkbox blocked the main thread for about 157ms.
How
Each row now uses content-visibility, so the browser skips layout for rows that are off-screen. The list is already a fixed-height scroll container, so this applies cleanly. Rows have padding on the top, left, and bottom, so paint containment does not clip the checkbox's focus ring.
Alongside that, in the same component:
buildTermsTreebuilds the tree in a single pass instead of cloning it twicesortBySelectedpartitions the list instead of sorting with a comparator that rewalks each subtree on every comparisonTesting Instructions
wp term generate category --count=5000 --max_depth=3.Testing Instructions for Keyboard
Same.
Screenshots or screencast
Before
CleanShot.2026-08-10.at.12.40.42.mp4
After
CleanShot.2026-08-10.at.12.38.37.mp4
Use of AI Tools
Assisted by Claude.