Skip to content

feat(preprod): Add SnapshotListView virtualized list component#113992

Open
NicoHinderling wants to merge 5 commits intomasterfrom
04-24-feat_preprod_add_snapshot_list_view
Open

feat(preprod): Add SnapshotListView virtualized list component#113992
NicoHinderling wants to merge 5 commits intomasterfrom
04-24-feat_preprod_add_snapshot_list_view

Conversation

@NicoHinderling
Copy link
Copy Markdown
Contributor

Introduces the new snapshot viewer's list view as a self-contained component, plus one supporting option on the d3-zoom hook the list needs. Not wired into any existing UI. Integration lives in the follow-up PR.

New file: snapshotListView.tsx (~1300 lines)

A grouped, virtualized list of snapshot cards using @tanstack/react-virtual. Renders PairCard for diff items and ImageCard for solo / added / removed. Each card has a status badge, a copy-link button, a metadata tooltip, and a body that switches between split / wipe / onion depending on diffMode. Split-pair bodies share zoom/pan via useSyncedD3Zoom.

The component takes SidebarItem[] plus image URL bases and exposes buildSnapshotLink, GroupHeader, CardHeader, DarkAware, and a few styled helpers for callers that want to render pieces of the list layout outside the virtualized container.

Supporting change: useD3Zoom.ts (+8 lines)

Adds an optional wheelRequiresModifier flag (default false) so the list view can require ctrl/meta/shift on wheel events. Without it, scrolling over the list would always be hijacked by zoom. Preserves existing behavior for all current callers.

What lands alongside:

  • No callers yet — the file compiles but is not imported anywhere. knip will still be happy because SnapshotListView is exported and TSC treats it as a top-level module.

3/N in the series slicing up #113958.

@NicoHinderling NicoHinderling force-pushed the 04-24-ref_preprod_sidebar_filter_pills branch from 492b207 to 73d9ab0 Compare April 28, 2026 18:32
@NicoHinderling NicoHinderling force-pushed the 04-24-feat_preprod_add_snapshot_list_view branch from da939be to 47b847f Compare April 28, 2026 18:32
@NicoHinderling NicoHinderling force-pushed the 04-24-feat_preprod_add_snapshot_list_view branch from 47b847f to 5f96cbb Compare April 28, 2026 18:54
@NicoHinderling NicoHinderling force-pushed the 04-24-ref_preprod_sidebar_filter_pills branch from 73d9ab0 to 33ebe4d Compare April 28, 2026 18:54
@NicoHinderling NicoHinderling force-pushed the 04-24-feat_preprod_add_snapshot_list_view branch from 5f96cbb to f2c43b9 Compare April 28, 2026 19:08
@NicoHinderling NicoHinderling force-pushed the 04-24-ref_preprod_sidebar_filter_pills branch from 33ebe4d to ef57ed8 Compare April 28, 2026 19:08
@NicoHinderling NicoHinderling force-pushed the 04-24-ref_preprod_sidebar_filter_pills branch from ef57ed8 to 613e534 Compare April 28, 2026 22:33
@NicoHinderling NicoHinderling force-pushed the 04-24-feat_preprod_add_snapshot_list_view branch from f2c43b9 to 13c039f Compare April 28, 2026 22:33
@NicoHinderling NicoHinderling marked this pull request as ready for review April 28, 2026 22:33
@NicoHinderling NicoHinderling requested a review from a team as a code owner April 28, 2026 22:33
Comment thread static/app/views/preprod/snapshots/main/snapshotListView.tsx
Comment thread static/app/views/preprod/snapshots/main/snapshotListView.tsx
Comment thread static/app/views/preprod/snapshots/main/snapshotDiffBodies.tsx Outdated
Comment thread static/app/views/preprod/snapshots/main/snapshotDiffBodies.tsx
@NicoHinderling NicoHinderling force-pushed the 04-24-ref_preprod_sidebar_filter_pills branch from 66de93f to cd8bb13 Compare April 28, 2026 22:49
@NicoHinderling NicoHinderling force-pushed the 04-24-feat_preprod_add_snapshot_list_view branch from 13c039f to f1e64ed Compare April 28, 2026 22:49
Comment thread static/app/views/preprod/snapshots/main/snapshotListView.tsx Outdated
Comment thread static/app/views/preprod/snapshots/main/snapshotListView.tsx
@NicoHinderling NicoHinderling force-pushed the 04-24-ref_preprod_sidebar_filter_pills branch from cd8bb13 to 19f405d Compare April 28, 2026 23:16
Comment thread static/app/views/preprod/snapshots/main/snapshotListView.tsx
Comment thread static/app/views/preprod/snapshots/main/snapshotListView.tsx Outdated
@NicoHinderling NicoHinderling force-pushed the 04-24-feat_preprod_add_snapshot_list_view branch from 6b5e8b4 to 126773c Compare April 28, 2026 23:44
@NicoHinderling NicoHinderling force-pushed the 04-24-ref_preprod_sidebar_filter_pills branch from c9fa4cf to 84281ac Compare April 28, 2026 23:44
Comment thread static/app/views/preprod/snapshots/main/snapshotListView.tsx
@NicoHinderling NicoHinderling force-pushed the 04-24-feat_preprod_add_snapshot_list_view branch from 093db44 to 49b03b5 Compare April 29, 2026 00:10
@NicoHinderling NicoHinderling force-pushed the 04-24-ref_preprod_sidebar_filter_pills branch from 84281ac to 6a042b9 Compare April 29, 2026 00:10
Base automatically changed from 04-24-ref_preprod_sidebar_filter_pills to master April 29, 2026 00:23
NicoHinderling and others added 5 commits April 28, 2026 17:24
Introduce the new snapshot viewer's list view as a self-contained
component. Not wired into the UI yet; follow-up PR integrates it
behind a view mode toggle.

