Skip to content

fix(preprod): Stabilize snapshot wipe frame resizing#115708

Open
cameroncooke wants to merge 1 commit into
masterfrom
fix/snapshot-wipe-frame
Open

fix(preprod): Stabilize snapshot wipe frame resizing#115708
cameroncooke wants to merge 1 commit into
masterfrom
fix/snapshot-wipe-frame

Conversation

@cameroncooke
Copy link
Copy Markdown
Contributor

@cameroncooke cameroncooke commented May 18, 2026

Snapshot wipe mode now keeps the divider aligned to the rendered snapshot image while viewport and card layouts resize. The previous implementation let the generic slider own both interaction math and visual decoration over the full available container, so the divider could stay mathematically centered in one coordinate system while the visible image moved inside another.

This change separates those concerns: Snapshots now provide a deterministic image-sized frame for the wipe interaction, and ContentSliderDiff remains a reusable slider over its own measured rect. The full-width patterned shell still provides the red/green visual outline, but it mirrors the slider's computed divider position instead of becoming the coordinate system for the drag behavior.

Deterministic snapshot wipe frame

SnapshotWipeFrame sizes itself from the max dimensions of the base/head images and the configured max height. Both single-image wipe mode and list-card wipe mode render the base/head images into that same frame, so the DOM box used by the slider matches the visible bitmap instead of the surrounding negative space.

Slider state and resize behavior

ContentSliderDiff now stores the divider as normalized progress and derives the active pixel position from the current measured slider width. That preserves user intent when the slider frame resizes or briefly collapses to zero width. It also observes an optional visualContainerRef, so if the outer shell changes size while the inner image frame remains stable, the shell's red/green split is recalculated from the current slider rect.

Outer visual shell

SnapshotWipeShell owns the full-width patterned background and red/green border decoration. The actual wipe interaction stays bounded to the image frame, while the shell projects the divider position outward so the visible border split stays aligned with the handle during drag and viewport resize.

Video

Screen.Recording.2026-05-18.at.10.39.43.mov

Refs EME-1109

Move the wipe view's success/danger borders from the slider's full container
onto a shared frame that hugs the actual image, so the colored outline matches
the snapshot rather than the surrounding negative space.

Track the divider position as a container-relative progress value instead of an
absolute pixel size. The slider now restores the user's chosen split when the
container is resized, including when it briefly collapses to zero width, and an
optional visualContainerRef lets callers project the divider onto an outer
frame for the image-hugging borders.

Add SnapshotWipeShell/Frame/Image helpers and use them from both
DiffImageDisplay and WipeCardBody so the two wipe entry points share layout,
sizing, and border styling.
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 18, 2026
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 18, 2026

EME-1109

@github-actions
Copy link
Copy Markdown
Contributor

📊 Type Coverage Diff

Metric Before After Delta
Coverage 93.53% 93.53% ±0%
Typed 135,648 135,695 🟢 +47
Untyped 9,388 9,389 🔴 +1
🔍 1 new type safety issue introduced

Type assertions (as) (1 new)

File Line Detail
static/app/components/contentSliderDiff/index.tsx 128 as CSSProperties{ '--divider-position': ${initialDividerPosition}px, '--divider-progress': di…

This is informational only and does not block the PR.

@cameroncooke cameroncooke marked this pull request as ready for review May 18, 2026 09:35
@cameroncooke cameroncooke requested a review from a team as a code owner May 18, 2026 09:35
Comment on lines +142 to +148
const visualRect = visualContainerRef.current.getBoundingClientRect();
const containerRect = containerRef.current.getBoundingClientRect();
const visualPosition = containerRect.left - visualRect.left + clampedSize;
setDividerCSSVars(visualContainerRef.current, progress, visualPosition);
}
if (dividerElem.current) {
const adjustedLeft = `${clampedSize - 6}px`;
dividerElem.current.style.left = adjustedLeft;

if (dividerElem.current) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The data-at-min-width and data-at-max-width attributes have inverted logic, causing the resize cursor to display incorrectly at the slider's boundaries.
Severity: LOW

Suggested Fix

Swap the conditions for setting the data-at-min-width and data-at-max-width attributes in applyDividerPosition. data-at-min-width should be true when clampedSize === BORDER_WIDTH, and data-at-max-width should be true when clampedSize === maxWidth.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/components/contentSliderDiff/index.tsx#L142-L148

Potential issue: In the `applyDividerPosition` function, the logic for setting the
`data-at-min-width` and `data-at-max-width` attributes is reversed. `data-at-min-width`
is incorrectly set to `true` when the slider position `clampedSize` is equal to
`maxWidth` (the rightmost boundary). Conversely, `data-at-max-width` is set to `true`
when `clampedSize` is at its minimum (`BORDER_WIDTH`, the leftmost boundary). This
causes the associated CSS to apply the wrong cursor: a `w-resize` (west) cursor at the
right edge and an `e-resize` (east) cursor at the left edge, which is confusing for the
user.

Did we get this right? 👍 / 👎 to inform future reviews.

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.

1 participant