From 920a7ea6f2c89a7236aa170b734f090b35d12a57 Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Mon, 18 May 2026 09:48:34 +0100 Subject: [PATCH] feat(preprod): Wrap snapshot wipe view borders around the image 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. --- .../contentSliderDiff/index.spec.tsx | 249 +++++++++++++++++- .../components/contentSliderDiff/index.tsx | 158 +++++++++-- .../main/imageDisplay/diffImageDisplay.tsx | 76 ++++-- .../snapshots/main/snapshotDiffBodies.tsx | 89 ++++--- .../snapshots/main/snapshotWipeFrame.tsx | 83 ++++++ 5 files changed, 557 insertions(+), 98 deletions(-) create mode 100644 static/app/views/preprod/snapshots/main/snapshotWipeFrame.tsx diff --git a/static/app/components/contentSliderDiff/index.spec.tsx b/static/app/components/contentSliderDiff/index.spec.tsx index 7cb686b5aa6f04..6814a4c38d1d85 100644 --- a/static/app/components/contentSliderDiff/index.spec.tsx +++ b/static/app/components/contentSliderDiff/index.spec.tsx @@ -1,10 +1,111 @@ +import {useRef} from 'react'; + import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; import * as useDimensions from 'sentry/utils/useDimensions'; import {ContentSliderDiff} from '.'; +function renderContentSliderDiff() { + return render( + Before Content} + after={
After Content
} + /> + ); +} + +function getSliderStyleProperty(property: string) { + return parseFloat( + screen.getByTestId('drag-handle').parentElement!.style.getPropertyValue(property) + ); +} + +function getDividerProgress() { + return getSliderStyleProperty('--divider-progress'); +} + +function createRect({ + width, + height, + left = 0, +}: { + height: number; + width: number; + left?: number; +}) { + return { + bottom: height, + height, + left, + right: left + width, + top: 0, + width, + x: left, + y: 0, + toJSON: () => ({}), + }; +} + +function mockElementRect({width, height}: {height: number; width: number}) { + jest + .spyOn(HTMLElement.prototype, 'getBoundingClientRect') + .mockReturnValue(createRect({width, height})); +} + +function mockImageWrappingLayout(sizes: { + containerWidth: number; + height: number; + imageWidth: number; +}) { + jest + .spyOn(HTMLElement.prototype, 'getBoundingClientRect') + .mockImplementation(function (this: HTMLElement) { + if (this.dataset.testId === 'visual-container') { + return createRect({ + width: sizes.containerWidth, + height: sizes.height, + left: 0, + }); + } + return createRect({ + width: sizes.imageWidth, + height: sizes.height, + left: (sizes.containerWidth - sizes.imageWidth) / 2, + }); + }); +} + +function ContentSliderDiffWithVisualContainer( + props: Omit, 'visualContainerRef'> +) { + const visualRef = useRef(null); + return ( +
+ +
+ ); +} + +function getOuterDividerPosition() { + return parseFloat( + screen.getByTestId('visual-container').style.getPropertyValue('--divider-position') + ); +} + +async function setDividerPosition(x: number) { + const target = screen.getByTestId('drag-handle'); + await userEvent.pointer([ + {keys: '[MouseLeft>]', target, coords: {x, y: 5}}, + {keys: '[/MouseLeft]', target, coords: {x, y: 5}}, + ]); +} + describe('ContentSliderDiff', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + it('divider can be dragged', async () => { jest.spyOn(useDimensions, 'useDimensions').mockReturnValue({width: 300, height: 300}); @@ -20,10 +121,10 @@ describe('ContentSliderDiff', () => { const dragHandle = screen.getByTestId('drag-handle'); - // Simulate dragging the divider await userEvent.pointer([ {keys: '[MouseLeft>]', target: dragHandle, coords: {x: 0, y: 5}}, {target: dragHandle, coords: {x: 10, y: 5}}, + {keys: '[/MouseLeft]', target: dragHandle, coords: {x: 10, y: 5}}, ]); expect(mockDragHandleMouseDown).toHaveBeenCalledTimes(1); @@ -32,7 +133,97 @@ describe('ContentSliderDiff', () => { it('does not render content when dimensions are zero', () => { jest.spyOn(useDimensions, 'useDimensions').mockReturnValue({width: 0, height: 0}); - render( + renderContentSliderDiff(); + + expect(screen.queryByTestId('before-content')).not.toBeInTheDocument(); + }); + + it('preserves the default centered divider position when resized', () => { + const useDimensionsSpy = jest.spyOn(useDimensions, 'useDimensions'); + useDimensionsSpy.mockReturnValue({width: 300, height: 300}); + + const {rerender} = renderContentSliderDiff(); + + expect(getDividerProgress()).toBeCloseTo(0.5); + + useDimensionsSpy.mockReturnValue({width: 600, height: 300}); + rerender( + Before Content} + after={
After Content
} + /> + ); + + expect(getDividerProgress()).toBeCloseTo(0.5); + }); + + it('preserves a user-set container-relative divider position when resized', async () => { + const useDimensionsSpy = jest.spyOn(useDimensions, 'useDimensions'); + useDimensionsSpy.mockReturnValue({width: 300, height: 300}); + mockElementRect({width: 300, height: 300}); + + const {rerender} = renderContentSliderDiff(); + + await setDividerPosition(210); + + expect(getDividerProgress()).toBeCloseTo(0.7); + + useDimensionsSpy.mockReturnValue({width: 600, height: 300}); + rerender( + Before Content} + after={
After Content
} + /> + ); + + expect(getDividerProgress()).toBeCloseTo(0.7); + }); + + it('projects an image-relative divider position onto the outer container when resized', async () => { + const useDimensionsSpy = jest.spyOn(useDimensions, 'useDimensions'); + const sizes = {containerWidth: 400, imageWidth: 200, height: 300}; + useDimensionsSpy.mockReturnValue({width: sizes.imageWidth, height: sizes.height}); + mockImageWrappingLayout(sizes); + + const {rerender} = render( + } + after={After content} + /> + ); + + await setDividerPosition(240); + + expect(getDividerProgress()).toBeCloseTo(0.7); + expect(getOuterDividerPosition()).toBeCloseTo(240); + + sizes.containerWidth = 800; + sizes.imageWidth = 400; + useDimensionsSpy.mockReturnValue({width: sizes.imageWidth, height: sizes.height}); + rerender( + } + after={After content} + /> + ); + + expect(getDividerProgress()).toBeCloseTo(0.7); + expect(getOuterDividerPosition()).toBeCloseTo(480); + }); + + it('preserves a user-set divider position when resized through zero width', async () => { + const useDimensionsSpy = jest.spyOn(useDimensions, 'useDimensions'); + useDimensionsSpy.mockReturnValue({width: 300, height: 300}); + mockElementRect({width: 300, height: 300}); + + const {rerender} = renderContentSliderDiff(); + + await setDividerPosition(210); + + expect(getDividerProgress()).toBeCloseTo(0.7); + + useDimensionsSpy.mockReturnValue({width: 0, height: 0}); + rerender( Before Content} after={
After Content
} @@ -40,5 +231,59 @@ describe('ContentSliderDiff', () => { ); expect(screen.queryByTestId('before-content')).not.toBeInTheDocument(); + + useDimensionsSpy.mockReturnValue({width: 600, height: 300}); + rerender( + Before Content} + after={
After Content
} + /> + ); + + expect(getDividerProgress()).toBeCloseTo(0.7); + }); + + it('preserves left boundary intent when resized', async () => { + const useDimensionsSpy = jest.spyOn(useDimensions, 'useDimensions'); + useDimensionsSpy.mockReturnValue({width: 300, height: 300}); + mockElementRect({width: 300, height: 300}); + + const {rerender} = renderContentSliderDiff(); + + await setDividerPosition(-20); + + expect(getDividerProgress()).toBeCloseTo(0); + + useDimensionsSpy.mockReturnValue({width: 600, height: 300}); + rerender( + Before Content} + after={
After Content
} + /> + ); + + expect(getDividerProgress()).toBeCloseTo(0); + }); + + it('preserves right boundary intent when resized', async () => { + const useDimensionsSpy = jest.spyOn(useDimensions, 'useDimensions'); + useDimensionsSpy.mockReturnValue({width: 300, height: 300}); + mockElementRect({width: 300, height: 300}); + + const {rerender} = renderContentSliderDiff(); + + await setDividerPosition(400); + + expect(getDividerProgress()).toBeCloseTo(1); + + useDimensionsSpy.mockReturnValue({width: 600, height: 300}); + rerender( + Before Content} + after={
After Content
} + /> + ); + + expect(getDividerProgress()).toBeCloseTo(1); }); }); diff --git a/static/app/components/contentSliderDiff/index.tsx b/static/app/components/contentSliderDiff/index.tsx index 7bc380e9bbeb12..3d9f9d6344ada8 100644 --- a/static/app/components/contentSliderDiff/index.tsx +++ b/static/app/components/contentSliderDiff/index.tsx @@ -1,4 +1,4 @@ -import {useRef, type CSSProperties} from 'react'; +import {useCallback, useLayoutEffect, useRef, type CSSProperties} from 'react'; import styled from '@emotion/styled'; import {Container} from '@sentry/scraps/layout'; @@ -23,6 +23,8 @@ interface Props { * Useful when we want to track analytics. */ onDragHandleMouseDown?: (e: React.MouseEvent) => void; + showBorders?: boolean; + visualContainerRef?: React.RefObject; } /** @@ -31,8 +33,16 @@ interface Props { * The before and after contents are not directly defined here and have to be provided, so it can be very flexible * (e.g. images, replays, etc). */ -function Body({onDragHandleMouseDown, after, before, minHeight = '0px'}: Props) { +function Body({ + onDragHandleMouseDown, + after, + before, + minHeight = '0px', + showBorders = true, + visualContainerRef, +}: Props) { const positionedRef = useRef(null); + const dividerProgressRef = useRef(DEFAULT_DIVIDER_PROGRESS); const viewDimensions = useDimensions({elementRef: positionedRef}); return ( @@ -47,7 +57,10 @@ function Body({onDragHandleMouseDown, after, before, minHeight = '0px'}: Props) {viewDimensions.width ? ( @@ -60,36 +73,79 @@ function Body({onDragHandleMouseDown, after, before, minHeight = '0px'}: Props) } const BORDER_WIDTH = 3; +const DEFAULT_DIVIDER_PROGRESS = 0.5; -interface SideProps extends Pick { +function getMaxDividerSize(containerWidth: number) { + return Math.max(BORDER_WIDTH, containerWidth - BORDER_WIDTH); +} + +function getDividerProgress(size: number, containerWidth: number) { + if (containerWidth === 0) { + return DEFAULT_DIVIDER_PROGRESS; + } + + return Math.max(0, Math.min(1, size / containerWidth)); +} + +function getDividerPosition(progress: number, containerWidth: number) { + return progress * containerWidth; +} + +function setDividerCSSVars(el: HTMLElement | null, progress: number, positionPx: number) { + el?.style.setProperty('--divider-progress', String(progress)); + el?.style.setProperty('--divider-position', `${positionPx}px`); +} + +interface SideProps extends Pick< + Props, + 'onDragHandleMouseDown' | 'before' | 'after' | 'showBorders' | 'visualContainerRef' +> { + dividerProgressRef: React.MutableRefObject; viewDimensions: ReturnType; } -function Sides({onDragHandleMouseDown, viewDimensions, before, after}: SideProps) { - const beforeElemRef = useRef(null); +function Sides({ + onDragHandleMouseDown, + viewDimensions, + dividerProgressRef, + showBorders = true, + visualContainerRef, + before, + after, +}: SideProps) { const dividerElem = useRef(null); + const emptyVisualContainerRef = useRef(null); + const visualContainerDimensions = useDimensions({ + elementRef: visualContainerRef ?? emptyVisualContainerRef, + }); const width = `${viewDimensions.width}px`; const containerRef = useRef(null); - - const {onMouseDown, onDoubleClick, setSize} = useResizableDrawer({ - direction: 'left', - initialSize: viewDimensions.width / 2, - min: 0, - onResize: newSize => { - const maxWidth = viewDimensions.width - BORDER_WIDTH; - const clampedSize = Math.max(BORDER_WIDTH, Math.min(maxWidth, newSize)); - - if (beforeElemRef.current) { - beforeElemRef.current.style.width = - viewDimensions.width === 0 - ? '100%' - : `${Math.max(BORDER_WIDTH, Math.min(maxWidth, newSize))}px`; + const initialDividerPosition = getDividerPosition( + dividerProgressRef.current, + viewDimensions.width + ); + const dividerSizeStyle = { + '--divider-position': `${initialDividerPosition}px`, + '--divider-progress': dividerProgressRef.current, + } as CSSProperties; + + const applyDividerPosition = useCallback( + (position: number) => { + const maxWidth = getMaxDividerSize(viewDimensions.width); + const clampedSize = Math.max(BORDER_WIDTH, Math.min(maxWidth, position)); + const progress = dividerProgressRef.current; + + setDividerCSSVars(containerRef.current, progress, clampedSize); + + if (visualContainerRef?.current && containerRef.current) { + 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) { dividerElem.current.setAttribute( 'data-at-min-width', String(clampedSize === maxWidth) @@ -100,8 +156,38 @@ function Sides({onDragHandleMouseDown, viewDimensions, before, after}: SideProps ); } }, + [dividerProgressRef, viewDimensions.width, visualContainerRef] + ); + + const {onMouseDown, onDoubleClick, setSize} = useResizableDrawer({ + direction: 'left', + initialSize: viewDimensions.width / 2, + min: 0, + onResize: (newSize, _oldSize, userEvent) => { + if (userEvent && viewDimensions.width > BORDER_WIDTH) { + dividerProgressRef.current = getDividerProgress(newSize, viewDimensions.width); + } + + applyDividerPosition( + getDividerPosition(dividerProgressRef.current, viewDimensions.width) + ); + }, }); + const syncDividerPosition = useCallback(() => { + applyDividerPosition( + getDividerPosition(dividerProgressRef.current, viewDimensions.width) + ); + }, [applyDividerPosition, dividerProgressRef, viewDimensions.width]); + + useLayoutEffect(() => { + syncDividerPosition(); + }, [ + syncDividerPosition, + visualContainerDimensions.height, + visualContainerDimensions.width, + ]); + const handleContainerMouseDown = (event: React.MouseEvent) => { if (event.button !== 0 || !containerRef.current) { return; @@ -114,13 +200,17 @@ function Sides({onDragHandleMouseDown, viewDimensions, before, after}: SideProps }; return ( - - + + {after} - + {before} @@ -191,7 +281,10 @@ const DragIndicator = styled('div')` const DragHandle = styled('div')` position: absolute; top: 0; - left: 0; + left: calc( + clamp(${BORDER_WIDTH}px, var(--divider-position), calc(100% - ${BORDER_WIDTH}px)) - + 6px + ); width: 12px; height: 100%; cursor: ew-resize; @@ -239,7 +332,7 @@ const DragHandle = styled('div')` } `; -const Cover = styled('div')` +const Cover = styled('div')<{$showBorders: boolean}>` border: ${BORDER_WIDTH}px solid; border-radius: ${p => p.theme.space.xs}; height: 100%; @@ -248,11 +341,18 @@ const Cover = styled('div')` left: 0px; top: 0px; - border-color: ${p => p.theme.tokens.border.success.moderate}; + border-color: ${p => + p.$showBorders ? p.theme.tokens.border.success.moderate : 'transparent'}; & + & { + width: clamp( + ${BORDER_WIDTH}px, + var(--divider-position), + calc(100% - ${BORDER_WIDTH}px) + ); border: ${BORDER_WIDTH}px solid; border-radius: ${p => p.theme.space.xs} 0 0 ${p => p.theme.space.xs}; - border-color: ${p => p.theme.tokens.border.danger.moderate}; + border-color: ${p => + p.$showBorders ? p.theme.tokens.border.danger.moderate : 'transparent'}; border-right-width: 0; } `; diff --git a/static/app/views/preprod/snapshots/main/imageDisplay/diffImageDisplay.tsx b/static/app/views/preprod/snapshots/main/imageDisplay/diffImageDisplay.tsx index 6b1f0ea87d67b7..f45c13e4cf2831 100644 --- a/static/app/views/preprod/snapshots/main/imageDisplay/diffImageDisplay.tsx +++ b/static/app/views/preprod/snapshots/main/imageDisplay/diffImageDisplay.tsx @@ -1,4 +1,4 @@ -import {useState} from 'react'; +import {useRef, useState} from 'react'; import styled from '@emotion/styled'; import {Image} from '@sentry/scraps/image'; @@ -10,7 +10,16 @@ import {ContentSliderDiff} from 'sentry/components/contentSliderDiff'; import {t} from 'sentry/locale'; import {computeMaskSize} from 'sentry/views/preprod/snapshots/main/computeMaskSize'; import {DiffOverlay} from 'sentry/views/preprod/snapshots/main/diffOverlay'; -import type {SnapshotDiffPair} from 'sentry/views/preprod/types/snapshotTypes'; +import { + getSnapshotWipeFrameStyle, + SnapshotWipeFrame, + SnapshotWipeImage, + SnapshotWipeShell, +} from 'sentry/views/preprod/snapshots/main/snapshotWipeFrame'; +import type { + SnapshotDiffPair, + SnapshotImage, +} from 'sentry/views/preprod/types/snapshotTypes'; import {useBufferedImageGroup} from './useBufferedImageUrl'; import {useSyncedD3Zoom} from './useD3Zoom'; @@ -68,7 +77,9 @@ export function DiffImageDisplay({ @@ -191,11 +202,15 @@ const splitZoomContainerStyle: React.CSSProperties = { }; function WipeView({ + baseImage, baseImageUrl, + headImage, headImageUrl, headLabel, }: { + baseImage: SnapshotImage; baseImageUrl: string; + headImage: SnapshotImage; headImageUrl: string; headLabel: string; }) { @@ -203,32 +218,41 @@ function WipeView({ baseImageUrl, headImageUrl, ]); + const visualContainerRef = useRef(null); + return ( - + {displayBaseUrl && displayHeadUrl && ( - - - - } - after={ - - - - } - minHeight="200px" - /> + + + + } + after={ + + } + showBorders={false} + visualContainerRef={visualContainerRef} + /> + + )} ); diff --git a/static/app/views/preprod/snapshots/main/snapshotDiffBodies.tsx b/static/app/views/preprod/snapshots/main/snapshotDiffBodies.tsx index ae257326ff32d2..d45f6da105decc 100644 --- a/static/app/views/preprod/snapshots/main/snapshotDiffBodies.tsx +++ b/static/app/views/preprod/snapshots/main/snapshotDiffBodies.tsx @@ -1,4 +1,4 @@ -import {memo, type ReactNode, useCallback, useEffect, useState} from 'react'; +import {memo, type ReactNode, useCallback, useEffect, useRef, useState} from 'react'; import styled from '@emotion/styled'; import {Container, Flex, Grid, Stack} from '@sentry/scraps/layout'; @@ -15,6 +15,12 @@ import {useSyncedD3Zoom} from './imageDisplay/useD3Zoom'; import {ZoomControls, zoomTransformStyle} from './imageDisplay/zoomControls'; import {computeMaskSize} from './computeMaskSize'; import {DiffOverlay} from './diffOverlay'; +import { + getSnapshotWipeFrameStyle, + SnapshotWipeFrame, + SnapshotWipeImage, + SnapshotWipeShell, +} from './snapshotWipeFrame'; export const MAX_IMAGE_HEIGHT = 480; @@ -177,35 +183,45 @@ export const WipeCardBody = memo(function WipeCardBody({ const minHeight = naturalHeight ? `${Math.min(Math.max(naturalHeight, WIPE_MIN_HEIGHT), MAX_IMAGE_HEIGHT)}px` : `${WIPE_MIN_HEIGHT}px`; + const visualContainerRef = useRef(null); + return ( - - - - - } - after={ - - - - } - minHeight={minHeight} - /> + + + + + } + after={ + + } + minHeight={minHeight} + showBorders={false} + visualContainerRef={visualContainerRef} + /> + + ); }); @@ -300,8 +316,7 @@ function LazyImage({ setLoaded(false); }, [src]); - const onLoad = useCallback(() => setLoaded(true), []); - const onError = useCallback(() => setLoaded(true), []); + const markLoaded = useCallback(() => setLoaded(true), []); const refCallback = useCallback((el: HTMLImageElement | null) => { if (el?.complete && el.naturalWidth > 0) { setLoaded(true); @@ -327,8 +342,8 @@ function LazyImage({ alt={alt} width={width || undefined} height={height || undefined} - onLoad={onLoad} - onError={onError} + onLoad={markLoaded} + onError={markLoaded} style={loaded ? undefined : {position: 'absolute', top: 0, left: 0, opacity: 0}} /> {loaded && children} @@ -376,14 +391,6 @@ const ZoomTransformLayer = styled('div')` will-change: transform; `; -const WipeImg = styled('img')` - display: block; - max-width: 100%; - max-height: ${MAX_IMAGE_HEIGHT}px; - height: auto; - object-fit: contain; -`; - const OnionImg = styled('img')` position: absolute; top: 0; diff --git a/static/app/views/preprod/snapshots/main/snapshotWipeFrame.tsx b/static/app/views/preprod/snapshots/main/snapshotWipeFrame.tsx new file mode 100644 index 00000000000000..52db9ca9604950 --- /dev/null +++ b/static/app/views/preprod/snapshots/main/snapshotWipeFrame.tsx @@ -0,0 +1,83 @@ +import type {CSSProperties} from 'react'; +import styled from '@emotion/styled'; + +import {NegativeSpaceContainer} from 'sentry/components/container/negativeSpaceContainer'; +import type {SnapshotImage} from 'sentry/views/preprod/types/snapshotTypes'; + +export function getSnapshotWipeFrameStyle({ + baseImage, + headImage, + maxHeight, +}: { + baseImage: SnapshotImage; + headImage: SnapshotImage; + maxHeight: string; +}): CSSProperties { + const width = Math.max(baseImage.width || 0, headImage.width || 0); + const height = Math.max(baseImage.height || 0, headImage.height || 0); + + if (!width || !height) { + return {width: '100%'}; + } + + return { + aspectRatio: `${width} / ${height}`, + width: `min(100%, ${width}px, calc(${maxHeight} * ${width / height}))`, + }; +} + +const BORDER_WIDTH = 3; + +export const SnapshotWipeShell = styled(NegativeSpaceContainer)<{$minHeight?: string}>` + position: relative; + display: flex; + align-items: center; + justify-content: center; + box-sizing: border-box; + width: 100%; + height: 100%; + min-height: ${p => p.$minHeight ?? 0}; + padding: ${p => p.theme.space.md}; + border-radius: ${p => p.theme.radius.md}; + + &::before, + &::after { + content: ''; + position: absolute; + inset: 0; + z-index: 2; + box-sizing: border-box; + border-radius: ${p => p.theme.radius.md}; + pointer-events: none; + } + + &::after { + z-index: 2; + border: ${BORDER_WIDTH}px solid ${p => p.theme.tokens.border.success.moderate}; + } + + &::before { + z-index: 3; + width: clamp( + ${BORDER_WIDTH}px, + var(--divider-position, 50%), + calc(100% - ${BORDER_WIDTH}px) + ); + border: ${BORDER_WIDTH}px solid ${p => p.theme.tokens.border.danger.moderate}; + border-right-width: 0; + border-radius: ${p => p.theme.radius.md} 0 0 ${p => p.theme.radius.md}; + } +`; + +export const SnapshotWipeFrame = styled('div')` + position: relative; + z-index: 1; + flex-shrink: 0; +`; + +export const SnapshotWipeImage = styled('img')` + display: block; + width: 100%; + height: 100%; + object-fit: contain; +`;