The list view renders grouped snapshot cards with its own virtualization
(via @tanstack/react-virtual), synced d3-zoom panes per card, status
badges, and diff/wipe/onion body variants. It operates directly on
SidebarItem data without any additional state plumbing, so callers
can render it by supplying items + image URLs.

Supporting change: useSyncedD3Zoom / useD3Zoom gain an optional
`wheelRequiresModifier` flag that the list view turns on so wheel
events without ctrl/meta/shift pass through to page scroll rather
than being captured for zoom.
…lute positioning

ZoomControls already renders with position: absolute internally, so the
extra wrapper collapsed to 0×0 and misplaced the controls.
- Preserve existing query params in buildSnapshotLink so copied links
  retain the user's current filters and view mode
- Use max of base/head image heights for pair card estimates to avoid
  layout shift when base is taller than head
- Store virtualizer in a ref so the keydown listener registers once
  instead of re-attaching on every render during scroll

Co-Authored-By: Claude <noreply@anthropic.com>
RowPositioner adds padding-bottom: theme.space.xl (16px) that was not
accounted for in estimatedHeight, causing cumulative scroll-position
jumps as measureElement corrected the underestimates.

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 aaf17be. Configure here.

Comment thread static/app/views/preprod/snapshots/main/snapshotDiffBodies.tsx
}

// >= 1% shows 1 decimal ("93.5"); < 1% shows up to 4 without trailing zeros ("0.0227")
function formatDiffPercent(diff: number): string {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would recommend using import {formatPercentage} from 'sentry/utils/number/formatPercentage'; if possible, no need for a new function if we can use existing

if (e.key === 'Enter') {
e.preventDefault();
onSelect();
} else if (e.key === ' ') {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not quite following what the space key is for here? The comment doesn't really make sense to me

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It will recenter the card in the list view if you hit it, but i had some key conflict and had to add this preventDefault logic unfortunately

It works nicely in the final demo

border-bottom: 1px solid ${p => p.theme.tokens.border.secondary};
`;

const MetadataJson = styled('pre')`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure what this looks like but might be better to use https://sentry.sentry.io/stories/core/codeblock/ to render code

);
});

const ImageEl = styled('img')`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is ImageEl, specifically the El short for? Not sure I'm a fan of this naming

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