refactor(virtual-grid): migrate to @tanstack/vue-virtual (FE-535)#11885
Open
dante01yoon wants to merge 4 commits intomainfrom
Open
refactor(virtual-grid): migrate to @tanstack/vue-virtual (FE-535)#11885dante01yoon wants to merge 4 commits intomainfrom
dante01yoon wants to merge 4 commits intomainfrom
Conversation
Reproduce the dropdown-blank-on-scroll symptom at the VirtualGrid layer. When scrollY drifts past the natural scroll maximum (items list shrinks while popover scroll position is retained, or rubberband over-scroll captured by useScroll), the naive offsetRows derivation collapses state.start === state.end === items.length and renderedItems becomes empty even though items > 0. Users then see a blank dropdown that only recovers when another scroll event fires. These two cases stay red on main and turn green once VirtualGrid clamps offsetRows to the maximum that keeps the last page visible.
When scrollY drifts past the natural scroll maximum (items shrink while the popover's scroll position is retained, or macOS rubberband over-scroll briefly outruns the limit), the naive `floor(scrollY / itemHeight)` makes state.start and state.end both clamp to items.length and renderedItems becomes empty. Vue then renders zero rows even though items > 0, and FormDropdownMenu does not show its empty-state icon either since the items array itself is non-empty. The user sees a blank dropdown until another scroll event re-syncs the position, which can take a long pause if no further interaction happens. Clamp offsetRows to max(0, totalRows - viewRows) so the grid always falls back to rendering the last page of items in this state. Once a real scroll event fires, normal windowing resumes from a valid position. Fixes FE-535
dosubot
Bot
added
the
size:S
Contributor
📝 WalkthroughWalkthroughReplaced custom scroll-based windowing in VirtualGrid with TanStack's useVirtualizer, updating props/emit behavior and rendering to virtual rows with absolute positioning. Tests were refactored to mock useVirtualizer via a new mockedVisibleEnd and include FE-535 cases validating end-of-list and approach-end behavior. ChangesVirtualGrid virtualization migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ❌ 1463 passed, 1 failed · 1 flaky❌ Failed Tests📊 Browser Reports
|
📦 Bundle: 5.26 MB gzip 🟢 -399 BDetailsSummary
Category Glance App Entry Points — 22.6 kB (baseline 22.6 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.24 MB (baseline 1.24 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 81.8 kB (baseline 81.8 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed / 2 unchanged Panels & Settings — 489 kB (baseline 489 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed / 11 unchanged User & Accounts — 17.5 kB (baseline 17.5 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed / 2 unchanged Editors & Dialogs — 112 kB (baseline 112 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 4 added / 4 removed UI Components — 62.9 kB (baseline 62.9 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed / 9 unchanged Data & Services — 3.05 MB (baseline 3.05 MB) • 🟢 -1.1 kBStores, services, APIs, and repositories
Status: 13 added / 13 removed / 4 unchanged Utilities & Hooks — 365 kB (baseline 365 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 13 added / 13 removed / 18 unchanged Vendor & Third-Party — 9.94 MB (baseline 9.94 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BBundles that do not match a named category
Status: 57 added / 57 removed / 78 unchanged ⚡ Performance Report
No regressions detected. All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-05-04T11:05:16.413Z",
"gitSha": "840727954e6479062dcc1413bff9d37cf419b2e4",
"branch": "jaewon/fe-535-virtual-grid-blank-on-scroll",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2010.8470000000125,
"styleRecalcs": 8,
"styleRecalcDurationMs": 8.275999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 326.43199999999996,
"heapDeltaBytes": 22830492,
"heapUsedBytes": 72250460,
"domNodes": 16,
"jsHeapTotalBytes": 14942208,
"scriptDurationMs": 14.601000000000003,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-idle",
"durationMs": 2016.8710000000374,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.12,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 335.04200000000003,
"heapDeltaBytes": 22967336,
"heapUsedBytes": 71737024,
"domNodes": 18,
"jsHeapTotalBytes": 14417920,
"scriptDurationMs": 15.056,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2033.0250000001797,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.185000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 336.20399999999995,
"heapDeltaBytes": 22925800,
"heapUsedBytes": 71673836,
"domNodes": 16,
"jsHeapTotalBytes": 14942208,
"scriptDurationMs": 14.697999999999999,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1815.979999999854,
"styleRecalcs": 71,
"styleRecalcDurationMs": 33.907,
"layouts": 12,
"layoutDurationMs": 3.224,
"taskDurationMs": 779.337,
"heapDeltaBytes": -2068724,
"heapUsedBytes": 46648188,
"domNodes": -264,
"jsHeapTotalBytes": 15593472,
"scriptDurationMs": 122.215,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1844.3620000000465,
"styleRecalcs": 76,
"styleRecalcDurationMs": 44.28099999999999,
"layouts": 12,
"layoutDurationMs": 4.022,
"taskDurationMs": 775.1400000000001,
"heapDeltaBytes": -3547200,
"heapUsedBytes": 62348440,
"domNodes": 58,
"jsHeapTotalBytes": 19755008,
"scriptDurationMs": 128.749,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2002.8959999999643,
"styleRecalcs": 80,
"styleRecalcDurationMs": 42.792,
"layouts": 12,
"layoutDurationMs": 3.91,
"taskDurationMs": 919.1,
"heapDeltaBytes": 18887204,
"heapUsedBytes": 68230004,
"domNodes": 62,
"jsHeapTotalBytes": 15728640,
"scriptDurationMs": 118.186,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1731.443999999783,
"styleRecalcs": 30,
"styleRecalcDurationMs": 17.185000000000002,
"layouts": 6,
"layoutDurationMs": 0.7009999999999998,
"taskDurationMs": 311.87399999999997,
"heapDeltaBytes": 164796,
"heapUsedBytes": 48524900,
"domNodes": 75,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 21.544000000000008,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1728.5460000000512,
"styleRecalcs": 31,
"styleRecalcDurationMs": 17.808999999999997,
"layouts": 6,
"layoutDurationMs": 0.737,
"taskDurationMs": 295.87,
"heapDeltaBytes": 253768,
"heapUsedBytes": 47877880,
"domNodes": 80,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 27.723,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1719.9839999998403,
"styleRecalcs": 32,
"styleRecalcDurationMs": 17.521,
"layouts": 6,
"layoutDurationMs": 0.65,
"taskDurationMs": 282.192,
"heapDeltaBytes": 115040,
"heapUsedBytes": 48855756,
"domNodes": 79,
"jsHeapTotalBytes": 14942208,
"scriptDurationMs": 17.695000000000004,
"eventListeners": 21,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 491.8870000001334,
"styleRecalcs": 9,
"styleRecalcDurationMs": 5.958999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 306.76099999999997,
"heapDeltaBytes": 8844960,
"heapUsedBytes": 57417828,
"domNodes": 14,
"jsHeapTotalBytes": 15728640,
"scriptDurationMs": 53.12299999999999,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 594.1309999998339,
"styleRecalcs": 13,
"styleRecalcDurationMs": 9.045000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 360.761,
"heapDeltaBytes": 9798396,
"heapUsedBytes": 57409060,
"domNodes": 22,
"jsHeapTotalBytes": 15466496,
"scriptDurationMs": 65.50399999999999,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "dom-widget-clipping",
"durationMs": 532.1570000000975,
"styleRecalcs": 11,
"styleRecalcDurationMs": 6.905999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 312.933,
"heapDeltaBytes": 9114736,
"heapUsedBytes": 57443920,
"domNodes": 17,
"jsHeapTotalBytes": 15990784,
"scriptDurationMs": 54.754999999999995,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2021.4710000000196,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.919999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 517.8439999999999,
"heapDeltaBytes": 4284304,
"heapUsedBytes": 62751016,
"domNodes": -262,
"jsHeapTotalBytes": 552960,
"scriptDurationMs": 76.92399999999999,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "large-graph-idle",
"durationMs": 2008.5340000000542,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.615,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 517.232,
"heapDeltaBytes": 2961812,
"heapUsedBytes": 59862028,
"domNodes": -262,
"jsHeapTotalBytes": 5533696,
"scriptDurationMs": 79.78200000000002,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2017.7619999999479,
"styleRecalcs": 10,
"styleRecalcDurationMs": 8.258000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 522.654,
"heapDeltaBytes": 9680268,
"heapUsedBytes": 68277804,
"domNodes": -262,
"jsHeapTotalBytes": 28672,
"scriptDurationMs": 78.688,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2097.328999999945,
"styleRecalcs": 67,
"styleRecalcDurationMs": 18.427,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1092.7720000000002,
"heapDeltaBytes": 9027424,
"heapUsedBytes": 68802040,
"domNodes": -259,
"jsHeapTotalBytes": 2854912,
"scriptDurationMs": 412.441,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2102.0750000000135,
"styleRecalcs": 69,
"styleRecalcDurationMs": 19.185000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1025.08,
"heapDeltaBytes": 10286744,
"heapUsedBytes": 68950072,
"domNodes": -262,
"jsHeapTotalBytes": 5214208,
"scriptDurationMs": 366.457,
"eventListeners": -159,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2128.3089999999447,
"styleRecalcs": 68,
"styleRecalcDurationMs": 18.386,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1102.79,
"heapDeltaBytes": 3902148,
"heapUsedBytes": 63348840,
"domNodes": -267,
"jsHeapTotalBytes": 2330624,
"scriptDurationMs": 437.875,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3133.9210000001003,
"styleRecalcs": 65,
"styleRecalcDurationMs": 19.214000000000002,
"layouts": 60,
"layoutDurationMs": 8.362000000000002,
"taskDurationMs": 1290.4979999999998,
"heapDeltaBytes": 7946356,
"heapUsedBytes": 69228508,
"domNodes": -269,
"jsHeapTotalBytes": 4485120,
"scriptDurationMs": 469.86000000000007,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3134.405999999899,
"styleRecalcs": 64,
"styleRecalcDurationMs": 18.158,
"layouts": 60,
"layoutDurationMs": 8.016,
"taskDurationMs": 1279.2250000000001,
"heapDeltaBytes": 11963984,
"heapUsedBytes": 72713812,
"domNodes": -270,
"jsHeapTotalBytes": -233472,
"scriptDurationMs": 466.701,
"eventListeners": -125,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3148.290000000088,
"styleRecalcs": 65,
"styleRecalcDurationMs": 18.322999999999997,
"layouts": 60,
"layoutDurationMs": 8.232999999999999,
"taskDurationMs": 1335.21,
"heapDeltaBytes": 12982784,
"heapUsedBytes": 74013252,
"domNodes": -268,
"jsHeapTotalBytes": 815104,
"scriptDurationMs": 511.51299999999986,
"eventListeners": -125,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2021.6239999999743,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.213000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 558.997,
"heapDeltaBytes": 8306464,
"heapUsedBytes": 68573116,
"domNodes": -264,
"jsHeapTotalBytes": -1019904,
"scriptDurationMs": 90.766,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "minimap-idle",
"durationMs": 2028.150000000096,
"styleRecalcs": 7,
"styleRecalcDurationMs": 5.885999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 497.641,
"heapDeltaBytes": 3674888,
"heapUsedBytes": 63892076,
"domNodes": -266,
"jsHeapTotalBytes": 28672,
"scriptDurationMs": 75.039,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2033.3679999998822,
"styleRecalcs": 7,
"styleRecalcDurationMs": 6.072000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 499.64099999999996,
"heapDeltaBytes": 9789044,
"heapUsedBytes": 70182396,
"domNodes": -266,
"jsHeapTotalBytes": -233472,
"scriptDurationMs": 75.46,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 587.0549999999639,
"styleRecalcs": 46,
"styleRecalcDurationMs": 11.252999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 370.606,
"heapDeltaBytes": 9110856,
"heapUsedBytes": 57773736,
"domNodes": 18,
"jsHeapTotalBytes": 16252928,
"scriptDurationMs": 134.41500000000002,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 571.9100000001163,
"styleRecalcs": 46,
"styleRecalcDurationMs": 10.237,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 349.243,
"heapDeltaBytes": 9123052,
"heapUsedBytes": 57757452,
"domNodes": 18,
"jsHeapTotalBytes": 15990784,
"scriptDurationMs": 125.493,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 538.0669999999554,
"styleRecalcs": 46,
"styleRecalcDurationMs": 9.995000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 335.241,
"heapDeltaBytes": 9264884,
"heapUsedBytes": 57829564,
"domNodes": 18,
"jsHeapTotalBytes": 15728640,
"scriptDurationMs": 116.59999999999998,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "subgraph-idle",
"durationMs": 1988.7099999998554,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.082,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 385.75800000000004,
"heapDeltaBytes": 22241132,
"heapUsedBytes": 71902364,
"domNodes": 18,
"jsHeapTotalBytes": 14942208,
"scriptDurationMs": 14.167,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2005.3159999999934,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.925,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 335.946,
"heapDeltaBytes": 23215688,
"heapUsedBytes": 71852340,
"domNodes": 18,
"jsHeapTotalBytes": 15728640,
"scriptDurationMs": 13.395000000000001,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2003.6239999999452,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.023,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 322.379,
"heapDeltaBytes": 22507556,
"heapUsedBytes": 71190712,
"domNodes": 18,
"jsHeapTotalBytes": 14680064,
"scriptDurationMs": 12.786,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1725.945000000138,
"styleRecalcs": 77,
"styleRecalcDurationMs": 40.8,
"layouts": 16,
"layoutDurationMs": 4.755,
"taskDurationMs": 671.0290000000001,
"heapDeltaBytes": 14855448,
"heapUsedBytes": 62968556,
"domNodes": 65,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 95.367,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1677.9059999998935,
"styleRecalcs": 75,
"styleRecalcDurationMs": 40.366,
"layouts": 16,
"layoutDurationMs": 5.069000000000001,
"taskDurationMs": 698.778,
"heapDeltaBytes": 14042452,
"heapUsedBytes": 62706204,
"domNodes": 60,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 97.938,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1686.1039999998866,
"styleRecalcs": 74,
"styleRecalcDurationMs": 36.33,
"layouts": 16,
"layoutDurationMs": 7.110000000000001,
"taskDurationMs": 682.8349999999999,
"heapDeltaBytes": -1730840,
"heapUsedBytes": 46900360,
"domNodes": -264,
"jsHeapTotalBytes": 15593472,
"scriptDurationMs": 87.27400000000002,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8144.048000000112,
"styleRecalcs": 250,
"styleRecalcDurationMs": 59.022999999999996,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3607.417,
"heapDeltaBytes": 10562892,
"heapUsedBytes": 69003888,
"domNodes": -263,
"jsHeapTotalBytes": 6524928,
"scriptDurationMs": 1190.064,
"eventListeners": -113,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8152.800999999954,
"styleRecalcs": 250,
"styleRecalcDurationMs": 56.582,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3425.198,
"heapDeltaBytes": 19838344,
"heapUsedBytes": 79234220,
"domNodes": -262,
"jsHeapTotalBytes": 6029312,
"scriptDurationMs": 1131.51,
"eventListeners": -113,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333338,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8140.1599999999235,
"styleRecalcs": 250,
"styleRecalcDurationMs": 58.137,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3441.5330000000004,
"heapDeltaBytes": 9922912,
"heapUsedBytes": 67034520,
"domNodes": -259,
"jsHeapTotalBytes": 6262784,
"scriptDurationMs": 1146.304,
"eventListeners": -113,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 13516.605000000027,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13500.02,
"heapDeltaBytes": -43029236,
"heapUsedBytes": 160343196,
"domNodes": -8331,
"jsHeapTotalBytes": 26013696,
"scriptDurationMs": 594.882,
"eventListeners": -16460,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 12685.647000000017,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12674.314000000002,
"heapDeltaBytes": -32031812,
"heapUsedBytes": 171878532,
"domNodes": -8331,
"jsHeapTotalBytes": 24965120,
"scriptDurationMs": 569.175,
"eventListeners": -16466,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.219999999999953,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 12975.12299999994,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12936.989999999998,
"heapDeltaBytes": -34757212,
"heapUsedBytes": 171695548,
"domNodes": -8331,
"jsHeapTotalBytes": 24440832,
"scriptDurationMs": 609.366,
"eventListeners": -16462,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.219999999999953,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-pan",
"durationMs": 15801.235000000133,
"styleRecalcs": 84,
"styleRecalcDurationMs": 19.411000000000012,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15776.167000000001,
"heapDeltaBytes": -36869456,
"heapUsedBytes": 159580956,
"domNodes": -8329,
"jsHeapTotalBytes": -2035712,
"scriptDurationMs": 873.0849999999999,
"eventListeners": -16488,
"totalBlockingTimeMs": 51,
"frameDurationMs": 17.223333333333237,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 15432.823999999982,
"styleRecalcs": 76,
"styleRecalcDurationMs": 18.093999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15410.740999999998,
"heapDeltaBytes": -44234128,
"heapUsedBytes": 154393256,
"domNodes": -8329,
"jsHeapTotalBytes": -4657152,
"scriptDurationMs": 837.747,
"eventListeners": -16490,
"totalBlockingTimeMs": 8,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 15470.589000000018,
"styleRecalcs": 76,
"styleRecalcDurationMs": 18.502000000000017,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15430.011999999997,
"heapDeltaBytes": -58944580,
"heapUsedBytes": 153459432,
"domNodes": -8331,
"jsHeapTotalBytes": -2560000,
"scriptDurationMs": 898.544,
"eventListeners": -16490,
"totalBlockingTimeMs": 54,
"frameDurationMs": 17.216666666666665,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 476.52900000002774,
"styleRecalcs": 26,
"styleRecalcDurationMs": 30.331000000000003,
"layouts": 5,
"layoutDurationMs": 1.766,
"taskDurationMs": 143.22199999999998,
"heapDeltaBytes": 5442564,
"heapUsedBytes": 55285040,
"domNodes": 196,
"jsHeapTotalBytes": 0,
"scriptDurationMs": 25.21,
"eventListeners": 69,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "workflow-execution",
"durationMs": 460.02199999998084,
"styleRecalcs": 18,
"styleRecalcDurationMs": 25.521,
"layouts": 5,
"layoutDurationMs": 1.4329999999999998,
"taskDurationMs": 118.698,
"heapDeltaBytes": 5014312,
"heapUsedBytes": 54958208,
"domNodes": 157,
"jsHeapTotalBytes": 524288,
"scriptDurationMs": 25.014000000000003,
"eventListeners": 69,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.799999999999727
},
{
"name": "workflow-execution",
"durationMs": 456.26599999991413,
"styleRecalcs": 15,
"styleRecalcDurationMs": 24.676999999999996,
"layouts": 5,
"layoutDurationMs": 1.6580000000000001,
"taskDurationMs": 125.83899999999998,
"heapDeltaBytes": 5023864,
"heapUsedBytes": 54996684,
"domNodes": 155,
"jsHeapTotalBytes": 524288,
"scriptDurationMs": 26.403,
"eventListeners": 69,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999727
}
]
} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/common/VirtualGrid.test.ts`:
- Around line 271-298: The test "keeps last page visible when items shrink below
current scroll (FE-535)" never actually shrinks the list; after mounting
VirtualGrid with items and a high mockedScrollY, update the items array to a
smaller set (reuse the items variable, e.g. replace with createItems(n) where n
< original count) after the initial render, trigger a reactive update (await
nextTick() or call the component's props update mechanism) and then assert that
renderedItems (queryAllByText(/^Item \d+$/)) still has length > 0; locate the
mounts and assertions around the items variable, mockedScrollY, VirtualGrid
render call and the final expect to implement this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f7a1a1b0-3f67-4377-96cb-1681abe2ac68
📒 Files selected for processing (2)
src/components/common/VirtualGrid.test.tssrc/components/common/VirtualGrid.vue
Comment on lines
+271
to
+298
| it('keeps last page visible when items shrink below current scroll (FE-535)', async () => { | ||
| const items = createItems(8) | ||
| mockedWidth.value = 400 | ||
| mockedHeight.value = 240 | ||
| mockedScrollY.value = 3000 | ||
|
|
||
| render(VirtualGrid, { | ||
| props: { | ||
| items, | ||
| gridStyle: defaultGridStyle, | ||
| defaultItemHeight: 100, | ||
| defaultItemWidth: 100, | ||
| maxColumns: 4, | ||
| bufferRows: 1 | ||
| }, | ||
| slots: { | ||
| item: `<template #item="{ item }"> | ||
| <div class="test-item">{{ item.name }}</div> | ||
| </template>` | ||
| }, | ||
| container: document.body.appendChild(document.createElement('div')) | ||
| }) | ||
|
|
||
| await nextTick() | ||
|
|
||
| const renderedItems = screen.queryAllByText(/^Item \d+$/) | ||
| expect(renderedItems.length).toBeGreaterThan(0) | ||
| }) |
Contributor
There was a problem hiding this comment.
Line 271 test name says “items shrink”, but the test never shrinks items.
Right now it mounts once with a small list and high scrollY, which mostly
duplicates the previous case. To cover the retained-scroll + list-shrink path,
update items after initial render and assert rendering still remains non-blank.
🔧 Proposed test adjustment
it('keeps last page visible when items shrink below current scroll (FE-535)', async () => {
- const items = createItems(8)
+ const initialItems = createItems(40)
mockedWidth.value = 400
mockedHeight.value = 240
mockedScrollY.value = 3000
- render(VirtualGrid, {
+ const { rerender } = render(VirtualGrid, {
props: {
- items,
+ items: initialItems,
gridStyle: defaultGridStyle,
defaultItemHeight: 100,
defaultItemWidth: 100,
maxColumns: 4,
bufferRows: 1
@@
})
await nextTick()
+ await rerender({
+ items: createItems(8),
+ gridStyle: defaultGridStyle,
+ defaultItemHeight: 100,
+ defaultItemWidth: 100,
+ maxColumns: 4,
+ bufferRows: 1
+ })
+ await nextTick()
+
const renderedItems = screen.queryAllByText(/^Item \d+$/)
expect(renderedItems.length).toBeGreaterThan(0)
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/common/VirtualGrid.test.ts` around lines 271 - 298, The test
"keeps last page visible when items shrink below current scroll (FE-535)" never
actually shrinks the list; after mounting VirtualGrid with items and a high
mockedScrollY, update the items array to a smaller set (reuse the items
variable, e.g. replace with createItems(n) where n < original count) after the
initial render, trigger a reactive update (await nextTick() or call the
component's props update mechanism) and then assert that renderedItems
(queryAllByText(/^Item \d+$/)) still has length > 0; locate the mounts and
assertions around the items variable, mockedScrollY, VirtualGrid render call and
the final expect to implement this change.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #11885 +/- ##
===========================================
- Coverage 72.01% 56.08% -15.93%
===========================================
Files 1493 1383 -110
Lines 89607 70778 -18829
Branches 25580 18810 -6770
===========================================
- Hits 64531 39697 -24834
- Misses 24167 30554 +6387
+ Partials 909 527 -382
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 993 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Existing FE-535 cases both fall into the maxOffsetRows=0 branch (small list, viewRows >= totalRows). Add a third case that exercises the other branch — a 100-item list scrolled far past the natural maximum, where maxOffsetRows must clamp the raw offsetRows (21 vs 100000/120) and the last page (including the final item) must remain visible. Asserting Item 99 is present also guards against a regression where the clamp accidentally pushes offsetRows toward the top of the list.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/common/VirtualGrid.test.ts (1)
271-298:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExercise the actual shrink-after-scroll scenario.
This still mounts once with
8items and a highscrollY; it never reproduces the retained-scroll + list-shrink case described by FE-535. Please rerender with a smalleritemsarray after the initial mount and assert the grid stays non-blank.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/VirtualGrid.test.ts` around lines 271 - 298, The test mounts VirtualGrid with createItems(8) and a high mockedScrollY but never simulates the shrink-after-scroll scenario; after the initial render + await nextTick(), rerender the component (or call the testing-library rerender) with a smaller items array (e.g. createItems(3)) while keeping mockedScrollY the same to reproduce retained-scroll + list-shrink, then assert via screen.queryAllByText(/^Item \d+$/) that renderedItems.length remains > 0 so the grid is not blank; update the test that starts with render(VirtualGrid, ...) and the assertion block accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/common/VirtualGrid.test.ts`:
- Around line 271-298: The test mounts VirtualGrid with createItems(8) and a
high mockedScrollY but never simulates the shrink-after-scroll scenario; after
the initial render + await nextTick(), rerender the component (or call the
testing-library rerender) with a smaller items array (e.g. createItems(3)) while
keeping mockedScrollY the same to reproduce retained-scroll + list-shrink, then
assert via screen.queryAllByText(/^Item \d+$/) that renderedItems.length remains
> 0 so the grid is not blank; update the test that starts with
render(VirtualGrid, ...) and the assertion block accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6d7814f8-99d2-4c50-946f-b75d4fb453ff
📒 Files selected for processing (1)
src/components/common/VirtualGrid.test.ts
Replace the hand-rolled scroll/offset/clamp logic with a single useVirtualizer driving rows of `cols` cells. The FE-535 bug class (offsetRows desync when scrollY drifts past the natural max — popover remount with retained scroll, macOS rubberband over-scroll) is structurally eliminated: tanstack reads scrollOffset fresh from the DOM each computation, the browser auto-clamps scrollTop when content shrinks, and bounds are handled inside the library. Public API (props, slots, emits) unchanged. All 5 consumers (FormDropdownMenu, AssetsSidebarListView, AssetsSidebarGridView, AssetGrid, ManagerDialog) continue to work without modification. - Drops `scrollThrottle` / `resizeDebounce` props (internal-only, no consumers passed them) - Drops manual top/bottom spacers in favor of getTotalSize() + absolutely positioned virtual rows - Drops first-child measurement loop (tanstack measures via ResizeObserver) - bufferRows -> overscan (semantic equivalent) - approach-end fires when the last virtual row index is within bufferRows of rowCount Tests: keep behavioral coverage (slot indices, maxColumns rendering, empty list, approach-end emission, last-page invariant). Drop the clamp-implementation-specific FE-535 reproductions since the bug class is gone — replaced with a single invariant test documenting the FE-535 motivation.
dosubot
Bot
added
size:L
dante01yoon
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/common/VirtualGrid.vue`:
- Around line 60-63: Clamp and normalize maxColumns before using it in layout
math: inside the computed cols() (and any similar usage for rowCount), if
maxColumns is finite convert it to a positive integer via Math.max(1,
Math.floor(maxColumns)) and use that value instead of returning maxColumns raw;
otherwise fall back to Math.floor(width.value / defaultItemWidth) || 1. Update
any other places that read maxColumns (e.g., rowCount calculation) to use the
same normalized/clamped value to prevent zero, negative or fractional column
counts.
- Around line 107-117: The watcher on virtualRows currently calls
emit('approach-end') repeatedly while the viewport stays near the end; modify
the watcher to only emit once per crossing by adding a small state guard (e.g.,
a ref like lastApproachIndex or hasEmittedApproachEnd) and only calling
emit('approach-end') when the last.index crosses the threshold (last.index >=
rowCount.value - bufferRows - 1) and is greater than the stored
lastApproachIndex (or if hasEmittedApproachEnd is false), then update that state
when you emit; reference the virtualRows watcher, last (rows.at(-1)), rowCount,
bufferRows, and the emit('approach-end') call when implementing this guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b542f1c9-bd91-4969-a9d6-1f0f40392c82
📒 Files selected for processing (2)
src/components/common/VirtualGrid.test.tssrc/components/common/VirtualGrid.vue
Comment on lines
60
to
63
| const cols = computed(() => { | ||
| if (maxColumns !== Infinity) return maxColumns | ||
| return Math.floor(width.value / itemWidth.value) || 1 | ||
| return Math.floor(width.value / defaultItemWidth) || 1 | ||
| }) |
Contributor
There was a problem hiding this comment.
Clamp maxColumns to a positive integer before using it in layout math.
Line 61 currently returns maxColumns as-is. If callers pass 0, negative, or fractional values, cols/rowCount become invalid and can destabilize virtualization state.
🔧 Proposed fix
+const normalizedMaxColumns = computed(() =>
+ maxColumns === Infinity ? Infinity : Math.max(1, Math.floor(maxColumns))
+)
+
const cols = computed(() => {
- if (maxColumns !== Infinity) return maxColumns
+ if (normalizedMaxColumns.value !== Infinity) {
+ return normalizedMaxColumns.value
+ }
return Math.floor(width.value / defaultItemWidth) || 1
})
const rowCount = computed(() => Math.ceil(items.length / cols.value))Also applies to: 65-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/common/VirtualGrid.vue` around lines 60 - 63, Clamp and
normalize maxColumns before using it in layout math: inside the computed cols()
(and any similar usage for rowCount), if maxColumns is finite convert it to a
positive integer via Math.max(1, Math.floor(maxColumns)) and use that value
instead of returning maxColumns raw; otherwise fall back to
Math.floor(width.value / defaultItemWidth) || 1. Update any other places that
read maxColumns (e.g., rowCount calculation) to use the same normalized/clamped
value to prevent zero, negative or fractional column counts.
Comment on lines
+107
to
+117
| watch( | ||
| virtualRows, | ||
| (rows) => { | ||
| const last = rows.at(-1) | ||
| if (!last) return | ||
| if (last.index >= rowCount.value - bufferRows - 1) { | ||
| emit('approach-end') | ||
| } | ||
| } | ||
| } | ||
| const onResize = debounce(updateItemSize, resizeDebounce) | ||
| watch([width, height], onResize, { flush: 'post' }) | ||
| whenever(() => items, updateItemSize, { flush: 'post' }) | ||
| onBeforeUnmount(() => { | ||
| onResize.cancel() | ||
| }) | ||
| }, | ||
| { flush: 'post' } | ||
| ) |
Contributor
There was a problem hiding this comment.
approach-end currently re-emits on every near-end row update.
Because Line 112 is evaluated inside a watcher of virtualRows, the event can fire repeatedly while the viewport remains near the end, which can trigger duplicate downstream loads.
🔧 Proposed fix
-watch(
- virtualRows,
- (rows) => {
- const last = rows.at(-1)
- if (!last) return
- if (last.index >= rowCount.value - bufferRows - 1) {
- emit('approach-end')
- }
- },
- { flush: 'post' }
-)
+const isNearEnd = computed(() => {
+ const last = virtualRows.value.at(-1)
+ if (!last) return false
+ return last.index >= rowCount.value - bufferRows - 1
+})
+
+watch(
+ isNearEnd,
+ (nearEnd, wasNearEnd) => {
+ if (nearEnd && !wasNearEnd) emit('approach-end')
+ },
+ { flush: 'post' }
+)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/common/VirtualGrid.vue` around lines 107 - 117, The watcher on
virtualRows currently calls emit('approach-end') repeatedly while the viewport
stays near the end; modify the watcher to only emit once per crossing by adding
a small state guard (e.g., a ref like lastApproachIndex or
hasEmittedApproachEnd) and only calling emit('approach-end') when the last.index
crosses the threshold (last.index >= rowCount.value - bufferRows - 1) and is
greater than the stored lastApproachIndex (or if hasEmittedApproachEnd is
false), then update that state when you emit; reference the virtualRows watcher,
last (rows.at(-1)), rowCount, bufferRows, and the emit('approach-end') call when
implementing this guard.
Contributor
There was a problem hiding this comment.
WDYT about migratintg the form dropdown widget to tanstack first, rather than mass-migrating all consumers of VirtualGrid like this?
Contributor
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
question: The PR title and description describe a one-line clamp on offsetRows, but commit 442fe7e replaces the hand-rolled virtualization with @tanstack/vue-virtual. The "Changes" and "Review Focus" sections still describe the old approach. Could the description be updated so reviewers know what they're actually evaluating (a refactor + adoption of a new dependency, not a localized clamp)?
Comment on lines
+107
to
+117
| watch( | ||
| virtualRows, | ||
| (rows) => { | ||
| const last = rows.at(-1) | ||
| if (!last) return | ||
| if (last.index >= rowCount.value - bufferRows - 1) { | ||
| emit('approach-end') | ||
| } | ||
| } | ||
| } | ||
| const onResize = debounce(updateItemSize, resizeDebounce) | ||
| watch([width, height], onResize, { flush: 'post' }) | ||
| whenever(() => items, updateItemSize, { flush: 'post' }) | ||
| onBeforeUnmount(() => { | ||
| onResize.cancel() | ||
| }) | ||
| }, | ||
| { flush: 'post' } | ||
| ) |
Contributor
There was a problem hiding this comment.
issue (non-blocking): the previous implementation used whenever(() => state.value.isNearEnd, ...) which is edge-triggered (only fires on false→true transition). The new watch(virtualRows, ...) runs every time virtualRows recomputes, and getVirtualItems() returns a fresh array on every read, so this can re-emit approach-end many times while the user lingers near the end of the list, and again immediately after newly-loaded items arrive (because the new last row is still within bufferRows of the new end).
Consumers happen to mitigate this today (AssetsSidebarTab.handleApproachEnd is useDebounceFn(..., 300) with an isLoadingMore guard), so it won't cause a runaway loop in this codebase, but the contract has silently changed. Consider tracking the previous "near end" boolean and emitting only on transitions to preserve the old semantics.
| }, | ||
| estimateSize: () => defaultItemHeight, | ||
| getScrollElement: () => container.value, | ||
| overscan: bufferRows |
Contributor
There was a problem hiding this comment.
nitpick (non-blocking): count uses a getter, but overscan: bufferRows and estimateSize: () => defaultItemHeight capture the prop values at construction. With Vue 3.5 reactive props destructuring, prop changes won't propagate to the virtualizer. No current consumer mutates bufferRows or defaultItemHeight, so this is theoretical, but mirroring the count getter pattern keeps the API consistent.
Comment on lines
60
to
63
| const cols = computed(() => { | ||
| if (maxColumns !== Infinity) return maxColumns | ||
| return Math.floor(width.value / itemWidth.value) || 1 | ||
| return Math.floor(width.value / defaultItemWidth) || 1 | ||
| }) |
Contributor
There was a problem hiding this comment.
question: the previous implementation re-derived cols from the first rendered item's measured clientWidth via updateItemSize() on resize. The new code uses only defaultItemWidth as the divisor. If any consumer renders items at a width different from the prop default and does not pass maxColumns, column count will now be wrong (and item indices emitted via the slot will desync from the visual grid). Did you verify all current callers either pass maxColumns or have defaultItemWidth exactly equal to the rendered item width?
| mockedScrollY.value = 0 | ||
|
|
||
| const onApproachEnd = vi.fn() | ||
| it('renders the last item when the entire range is visible (FE-535 invariant)', async () => { |
Contributor
There was a problem hiding this comment.
suggestion (non-blocking): the new test mocks useVirtualizer and asserts that all items render when mockedVisibleEnd = Infinity, which verifies wiring rather than the original FE-535 triggers (PrimeVue Popover retaining mount + retained scrollY across reopen, macOS rubberband over-scroll between throttled samples). The previous tests exercised the actual scroll-state-desync bug. Two options to close this:
- Drive a real
useVirtualizeragainst a JSDOM scroll container in the unit test, instead of mocking it out. - Add a Playwright spec that reopens the LoadImage form-dropdown from the original bug report and asserts thumbnails are visible after a scroll → close → reopen cycle.
Without one of these, FE-535 could regress on a future @TanStack upgrade and we wouldn't catch it.
Comment on lines
+92
to
100
| function rowStyle(virtualRow: VirtualItem): CSSProperties { | ||
| return { | ||
| start: clamp(fromCol, 0, items?.length), | ||
| end: clamp(toCol, fromCol, items?.length), | ||
| isNearEnd: hasMoreToRender && remainingCol <= cols.value * bufferRows | ||
| position: 'absolute', | ||
| top: 0, | ||
| left: 0, | ||
| width: '100%', | ||
| transform: `translateY(${virtualRow.start}px)` | ||
| } | ||
| }) | ||
| const renderedItems = computed(() => | ||
| isValidGrid.value ? items.slice(state.value.start, state.value.end) : [] | ||
| ) | ||
|
|
||
| function rowsToHeight(itemsCount: number): string { | ||
| const rows = Math.ceil(itemsCount / cols.value) | ||
| return `${rows * itemHeight.value}px` | ||
| } |
Contributor
There was a problem hiding this comment.
nitpick (non-blocking): rowStyle() is a plain function so a fresh style object is constructed for every virtual row on every render. Inlining the style literal in the template (or at least caching the static fields) would let Vue skip work when only virtualRow.start changes. Negligible at ~20 visible rows, mention only because this loop is the hot path during scroll.
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
2 participants
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.
Summary
Migrate
VirtualGridfrom a hand-rolled scroll/offset/clamp implementation to@tanstack/vue-virtual(already a project dependency, used byJobAssetsList). The FE-535 bug class is structurally eliminated rather than patched.Fixes FE-535
Why migrate instead of clamp
Original PR (
fix(virtual-grid): clamp offsetRows…) addedmaxOffsetRows = max(0, totalRows - viewRows)to prevent the empty-window symptom. That works, but the bug surfaces from a deeper class of issue: manualscrollY→ offset translation drifts when the source of truth (DOM scroll position) changes outside our event-driven sampling window. Two confirmed triggers:Popoverkeeps the menu mounted on close → on reopen,useScroll'sscrollYcarries the previous deep value whileitemsmay have shrunk (WidgetSelectDropdown.vue:148-152refreshes outputs on open).scrollYpast the natural max between throttled samples.Adding clamps closes the two known holes; switching to a virtualizer that reads scrollOffset fresh from the DOM each computation removes the entire class — the browser has already auto-clamped
scrollTopwhen content shrinks, and bound checks live inside the library.What changes
src/components/common/VirtualGrid.vue— internals replaced withuseVirtualizer(single row-virtualizer,count = ceil(items.length / cols)). Each virtual row is absolutely positioned viatransform: translateY(start), withcolscells laid out by the consumer'sgridStyle(CSS Grid) inside.items,gridStyle,bufferRows,defaultItemHeight,defaultItemWidth,maxColumnsprops;#itemslot with{ item, index };approach-endemit.bufferRowsmaps to tanstackoverscan.scrollThrottle/resizeDebounceprops (internal-only, no consumer passed them).FormDropdownMenu,AssetsSidebarListView,AssetsSidebarGridView,AssetGrid,ManagerDialog) work without modification — verified by 613 passing tests across consumer suites.Net diff: +110 / -236.
Test changes
VirtualGrid.test.ts:useScrollmock for a controllableuseVirtualizermock (same pattern asmockTanstackVirtualizer.tsused byJobAssetsList).clampimplementation; that code path is gone). Replaced with one invariant test documenting the FE-535 motivation: "given items > 0, the user can see them — the grid never silently collapses".maxColumnsrendering, empty list,approach-endemission timing, last-page invariant, cols enforcement.Review focus
gridStyle? Each visible row renders a sub-grid via the consumer-providedgridStyleplus thegridTemplateColumnsoverride whenmaxColumnsis finite. Vertical positioning is viatransform; horizontal layout is unchanged from the previous implementation.approach-endsemantics: emits when last virtual row index ≥rowCount - bufferRows - 1. Equivalent to the previous "≤bufferRowsrows remaining" trigger.Video
before
before.mov
after
after.mov
Live verification of the migration in cloud env still pending (LoadImage form-dropdown widget repro).