Repository navigation
Virtualize artifacts list with reusable component - #22061
Conversation
Extract SourceControlVirtualFileList to a generic VirtualizedList component and apply windowing to the artifacts table for efficient rendering of large lists. Add aria-setsize and aria-posinset announcements to windowed rows when opted in.
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (14)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe shared virtualized list is renamed and extended with optional list-position announcements and scroll-offset preservation. The artifact collection now uses windowed rows through a new Priority: ➖ Normal 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description covers the problem, solution, changes, rationale, testing, visual proof, and checklist. However, the required Linked Issue section states "None," although the template requires an issue link.
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
The component move left one consumer behind. ConflictReviewFileTree still imports the deleted source-control/listing/virtual-file-list, so the renderer build and the conflict-review test suite both fail to resolve the module.
Reviewed changes
- Extracted
VirtualizedList— the source-control list implementation moved tosrc/renderer/src/components/virtualized-list.tsx, itsSOURCE_CONTROL_*constants renamed toVIRTUALIZED_LIST_*, and the combined-diff / branch-section / section-file-list call sites now import the shared component. - Virtualized the artifacts table —
ArtifactListRowextracted,ArtifactListRowsnow windows throughVirtualizedList, and the parentdivide-yband was replaced by a per-rowLIST_TABLE_ROW_DIVIDER_CLASS. - Added list-position announcements — opt-in
announceListPositionaddsrole="list"/listitemplusaria-setsize/aria-posinset, andhasUnloadedRowspublishes-1while a server cursor remains. - Kept scroll position across threshold crossing —
initialOffsetseeds the virtualizer from the scroller's current offset, with tests covering append, search clear, and the exact 49→50 boundary. - Test coverage — a large new
artifact-list-windowing.test.tsxderives row geometry from the row's own classes, plus updates to the virtualized-list and source-control suites.
🚨 Conflict review file tree still imports the removed module
ConflictReviewFileTree.tsx:12 imports SourceControlVirtualFileList from @/components/right-sidebar/source-control/listing/virtual-file-list, which this PR moved to @/components/virtualized-list. That is a production editor surface (reached from ConflictComponents.tsx:17), so the renderer build cannot resolve the module. The companion conflict-review-file-tree-windowing.test.tsx imports the same deleted module and also still queries the old [data-testid="source-control-virtual-list"].
Technical details
# Dangling `source-control/listing/virtual-file-list` imports after the move
## Affected sites
- `src/renderer/src/components/editor/ConflictReviewFileTree.tsx:12` — imports `SourceControlVirtualFileList` from the removed module and uses it at line 106.
- `src/renderer/src/components/editor/conflict-review-file-tree-windowing.test.tsx:6-9` — imports `SOURCE_CONTROL_FILE_ROW_OVERSCAN` and `SOURCE_CONTROL_VIRTUALIZE_MIN_ROWS` from the removed module.
- `src/renderer/src/components/editor/conflict-review-file-tree-windowing.test.tsx:87,248,327,382,600,682` — query `[data-testid="source-control-virtual-list"]`, which is now `virtualized-list`.
## Required outcome
- `ConflictReviewFileTree` compiles against the shared `VirtualizedList` and its windowing test suite resolves and passes.
- No reference to the removed module path or the old test id remains anywhere in the repo.
## Suggested approach
- In `ConflictReviewFileTree.tsx`, import `{ VirtualizedList }` from `@/components/virtualized-list` and rename the JSX usage.
- In the windowing test, import `{ VIRTUALIZED_LIST_OVERSCAN, VIRTUALIZED_LIST_MIN_ROWS }` from `@/components/virtualized-list` and switch the selector to `[data-testid="virtualized-list"]`.
## Evidence
- `pnpm vitest run src/renderer/src/components/editor/conflict-review-file-tree-windowing.test.tsx` fails at transform with `Failed to resolve import "@/components/right-sidebar/source-control/listing/virtual-file-list"`.ℹ️ Nitpicks
SourceControl.virtual-file-list.test.tsxstill carries the old component name in its filename and describes itself in terms of source control, though it now exercises the sharedVirtualizedList; renaming it would keep the move honest.announceListPositionproduces list semantics only in the virtualized path, so an artifacts list announces a list at 50+ rows and no list below that. The comment calls this out as intentional — worth a quick confirmation that the screen-reader discontinuity is acceptable.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Problem
The artifacts list renders all rows in the DOM regardless of size, degrading performance with large collections. Virtualization logic exists in source control but is tightly coupled and not reusable.
Solution
Extract virtualization into a generic
VirtualizedListcomponent applicable across features. Apply it to artifacts, enabling windowing of large lists. Add support for announcing list position to screen readers and handling paginated lists with "load more" controls.What Changed
ArtifactListRowcomponent to extract individual row rendering and actionsArtifactListRowsto useVirtualizedListinstead of rendering all rowsSourceControlVirtualFileList→VirtualizedListas a reusable component in shared locationSOURCE_CONTROL_*constants to genericVIRTUALIZED_LIST_*equivalentsVirtualizedListwithannounceListPositionandhasUnloadedRowsoptionsWhy
Centralizes list virtualization logic used by multiple features, enabling code reuse. Artifacts list now windows rows like source control, supporting large collections without performance degradation. Adds accessibility support for announcing list position and handling paginated lists with unknown totals.
Testing
Comprehensive new tests in
artifact-list-windowing.test.tsxand updatedvirtualized-list.test.tsxcovering natural flow below threshold, windowing above threshold, scroll margin handling, row identity preservation, selection/deletion in windowed view, pagination without scroll jump, and accessibility announcements.Visual Proof
N/A — refactoring for performance and code reuse; no user-facing behavioral change beyond ability to handle larger lists without lag.
Linked Issue
None
Review
No accessibility tree changes for opt-out callers. Preserves scroll position across threshold crossing. Maintains column alignment and row dividers per design system.
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdChecklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass