From dd9397f11426950c3ff9c6f19e2608139f55b082 Mon Sep 17 00:00:00 2001 From: Catherine Lee <55311782+c298lee@users.noreply.github.com> Date: Mon, 18 Mar 2024 15:57:09 -0400 Subject: [PATCH] fix(feedback): Smoother cropping experience and better UI (#11165) Fixes some cropping jank: 1. Cropping is smoother: previously the event listeners weren't working when the mouse was on the cropping buttons, which was causing the jankiness 2. Changes the cursor when on the cropping buttons 3. Increased the minimum size required for cropping so it's easier to resize https://github.com/getsentry/sentry-javascript/assets/55311782/71b7401e-c9f2-44c2-9f92-64c1c80d24ef Fixes https://github.com/getsentry/sentry/issues/67056 --- .../components/ScreenshotEditor.tsx | 86 +++++++++++-------- 1 file changed, 49 insertions(+), 37 deletions(-) diff --git a/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx b/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx index d3753631ec1c..a698f388b7c0 100644 --- a/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx +++ b/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx @@ -1,4 +1,4 @@ -// eslint-disable max-lines +/* eslint-disable max-lines */ import type { ComponentType, VNode, h as hType } from 'preact'; // biome-ignore lint: needed for preact import { h } from 'preact'; // eslint-disable-line @typescript-eslint/no-unused-vars @@ -8,6 +8,10 @@ import type { Dialog } from '../../types'; import { createScreenshotInputStyles } from './ScreenshotInput.css'; import { useTakeScreenshot } from './useTakeScreenshot'; +const CROP_BUTTON_SIZE = 30; +const CROP_BUTTON_BORDER = 3; +const CROP_BUTTON_OFFSET = CROP_BUTTON_SIZE + CROP_BUTTON_BORDER; + interface FactoryParams { h: typeof hType; imageBuffer: HTMLCanvasElement; @@ -19,10 +23,10 @@ interface Props { } interface Box { - startx: number; - starty: number; - endx: number; - endy: number; + startX: number; + startY: number; + endX: number; + endY: number; } interface Rect { @@ -34,10 +38,10 @@ interface Rect { const constructRect = (box: Box): Rect => { return { - x: Math.min(box.startx, box.endx), - y: Math.min(box.starty, box.endy), - width: Math.abs(box.startx - box.endx), - height: Math.abs(box.starty - box.endy), + x: Math.min(box.startX, box.endX), + y: Math.min(box.startY, box.endY), + width: Math.abs(box.startX - box.endX), + height: Math.abs(box.startY - box.endY), }; }; @@ -51,7 +55,7 @@ const getContainedSize = (img: HTMLCanvasElement): Box => { } const x = (img.clientWidth - width) / 2; const y = (img.clientHeight - height) / 2; - return { startx: x, starty: y, endx: width + x, endy: height + y }; + return { startX: x, startY: y, endX: width + x, endY: height + y }; }; // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -62,7 +66,7 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor const canvasContainerRef = useRef(null); const cropContainerRef = useRef(null); const croppingRef = useRef(null); - const [croppingRect, setCroppingRect] = useState({ startx: 0, starty: 0, endx: 0, endy: 0 }); + const [croppingRect, setCroppingRect] = useState({ startX: 0, startY: 0, endX: 0, endY: 0 }); const [confirmCrop, setConfirmCrop] = useState(false); useEffect(() => { @@ -85,7 +89,7 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor cropButton.style.top = `${imageDimensions.y}px`; } - setCroppingRect({ startx: 0, starty: 0, endx: imageDimensions.width, endy: imageDimensions.height }); + setCroppingRect({ startX: 0, startY: 0, endX: imageDimensions.width, endY: imageDimensions.height }); } useEffect(() => { @@ -118,44 +122,51 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor setConfirmCrop(false); const handleMouseMove = makeHandleMouseMove(corner); const handleMouseUp = (): void => { - croppingRef.current && croppingRef.current.removeEventListener('mousemove', handleMouseMove); + DOCUMENT.removeEventListener('mousemove', handleMouseMove); DOCUMENT.removeEventListener('mouseup', handleMouseUp); setConfirmCrop(true); }; DOCUMENT.addEventListener('mouseup', handleMouseUp); - croppingRef.current && croppingRef.current.addEventListener('mousemove', handleMouseMove); + DOCUMENT.addEventListener('mousemove', handleMouseMove); } const makeHandleMouseMove = useCallback((corner: string) => { return function (e: MouseEvent) { + if (!croppingRef.current) { + return; + } + const cropCanvas = croppingRef.current; + const cropBoundingRect = cropCanvas.getBoundingClientRect(); + const mouseX = e.clientX - cropBoundingRect.x; + const mouseY = e.clientY - cropBoundingRect.y; switch (corner) { case 'topleft': setCroppingRect(prev => ({ ...prev, - startx: Math.min(e.offsetX, prev.endx - 30), - starty: Math.min(e.offsetY, prev.endy - 30), + startX: Math.min(Math.max(0, mouseX), prev.endX - CROP_BUTTON_OFFSET), + startY: Math.min(Math.max(0, mouseY), prev.endY - CROP_BUTTON_OFFSET), })); break; case 'topright': setCroppingRect(prev => ({ ...prev, - endx: Math.max(e.offsetX, prev.startx + 30), - starty: Math.min(e.offsetY, prev.endy - 30), + endX: Math.max(Math.min(mouseX, cropCanvas.width), prev.startX + CROP_BUTTON_OFFSET), + startY: Math.min(Math.max(0, mouseY), prev.endY - CROP_BUTTON_OFFSET), })); break; case 'bottomleft': setCroppingRect(prev => ({ ...prev, - startx: Math.min(e.offsetX, prev.endx - 30), - endy: Math.max(e.offsetY, prev.starty + 30), + startX: Math.min(Math.max(0, mouseX), prev.endX - CROP_BUTTON_OFFSET), + endY: Math.max(Math.min(mouseY, cropCanvas.height), prev.startY + CROP_BUTTON_OFFSET), })); break; case 'bottomright': setCroppingRect(prev => ({ ...prev, - endx: Math.max(e.offsetX, prev.startx + 30), - endy: Math.max(e.offsetY, prev.starty + 30), + endX: Math.max(Math.min(mouseX, cropCanvas.width), prev.startX + CROP_BUTTON_OFFSET), + endY: Math.max(Math.min(mouseY, cropCanvas.height), prev.startY + CROP_BUTTON_OFFSET), })); break; } @@ -229,33 +240,33 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor
{ e.preventDefault();