feat(ui): paginate glossary terms with cursor Prev/Next and paged expand-all (#32223) - #32473
Conversation
…and-all (#32223) * feat(ui): paginate glossary terms with cursor Prev/Next and paged expand-all * fix bot comments * fix(ui): use singular term-count label for count=1 (i18next) Addresses the gitar-bot review finding: a parent with one nested term rendered '1 terms'. Pick label.count-term (singular) vs label.count-term-plural based on the count. Adds count-term across locales. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(ui): drive glossary pagination test via cursor Prev/Next The large-glossary test asserted infinite-scroll behaviour (scroll to bottom, expect the directChildrenOf+after fetch, then 100 appended rows). Cursor pagination replaced infinite scroll, so that fetch never fires on scroll and the test timed out. Rewrite it to click Next (which triggers the directChildrenOf+after fetch), assert the page is replaced (still 50 rows, Previous now enabled), then Prev back to the first page. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ui): keep glossary hierarchy intact across partial expand-all pages Two defects flagged in review on the paginated glossary terms table: 1. buildTree promoted parentless descendants to roots. Progressive expand-all paginates all levels by name (backend orders by name,id), so a nested term can arrive on an earlier page than its parent term. buildTree then rendered that orphan as a top-level row, corrupting the hierarchy. Hold such orphans back until the page carrying their parent loads, at which point they attach correctly. 2. The Expand/Collapse-All control desynced on partial loads. The action was chosen by an expandedRowKeys === expandableKeys equality that a partially loaded tree never satisfies, and toggleExpandBtn was blindly flipped. So a second click (or one after a manual row collapse) re-ran the expand path, reset the accumulated pages, and hid the only Load More control. Drive the action, label, and icon off the explicit expand-all mode flag instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix checkstyle * fix(ui): address gitar review on glossary expand-all hierarchy - buildTree: push the live node into its parent's children instead of a shallow snapshot. With lazy child-array creation, a snapshot kept the node's original (undefined) children array, so a grandchild attached on a later page bound to the original node and vanished from the tree. - buildTree: hold an orphan back based on the parent FQN's presence (excluding an explicit glossary parent) rather than a populated parent.type, which the API may omit and would otherwise let the orphan become a spurious root. - toggleExpandAll: list the fetch helpers (and handlePagingChange) it actually calls in its dependency array so the memoized callback can't invoke a stale closure. Adds regression tests for the missing-parent-type orphan and the intermediate-with-no-inline-children grandchild cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ui): don't hold back a term view's direct children in buildTree The orphan guard treated any term whose parent term is absent from the fetched data as an orphan. That is correct in the glossary view (top-level terms have no term parent), but wrong when expanding a glossary term: fetchExpadedTree loads that term's descendants, so the term itself is not in the data and its direct children reference an absent parent — they were wrongly held back, leaving the expanded table empty. Pass the view's root FQN (the glossary or the term being viewed) to buildTree so its direct children are recognised as genuine roots. Fixes the 'Verify Expand All For Nested Glossary Terms' e2e regression. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix checkstyle --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit ba06d99)
anuj-kumary
added
UI
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Code Review ✅ Approved 2 resolved / 2 findingsPaginates the glossary terms table with cursor-based Prev/Next navigation and a paged expand-all, so large glossaries no longer load every nested term up front. The backport cleanly applies to Consider guarding ✅ 2 resolved✅ Quality: Both load-more bar and Prev/Next pager render in expand-all
✅ Edge Case: Expand-all mode not reset when search/status changes mid-flight
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
Jest test CoverageUI tests summary
|
|
2 participants
Summary
Backport of #32223 to
2.0.Paginates the glossary terms table with cursor-based Prev/Next navigation and a paged expand-all, so large glossaries no longer load every nested term up front.
Backport notes
Cherry-picked from
ba06d99e(squash merge of #32223) with-x. Resulting diff matches the original exactly: 27 files, +623/-349.One conflict, in
GlossaryTermTab.component.tsx— an import block.2.0has:whereas
mainadditionally carriesimport { TFunction } from 'i18next';(pre-existing onmainfrom an earlier PR that was never backported — not part of #32223). Taking the incoming side wholesale would have pulled that import into2.0, where nothing uses it, trippingno-unused-vars.Resolved to
2.0's shape with only #32223's actual change applied — droppingunionBy, whose sole call site this PR removes:Verified afterwards that neither
unionBynorTFunctionhas any remaining reference in the file.Test plan
Run locally against
2.0(a freshyarn install --frozen-lockfile, since2.0pins differentzustand/eslint/qsversions thanmain):jest— 7 suites, 172 tests passing (GlossaryUtils,GlossaryTermTab, and allRichTextEditorsuites)eslinton all 6 changed source files — no findingsprettier --checkon every changed file, locale JSON included — cleanyarn check-i18n— passes; the 4 new keys are present and sorted in all 24 locale filesja-jp,de-de,ru-ru,zh-cn,fr-fr,he-he— genuine translations,{{count}}/{{total}}/{{current}}interpolations intactNote
2.0runs noui-checkstylein CI (it does not fire for any2.0PR), so the eslint/prettier/i18n gates above were run by hand rather than by a workflow.🤖 Generated with Claude Code
skip-pr-checksapplied per the 2.0 backport convention (#32444, #32416, #32395): a backport has no issue of its own to link — the original is tracked on #32223.