Skip to content

Commit

Permalink
fix(feedback): Smoother cropping experience and better UI (getsentry#…
Browse files Browse the repository at this point in the history 
…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 getsentry/sentry#67056
  • Loading branch information
@c298lee @cadesalaberry
c298lee authored and cadesalaberry committed Apr 19, 2024
1 parent 6873adb commit dd9397f
Showing 1 changed file with 49 additions and 37 deletions.
86 changes: 49 additions & 37 deletions packages/feedback/src/screenshot/components/ScreenshotEditor.tsx
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand All @@ -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 {
Expand All @@ -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),
};
};

Expand All @@ -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
Expand All @@ -62,7 +66,7 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor
const canvasContainerRef = useRef<HTMLDivElement>(null);
const cropContainerRef = useRef<HTMLDivElement>(null);
const croppingRef = useRef<HTMLCanvasElement>(null);
const [croppingRect, setCroppingRect] = useState<Box>({ startx: 0, starty: 0, endx: 0, endy: 0 });
const [croppingRect, setCroppingRect] = useState<Box>({ startX: 0, startY: 0, endX: 0, endY: 0 });
const [confirmCrop, setConfirmCrop] = useState(false);

useEffect(() => {
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -229,33 +240,33 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor
<div class="cropButtonContainer" style={{ position: 'absolute' }} ref={cropContainerRef}>
<canvas style={{ position: 'absolute' }} ref={croppingRef}></canvas>
<CropCorner
left={croppingRect.startx}
top={croppingRect.starty}
left={croppingRect.startX}
top={croppingRect.startY}
onGrabButton={onGrabButton}
corner="topleft"
></CropCorner>
<CropCorner
left={croppingRect.endx - 30}
top={croppingRect.starty}
left={croppingRect.endX - CROP_BUTTON_SIZE}
top={croppingRect.startY}
onGrabButton={onGrabButton}
corner="topright"
></CropCorner>
<CropCorner
left={croppingRect.startx}
top={croppingRect.endy - 30}
left={croppingRect.startX}
top={croppingRect.endY - CROP_BUTTON_SIZE}
onGrabButton={onGrabButton}
corner="bottomleft"
></CropCorner>
<CropCorner
left={croppingRect.endx - 30}
top={croppingRect.endy - 30}
left={croppingRect.endX - CROP_BUTTON_SIZE}
top={croppingRect.endY - CROP_BUTTON_SIZE}
onGrabButton={onGrabButton}
corner="bottomright"
></CropCorner>
<div
style={{
left: Math.max(0, croppingRect.endx - 191),
top: Math.max(0, croppingRect.endy + 8),
left: Math.max(0, croppingRect.endX - 191),
top: Math.max(0, croppingRect.endY + 8),
display: confirmCrop ? 'flex' : 'none',
}}
class="crop-btn-group"
Expand All @@ -265,10 +276,10 @@ export function makeScreenshotEditorComponent({ h, imageBuffer, dialog }: Factor
e.preventDefault();
if (croppingRef.current) {
setCroppingRect({
startx: 0,
starty: 0,
endx: croppingRef.current.width,
endy: croppingRef.current.height,
startX: 0,
startY: 0,
endX: croppingRef.current.width,
endY: croppingRef.current.height,
});
}
setConfirmCrop(false);
Expand Down Expand Up @@ -316,7 +327,8 @@ function CropCorner({
borderLeft: corner === 'topleft' || corner === 'bottomleft' ? 'solid purple' : 'none',
borderRight: corner === 'topright' || corner === 'bottomright' ? 'solid purple' : 'none',
borderBottom: corner === 'bottomleft' || corner === 'bottomright' ? 'solid purple' : 'none',
borderWidth: '3px',
borderWidth: `${CROP_BUTTON_BORDER}px`,
cursor: corner === 'topleft' || corner === 'bottomright' ? 'nwse-resize' : 'nesw-resize',
}}
onMouseDown={e => {
e.preventDefault();
Expand Down

0 comments on commit dd9397f

Please sign in to comment.