[perf] Virtualize the five entity grids#2119
Open
frankrousseau wants to merge 14 commits into
Open
Conversation
PERF-1 pilot: large productions (thousands of edits/shots/assets) freeze the page because Kitsu's datatables render every row's DOM regardless of scroll position. @tanstack/vue-virtual is a headless virtualizer that fits the intricate sticky-header/sticky-column table layout used across the list components, unlike component libraries that own their own DOM. This commit only adds the dependency; EditList.vue is virtualized as the pilot in the next commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PERF-1 pilot on a single grid. EditList renders every edit's <tr>
unconditionally, so productions with thousands of edits freeze the
Edits page. useVirtualizer (added via a setup() hook, the rest of the
component stays Options API) now only mounts rows near the viewport.
Row-positioning technique: real <tr>/<td> spacer rows (top/bottom) sized
to the off-screen rows' total height, rather than absolutely-positioned
rows with a translateY transform. Several columns in this table
(metadata descriptors) have no explicit CSS width and rely on the
browser's native <table> auto-layout across all currently-rendered rows
to stay aligned with the header; a transform-based tbody would force
each row into its own independent `display:table` box, breaking that
shared column-width resolution. Spacer rows keep every rendered row (and
the header) in the same table layout context, so column widths and the
existing sticky header/columns (position: sticky + the offsets computed
in updateOffsets(), which only ever reads header refs) are unaffected.
Row height is content-driven (big-thumbnail toggle, wrapping
descriptions), so rows are dynamically measured via
rowVirtualizer.measureElement, with a rough per-thumbnail-size estimate
only used before a row's first measurement.
`i` (a row's real index into displayedEdits, not its position among the
rendered subset) is threaded through unchanged everywhere the template
already depended on it (isSelected, the editor-i-j/validation-i-j ref
names, the z-index stacking hack), by iterating a `visibleRows` computed
of `{ edit, i }` pairs instead of `displayedEdits` directly.
DOM-reads adapted: entity_list.js's onTaskSelected() rebuilds a
shift-click range-selection rectangle by reading
`this.$refs['validation-i-j']` for every cell in range, which silently
drops any cell outside the rendered window once rows are virtualized.
Overridden locally in EditList.vue (mixin methods lose to a same-named
component method, so ShotList/AssetList keep the original DOM-reading
version) to resolve the same rectangle from
displayedEdits/taskTypeMap/taskMap instead.
Known, not adapted (out of this pilot's contained scope - shared across
ShotList/AssetList/EpisodeList, not just EditList):
- descriptors.js keyMetadataNavigation (Ctrl+Arrow between metadata
inputs) wraps first<->last row via `this.$refs['editor-i-j']`; once
virtualized, wrapping to a far-away unrendered row is a silent no-op.
- selection.js select()/onKeyDown (Alt+Arrow between validation cells)
steps by one row via the same ref pattern; in practice low-risk since
overscan=10 keeps the adjacent row mounted, but not guaranteed at the
rendered window's edge.
Both would need scrollToIndex + a wait-for-mount choreography that
belongs with the later EntityDataGrid extraction, not this single-list
pilot.
No component test added: @tanstack/virtual-core's default viewport
tracking needs ResizeObserver, which jsdom doesn't implement (guarded
with `if (!ResizeObserver) return` in observeElementRect), and jsdom's
getBoundingClientRect always returns zeros. Without either, the
virtualizer's viewport rect stays stuck at its {width:0, height:0}
default, so a test can't exercise "only visible rows render" - it would
either pass trivially regardless of correctness or require mocking the
entire size-observation pipeline. Matches this repo's existing testing
split (vite.config.js: components are exercised through the dev app;
coverage thresholds are scoped to src/lib and src/store only).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The assignee avatar stack sits in a flex-wrap:wrap wrapper, so a task with many assignees wraps to a second line and grows its row. Fine for plain tables, but a virtualized list needs row heights that do not depend on assignation counts. New maxAssignees prop (default 0 = no cap, historical wrapping behavior untouched for ShotList/AssetList/every other consumer): when set, at most that many avatars render, the rest collapses into a "+N" badge whose tooltip lists the hidden names, and the status wrapper switches to a single non-wrapping line (nowrap + overflow hidden + no shrink) so the cell height is constant no matter the status-name length or people count. The badge uses theme variables so it follows dark mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up to the virtualization pilot, from design review: rows should have a fixed height within each display mode instead of relying on per-row measurement, with measureElement kept only as a safety net. Two height-breakers handled: - Contact sheet mode goes through the same virtualized table rows (it is a ComboboxDisplayOptions toggle that EditList forwards to ValidationCell as :contact-sheet, which renders a fixed 150x100 preview wrapper per cell), so it gets its own estimate: ROW_HEIGHT_ESTIMATE_CONTACT_SHEET = 102 (100px wrapper + row border; the validation td has no padding). estimateSize checks bigThumbnails first because in the combined mode the name column's 100px thumbnail plus its 6px cell padding is the taller constraint. - Assignment overflow: EditList now passes max-assignees=3 to both validation-cell usages (opt-in prop added to ValidationCell in the previous commit; other lists keep the wrapping behavior). 3 avatars (23px each) plus the status tag fit the 150px validation cell on one line; anything beyond collapses into a "+N" badge and the cell clips instead of wrapping, so assignation counts can no longer change a row's height. Estimates per mode: 52 normal, 116 big thumbnails (also when combined with contact sheet), 102 contact sheet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The zebra striping comes from global `.datatable-row:nth-child(even)` rules in App.vue. With rows virtualized, the DOM only holds a window of rows behind a spacer row, so nth-child parity re-anchors on whichever row is rendered first: every visible stripe flips each time the scroll window advances by one row, which reads as full-table flicker. Bind stripe-even/stripe-odd classes from the row's data index instead (i odd = --background-alt, matching what nth-child produced when all rows were in the DOM) and override the nth-child rules in scoped CSS, with hover redeclared after the stripes so it still wins. The only nth-child cosmetic left untouched is the corner-radius / last-row special-casing, which can be off by one row at the very bottom of the list - invisible in practice and not worth a deeper override. Also qualify the spacer-row reset with .datatable-body: it previously tied with shared.scss's `.data-list .datatable-body td` padding rule (same specificity), leaving the winner to injection order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Port of the EditList pilot to the grouped assets grid, targeting the
5000-asset stress production. AssetList rendered one tbody per asset
type (type-header row + all asset rows), freezing the page on large
productions.
Flattening design: the grouped tbodys are linearized in setup() into one
flat items array mixing type-header entries ({ isHeader, group, k }) and
asset entries ({ asset, i, k }), in display order. A single virtualizer
windows over that flat list inside ONE tbody, between top/bottom spacer
rows (same table-auto-layout rationale as the pilot: several columns
have no explicit width and rely on shared column-width resolution, and
the sticky header/columns only read thead refs). Type-header rows keep
their exact markup and classes. filteredDisplayedAssets moved from the
Options computed block into setup() because the flattening derives from
it; logic unchanged.
`i` and `k` keep their historical meaning (index inside the filtered
group / group index), so every getIndex(i, k)-based selection
coordinate, ref name, and MetadataInput index is unchanged. getItemKey
uses asset.id for rows and `header-<asset_type_id>` for headers.
Estimates per mode: 52 normal, 116 big thumbnails, 102 contact sheet,
56 type-header rows; measureElement stays as safety net, overscan 10.
Assignee avatars capped via :max-assignees="3" (ValidationCell opt-in
prop) so assignation counts cannot change row heights.
Zebra striping switched from the global nth-child rules to data-driven
stripe-even/stripe-odd classes on the group-local index, matching the
parity the per-tbody nth-child rules produced (header at child 1, asset
i at child i+2, multi-section odd = alt => i odd = alt); without this,
stripes flip on every one-row window shift. Hover redeclared after.
All-rows assumptions found:
- entity_list.js onTaskSelected() shift-rectangle reads
$refs['validation-x-y'] per cell: overridden locally (same duplicate
as EditList; consolidating both copies into the shared mixin is
ARCH-4's job). The override resolves the rectangle from a
rowIndexToAsset map built in getIndex coordinates (offsets from the
unfiltered groups, i within the filtered group - exactly what the
rendered cells advertise), and mirrors template selectability: sticked
cells always selectable, non-sticked via isSelectable() (workflow +
shared-asset checks).
- toggleLine checkbox shift-select, onSelectColumn (select-all from the
header menu, reads assetStore.cache.result), inWorkflowFilledColumns,
AssetListNumbers per-type stats (fed assetCache.result), and
selectTaskFromQuery are all data-driven already: untouched.
- updateOffsets/nameResizeObserver only read thead refs: untouched.
- selection.js Alt+Arrow select(i+-1) steps rows via refs: adjacent rows
stay mounted thanks to overscan 10; silent no-op at the window edge
(shared mixin, deferred to ARCH-4 like the pilot).
- descriptors.js keyMetadataNavigation: AssetList never had the
editor-i-j row refs it looks up, so Ctrl+Arrow from the resolution
input already threw before virtualization; pre-existing, unchanged.
Known cosmetic leftovers (same as pilot): multi-section nth-child(2) /
last-child corner-radius rules can hit a mid-list row at the window
edge under the sticky header; not worth a deeper override.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The progressive-display mechanism (PAGE_SIZE=200 initial slice + displayMore* growing it on scroll-to-bottom) existed to keep the rendered DOM small. With AssetList/EditList rows virtualized it only throttled the virtualizer: the scrollbar reflected 200 rows and kept growing, with a load-more hitch every page - measured on Assets with 1120 assets, the list felt non-virtualized. Where the cap lived and what changed: - assets.js LOAD_ASSETS_END and helpers.buildResult (search/filter/sort path): displayedAssets was cache/result.slice(0, PAGE_SIZE or current length); now the full result. edits.js LOAD_EDITS_END, helpers.buildResult and NEW_EDIT_END: same change. - The lists still receive a copy (.slice()) of the cache arrays, never the array itself: mutations like NEW_ASSET_END push into both cache.result AND displayedAssets and rely on them being distinct (aliasing them would display freshly created entities twice). - AssetList/EditList local onBodyScroll overrides (scroll emit + scroll-to-bottom -> displayMore dispatch) removed; the entityListMixin version (scroll-position emit only) takes over, so scroll-position persistence on the pages is unchanged. - DISPLAY_MORE_ASSETS / DISPLAY_MORE_EDITS mutations kept: their own length guard makes them no-ops now. Breakdown and Playlist still dispatch displayMoreAssets from their non-virtualized side panels; those calls become harmless. Deliberately NOT touched: the shots/sequences/episodes modules and lib/pagination - their lists are not virtualized yet. Consequence to watch (live pass): Breakdown's available-assets panel (assetsByType) and Playlist's addition panel (displayedAssetsByType) read the same displayedAssets state and are NOT virtualized, so they now render every asset at once. Rows there are much lighter than the grid and both panels are search-filtered, but on very large productions they lose the paging cushion; if that measurably regresses, the follow-up is local pagination (or virtualization) in those two panels, not restoring the store cap. Header counts were already total: setListStats always received the full result, so displayedAssetsLength/Count semantics are unchanged. No store spec updates needed: tests/unit/store/assets.spec.js has no page-size assertions (and no batched-deletion describe exists on this branch); the suite stays at its 1032-passed baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live testing showed scrolling noticeably smoother in Chrome than Firefox on the virtualized Assets/Edits grids. Firefox's scroll anchoring fights windowed updates: each time the scroll window shifts, the top spacer row resizes and the browser re-anchors the scroll position against it, producing micro-jumps. overflow-anchor: none on the scroll container is the standard TanStack Virtual mitigation (Chrome ignores anchoring inside these updates anyway, so it is a no-op there). The wrapper element lives inside each list component (the div.datatable-wrapper carrying ref="body"), so a scoped style rule is the narrowest possible scoping: it compiles to .datatable-wrapper[data-v-...] and cannot affect the global .datatable-wrapper rules that non-virtualized tables use. AssetList already had a scoped wrapper rule (min-height/flex) - extended it; EditList gets a new one. Second Firefox factor identified but deliberately NOT acted on here: every window shift re-runs table auto-layout across the rendered rows, a direct consequence of the spacer-row + shared-auto-layout design the pilot chose to keep unsized columns aligned with the header. If Firefox still lags after the anchoring fix, the escalation path is explicit column sizing + table-layout: fixed, which belongs to the ARCH-4 EntityDataGrid generalization, not this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Port of the established recipe (EditList pilot -> AssetList) to the
largest grid. ShotList rendered one tbody per sequence (header row +
all shot rows), freezing the page on large productions.
Flattening: the per-sequence tbodys are linearized in setup() into one
flat items array mixing sequence-header entries ({ isHeader, group, k,
key: header-<sequence_id> }) and shot entries ({ shot, i, k, key:
shot.id }), windowed by a single virtualizer inside ONE tbody between
top/bottom spacer rows (same shared-auto-layout rationale as the
pilot). Sequence-header rows keep their exact markup/classes. Unlike
AssetList there is no local display-settings filtering layer: the
flattening derives straight from the displayedShots prop, so TV-show
episode scoping (applied upstream in the shots store when the route
changes episode) flows through unchanged.
`i`/`k` keep their historical meaning (index in group / group index):
getIndex(i, k) selection coordinates, validation-x-y and editor-x-y ref
names, and MetadataInput indexes are unchanged. Estimates per mode: 52
normal, 116 big thumbnails, 102 contact sheet, 56 sequence headers;
measureElement safety net, overscan 10. Assignee avatars capped via
:max-assignees="3" on both validation-cell usages. Data-driven
stripe-even/stripe-odd classes on the group-local index replace the
nth-child zebra (same parity mapping as AssetList), and the wrapper
gets scoped overflow-anchor: none (Firefox anchoring mitigation).
No v-memo existed anywhere in the template (checked per the PERF-10
note) - nothing to drop.
All-rows assumptions found:
- entity_list.js onTaskSelected() shift-rectangle reads
$refs['validation-x-y'] per cell: overridden locally like
EditList/AssetList (consolidation into the mixin is the next planned
step). ShotList never binds :selectable on cells, so every column in
range is selectable, matching the rendered default.
- toggleLine checkbox shift-select (displayedShots.flat()),
onSelectColumn (shotStore.cache.result), the frame/drawing/time
footer counts (displayedShots* store getters computed from the full
result in setListStats), isEmptyList, selectTaskFromQuery and
Shots.vue's setScrollPosition: all data-driven already, untouched.
- updateOffsets reads thead refs only: untouched.
- Ctrl+Arrow keyMetadataNavigation: ShotList DOES have editor-x-y row
refs (unlike AssetList), so single-step navigation keeps working
thanks to overscan 10; the first<->last wrap-around to a far
unrendered row breaks (ref lookup throws) - shared mixin, deferred to
ARCH-4 exactly like the pilot documented for EditList.
- Alt+Arrow selection.js select() steps one row via refs: adjacent rows
stay mounted (overscan); silent no-op at the window edge (deferred,
shared mixin).
- Drag-to-pan startBrowsing moved from per-sequence tbodys to the
single tbody; document-level move/up handlers unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same inversion as assets/edits (007408cd6): the PAGE_SIZE progressive display only throttled the virtualized ShotList - short growing scrollbar and a display-more hitch every 200 rows. Where the cap lived and what changed in shots.js: - LOAD_SHOTS_END, helpers.buildResult (search/filter/sort path) and NEW_SHOT_END sliced displayedShots to PAGE_SIZE; they now hold the full result. Still a .slice() copy: the cache arrays are mutated in place elsewhere and mutations rely on displayedShots being distinct. - DISPLAY_MORE_SHOTS kept as a self-guarded no-op for external callers. - ShotList's local onBodyScroll (scroll emit + scroll-to-bottom -> displayMoreShots) removed; the entityListMixin version (scroll emit only) takes over, so Shots.vue scroll-position persistence is unchanged. Consumer inventory of state.displayedShots before removing the cap: - ShotList (virtualized) - the target. - Playlist.vue: non-virtualized addition panel iterating displayedShotsBySequence, plus its own scroll-more wiring dispatching displayMoreShots (now a harmless no-op). Same exposure as the assets change: the panel loses its paging cushion on huge productions; same follow-up path if it measurably regresses (paginate/virtualize that panel locally, not restore the store cap). - Task.vue and Breakdown.vue map the displayedShots getter but never use it (dead mapGetters entries) - unaffected. - ManageShotsModal has an unrelated local ref that shadows the name - unaffected. displayedShotsLength/Count/Frames/Drawings/TimeSpent/Estimation were already computed from the full result (setListStats receives result/cache.shots in every path), so header and footer totals are unchanged. No shots store spec exists, and no spec asserts paging. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The virtualization work left three near-identical local onTaskSelected overrides (EditList, AssetList, ShotList), each resolving the shift-click rectangle from data because the mixin's version read `this.$refs['validation-x-y']` for every cell - which silently drops cells outside the rendered window once rows are virtualized. Consumer check before promoting: among entityListMixin users (the 5 entity grids + SequenceStatsList / EpisodeStatsList / ProductionAssetTypeList / ShotLine / Breakdown), only the 5 entity grids reference onTaskSelected at all; the pages' onTaskSelected(task) handlers (Shot/Asset/Episode/Sequence/TaskType) are unrelated single-argument methods from entity.js or local. So no $refs-dependent consumer remains and the data-driven rectangle moves INTO the mixin. The mixin now resolves the rectangle from two per-list hooks: - entityForRow(lineIndex): global row index -> entity, in the exact coordinates the list's cells advertise as row-x. One-liners for the flat lists (displayedEdits/Sequences/Episodes[i]), Map lookups for the grouped ones (rowIndexToAsset / rowIndexToShot, which encode the getIndex(i, k) offsets). Default returns undefined (mixin consumers that never wire cell selection keep empty ranges). - isCellSelectable(entity, columnId, columnIndex): default true (ValidationCell's selectable default); AssetList overrides it to mirror its template (sticked always selectable, non-sticked through isSelectable()'s workflow + shared-asset checks). allDisplayedValidationColumns also moves into the mixin (was duplicated per list). The local overrides in EditList/AssetList/ShotList are deleted; SequenceList/EpisodeList (still full-DOM until their ports) gain their entityForRow one-liners so behavior is identical - for non-virtualized lists the data-driven rectangle selects exactly the same cells the $refs walk did. Deliberately NOT the ARCH-4 EntityDataGrid extraction: this only dedups the selection logic; templates, virtualizers and column plumbing stay per-list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mechanical port of the established recipe: SequenceList is a flat list (single tbody, no group headers), so the virtualizer windows directly over displayedSequences with sequence.id keys - no flattening layer needed. Spacer rows, per-mode estimates (52 normal / 116 big thumbnails / 102 contact sheet), measureElement safety net, overscan 10, :max-assignees="3" on both validation cells, data-driven stripe-even/stripe-odd on the global index (plain .datatable-row nth-child parity, like EditList), scoped overflow-anchor: none. All-rows assumptions: none beyond what the consolidated entity_list.js onTaskSelected already handles (entityForRow hook was added in the previous commit). No local onBodyScroll or display-more wiring existed (the sequences store never paged displayedSequences), no row checkboxes, footer stats come from store getters. Pre-existing and untouched: the resolution input's Ctrl+keyup handler references getIndex(i, k), which never existed in this component (throws when triggered today, before and after this change - flat lists have no getIndex); flagged for a later cleanup pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mechanical port of the established recipe: EpisodeList is a flat list (single tbody, no group headers), so the virtualizer windows directly over displayedEpisodes with episode.id keys. Spacer rows, per-mode estimates (52 normal / 116 big thumbnails / 102 contact sheet), measureElement safety net, overscan 10, :max-assignees="3" on both validation cells, data-driven stripe-even/stripe-odd on the global index, scoped overflow-anchor: none on the wrapper. All-rows assumptions: none beyond what the consolidated entity_list.js onTaskSelected already handles (entityForRow hook added in the consolidation commit). No local onBodyScroll or display-more wiring existed (the episodes store never paged displayedEpisodes), no row checkboxes, footer stats come from store getters, the per-row status combobox emits field-changed with the row's own entity. Pre-existing and untouched: the resolution input's Ctrl+keyup handler references getIndex(i, k), which never existed in this component (throws when triggered, before and after this change); flagged for a later cleanup pass together with SequenceList's identical copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live regression: the Ready For column rendered 81px wide instead of
180px. Under row virtualization, table auto-layout computes column
widths from the ~25 rendered rows only; `width`/`max-width` are just
preferences the algorithm may shrink, so columns whose real width used
to be defended by row content across the full list collapse. min-width
is the only binding constraint.
Audit of all five virtualized lists - every thead column checked for a
binding min-width anchored in an always-rendered cell:
Constrained by this commit:
- AssetList th/td.ready-for: min-width 180px added (had width/max-width
180px only - the reported regression).
- AssetList and SequenceList resolution: the existing 110px floor was
declared on td only, which stops binding when no row is in the
window; the rule now covers th.resolution as well (thead is never
virtualized). Same 110px value.
Verified already bound, no change needed:
- Metadata/descriptor columns (the pilot's known width-less
td.metadata-descriptor): anchored by MetadataHeader's own
`th.metadata-descriptor { min-width: 120px }` - the thead is outside
the window, so the floor survives virtualization in all five lists.
- Validation columns: ValidationHeader's root th carries
.validation-cell, so each list's scoped 150px min-width binds in the
thead (plus ValidationHeader's own 120px floor); minimized columns
have the 30px hidden-validation-cell floors.
- Name (110-200px), episode (80/100px), description (200px),
time-spent/estimation (60/70px), status (120px, EpisodeList),
frames/drawings (80px), framein/frameout (60px), fps (70px),
max-retakes (80px), actions (160px): all already min-width-bound in
the lists' scoped styles.
No table-layout: fixed - explicit column sizing is the ARCH-4
escalation, out of scope here.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.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.
Problems
$refs.Solutions
@tanstack/vue-virtualwindows the rows of ShotList/AssetList/EditList/SequenceList/EpisodeList: spacer rows (preserves table auto-layout + sticky headers), fixed per-display-mode height estimates with measured fallback, stable keys,overscan: 10.DISPLAY_MORE_*kept as no-ops for external callers).maxAssigneescap (+N badge) so rows keep fixed heights; stripes computed from data order;min-widthbound on collapse-prone columns;overflow-anchor: nonescoped to the virtualized wrappers.entity_list.jsas one data-driven implementation (−220 duplicated lines).