Skip to content

perf(preprod): Virtualize snapshot sidebar for 40k image builds#115836

Merged
NicoHinderling merged 5 commits into
masterfrom
perf/virtualize-snapshot-sidebar
May 19, 2026
Merged

perf(preprod): Virtualize snapshot sidebar for 40k image builds#115836
NicoHinderling merged 5 commits into
masterfrom
perf/virtualize-snapshot-sidebar

Conversation

@NicoHinderling
Copy link
Copy Markdown
Contributor

The snapshot sidebar rendered every item as a real DOM node. When viewing
a build with 40,000 images, this meant 40k+ DOM elements in the sidebar
alone, causing severe page lag — scrolling, typing in search, and
navigating were all sluggish.

This replaces the flat rendering with @tanstack/react-virtual (the
same library already used by the main list view). Only ~50 sidebar nodes
exist in the DOM at any time (visible items + 25 overscan). The section
header collapse/expand, search filtering, active-item highlighting, and
scroll-to-active behaviors are all preserved.

Additional changes:

  • Added loading="lazy" and decoding="async" to the LazyImage
    component's <img> as a belt-and-suspenders measure alongside the
    existing virtualizer
  • Replaced <Disclosure> section headers with a standalone button
    (Disclosure wraps children, which is incompatible with virtualization),
    preserving aria-expanded for accessibility
  • Memoized SidebarItem and SectionHeaderRow sub-components
  • Fixed scroll-to-active to only fire on actual activeItemKey changes,
    not on collapse/expand which also mutates virtualRows

The snapshot sidebar rendered all items as real DOM nodes, causing
severe lag when viewing builds with 40,000+ images. Replace with
@tanstack/react-virtual to only render ~50 nodes at a time.

Also adds loading="lazy" to list view images and updates tests for
JSDOM compatibility with the virtualizer.

Co-Authored-By: Claude <noreply@anthropic.com>
@NicoHinderling NicoHinderling marked this pull request as ready for review May 19, 2026 19:26
@NicoHinderling NicoHinderling requested a review from a team as a code owner May 19, 2026 19:26
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 19, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

📊 Type Coverage Diff

Metric Before After Delta
Coverage 93.56% 93.56% ±0%
Typed 135,578 135,596 🟢 +18
Untyped 9,334 9,336 🔴 +2
🔍 2 new type safety issues introduced

Non-null assertions (!) (2 new)

File Line Detail
static/app/views/preprod/snapshots/sidebar/snapshotSidebarContent.tsx 137 virtualRows[i]!
static/app/views/preprod/snapshots/sidebar/snapshotSidebarContent.tsx 210 virtualRows[vi.index]!

This is informational only and does not block the PR.

@NicoHinderling NicoHinderling enabled auto-merge (squash) May 19, 2026 19:31
@NicoHinderling NicoHinderling disabled auto-merge May 19, 2026 19:31
Copy link
Copy Markdown
Contributor

@mtopo27 mtopo27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix snapshots

NicoHinderling and others added 2 commits May 19, 2026 12:36
The virtualizer needs to measure the scroll container before rendering
items. In the snapshot testing environment, the measurement cycle
hasn't completed by screenshot time, resulting in empty sidebars.
initialRect provides dimensions for the first paint.

Co-Authored-By: Claude <noreply@anthropic.com>
Initialize prevActiveItemKeyRef to undefined so the scroll-to-active
effect fires on initial mount when activeItemKey is already set (e.g.,
deep linking to a specific snapshot).

Co-Authored-By: Claude <noreply@anthropic.com>
…tyling

The virtualized section header button had larger padding (8px 16px) and
gap (6px) than the original Disclosure component. Align height, padding,
gap, and font-size with the form.xs theme values to eliminate visual
regression.

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 51c6d93. Configure here.

Comment thread static/app/views/preprod/snapshots/sidebar/snapshotSidebarContent.tsx Outdated
Replace the custom SectionHeaderButton with the original
SectionDisclosure + Disclosure.Title components, and restore the
original Stack scroll container. This ensures zero visual regression
while keeping the virtualizer for performance.

Co-Authored-By: Claude <noreply@anthropic.com>
@NicoHinderling NicoHinderling merged commit 76e2447 into master May 19, 2026
70 of 71 checks passed
@NicoHinderling NicoHinderling deleted the perf/virtualize-snapshot-sidebar branch May 19, 2026 20:36
NicoHinderling added a commit that referenced this pull request May 20, 2026
…15922)

Remove `loading="lazy"` and `decoding="async"` from the `LazyImage`
component
in the snapshot diff view.

`LazyImage` already manages its own visibility via `loaded` state and
`opacity: 0` positioning — it renders the `<img>` immediately but hides
it
until `onLoad` fires. Stacking the browser's native `loading="lazy"` on
top
caused the browser to defer fetching images it considered off-screen
(inside
scrollable containers with `opacity: 0`), so the `onLoad` event never
fired
and base images stayed blank.

Introduced in #115836.

Co-authored-by: Claude <noreply@anthropic.com>
JonasBa pushed a commit that referenced this pull request May 21, 2026
…15922)

Remove `loading="lazy"` and `decoding="async"` from the `LazyImage`
component
in the snapshot diff view.

`LazyImage` already manages its own visibility via `loaded` state and
`opacity: 0` positioning — it renders the `<img>` immediately but hides
it
until `onLoad` fires. Stacking the browser's native `loading="lazy"` on
top
caused the browser to defer fetching images it considered off-screen
(inside
scrollable containers with `opacity: 0`), so the `onLoad` event never
fired
and base images stayed blank.

Introduced in #115836.

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants