Skip to content

fix(preprod): Fix snapshot list scroll-to and duplicate React keys#115396

Merged
NicoHinderling merged 2 commits into
masterfrom
fix/snapshot-list-scroll-settling
May 12, 2026
Merged

fix(preprod): Fix snapshot list scroll-to and duplicate React keys#115396
NicoHinderling merged 2 commits into
masterfrom
fix/snapshot-list-scroll-settling

Conversation

@NicoHinderling
Copy link
Copy Markdown
Contributor

@NicoHinderling NicoHinderling commented May 12, 2026

Two fixes to the snapshot list virtualizer:

Duplicate React keys: Card IDs used the image content hash (image.key),
but images with identical visual content within the same group share a hash.
Switched to image_file_name which is always unique.

Scroll-to drift: When navigating to a specific snapshot via URL param or
sidebar click, scrollIntoView correctly positioned the target on the first
frame. However, the virtualizer's ResizeObserver then measured newly-visible
items (rendered by overscan after the scroll), recalculated all translateY
positions via flushSync, and shifted the target element out of view — while
shouldAdjustScrollPositionOnItemSizeChange = false prevented scroll
compensation.

Fixed by retrying scrollIntoView across animation frames until the element's
position converges (changes < 2px between frames), with a max of 8 retries.
This lets the measurement cascade settle before locking in the final position.

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 12, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

📊 Type Coverage Diff

✅ No new type safety issues introduced. Coverage: 93.50%

@NicoHinderling NicoHinderling force-pushed the fix/snapshot-list-scroll-settling branch from 7b7d615 to c0d9e10 Compare May 12, 2026 20:35
@NicoHinderling NicoHinderling changed the title fix(preprod): Stabilize virtualizer scroll position during programmatic scrolls fix(preprod): Fix snapshot list scroll-to and duplicate React keys May 12, 2026
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 12, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

@NicoHinderling NicoHinderling marked this pull request as ready for review May 12, 2026 21:15
@NicoHinderling NicoHinderling requested review from a team as code owners May 12, 2026 21:15
@NicoHinderling
Copy link
Copy Markdown
Contributor Author

NicoHinderling commented May 12, 2026

forgive me reviewers, pls ignore... it was llm trolling w the rebase 😢

Comment thread static/app/views/seerExplorer/types.tsx Outdated
NicoHinderling and others added 2 commits May 12, 2026 14:22
Two fixes to the snapshot list virtualizer:

1. Use image_file_name instead of content hash for card React keys.
   Images with identical content within the same group shared a hash,
   causing duplicate key warnings and potential rendering issues.

2. Retry scrollIntoView across frames until the element position
   converges. The virtualizer's ResizeObserver measures newly-visible
   items after the initial scroll, recalculating translateY positions
   and shifting the target element. A single scrollIntoView was
   invalidated by these subsequent measurement cascades.

Co-Authored-By: Claude <noreply@anthropic.com>
Replace hardcoded LIST_CONTENT_WIDTH_ASSUMPTION with actual container
width measured via ResizeObserver, so virtualizer height estimates
match rendered sizes. Remove shouldAdjustScrollPositionOnItemSizeChange
override that suppressed scroll compensation, which caused both
programmatic scroll-to drift and scroll-up position jumps. Simplify
initial scroll retry from 8-frame convergence loop to 3-frame
element lookup.

Co-Authored-By: Claude <noreply@anthropic.com>
@NicoHinderling NicoHinderling force-pushed the fix/snapshot-list-scroll-settling branch from cf72370 to 4eaad53 Compare May 12, 2026 21:22
@NicoHinderling NicoHinderling merged commit 49b41a8 into master May 12, 2026
75 checks passed
@NicoHinderling NicoHinderling deleted the fix/snapshot-list-scroll-settling branch May 12, 2026 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components 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