feat(feedback): Revamp of user feedback screenshot editing #15424

c298lee · 2025-02-14T21:44:22Z

DO NOT MERGE

Allows you to edit screenshots with highlight and hide boxes, with the ability to delete individual boxes and resize the page. This will replace the previous cropping and pen annotation.

Closes getsentry/sentry#69786

github-actions · 2025-02-14T21:49:06Z

size-limit report 📦

Path	Size	% Change	Change
@sentry/browser	23.05 KB	-	-
@sentry/browser - with treeshaking flags	22.84 KB	-	-
@sentry/browser (incl. Tracing)	36.07 KB	-	-
@sentry/browser (incl. Tracing, Replay)	73.08 KB	-	-
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags	66.55 KB	-	-
@sentry/browser (incl. Tracing, Replay with Canvas)	77.34 KB	-	-
@sentry/browser (incl. Tracing, Replay, Feedback)	89.69 KB	-0.66%	-608 B 🔽
@sentry/browser (incl. Feedback)	39.62 KB	-1.46%	-600 B 🔽
@sentry/browser (incl. sendFeedback)	27.68 KB	-0.01%	-1 B 🔽
@sentry/browser (incl. FeedbackAsync)	32.46 KB	-0.06%	-17 B 🔽
@sentry/react	24.87 KB	-	-
@sentry/react (incl. Tracing)	37.95 KB	-	-
@sentry/vue	27.23 KB	-	-
@sentry/vue (incl. Tracing)	37.76 KB	-	-
@sentry/svelte	23.09 KB	-	-
CDN Bundle	24.25 KB	-	-
CDN Bundle (incl. Tracing)	36.1 KB	-	-
CDN Bundle (incl. Tracing, Replay)	70.94 KB	-	-
CDN Bundle (incl. Tracing, Replay, Feedback)	76.1 KB	-0.01%	-6 B 🔽
CDN Bundle - uncompressed	70.91 KB	-	-
CDN Bundle (incl. Tracing) - uncompressed	107.17 KB	-	-
CDN Bundle (incl. Tracing, Replay) - uncompressed	218.45 KB	-	-
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed	230.99 KB	-0.02%	-33 B 🔽
@sentry/nextjs (client)	38.94 KB	-	-
@sentry/sveltekit (client)	36.48 KB	-	-
@sentry/node	129.25 KB	-	-
@sentry/node - without tracing	98.03 KB	-	-
@sentry/aws-serverless	107.45 KB	-	-

View base workflow run

codecov · 2025-02-14T21:50:02Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
4638	1	4637	323

View the top 1 failed test(s) by shortest run time

should report ANR when event loop blocked worker can be stopped and restarted

Stack Traces | 15s run time

Error: thrown: "Exceeded timeout of 15000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."
    at .../suites/anr/test.ts:204:3
    at _dispatchDescribe (.../jest-circus/build/index.js:98:26)
    at describe (.../jest-circus/build/index.js:60:5)
    at Object.<anonymous> (.../suites/anr/test.ts:109:1)
    at Runtime._execModule (.../jest-runtime/build/index.js:1646:24)
    at Runtime._loadModule (.../jest-runtime/build/index.js:1185:12)
    at Runtime.requireModule (.../jest-runtime/build/index.js:1009:12)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:13)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at runTestInternal (.../jest-runner/build/runTest.js:389:16)

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

packages/feedback/src/screenshot/components/ScreenshotEditorv2.tsx

packages/feedback/src/screenshot/components/Toolbarv2.tsx

packages/feedback/src/screenshot/components/ScreenshotInput.css.ts

packages/feedback/src/screenshot/components/ScreenshotEditorv2.tsx

packages/feedback/src/screenshot/components/Toolbarv2.tsx

ryan953 · 2025-02-20T22:50:42Z

packages/feedback/src/screenshot/components/ScreenshotInput.css.ts

@@ -131,12 +132,34 @@ export function createScreenshotInputStyles(styleNonce?: string): HTMLStyleEleme
  border: var(--button-border, var(--border));
  border-radius: var(--button-border-radius, 6px);
  background: var(--button-background, var(--background));
-  color: var(--button-foreground, var(--foreground));


was this wrong before, not matching docs? now it's fixed?

I think when I added the annotation stuff, I mixed up the names and used the wrong one. This is technically not in the docs, but now it matches what's used in the form buttons

ryan953 · 2025-02-20T22:50:45Z

packages/feedback/src/screenshot/components/ScreenshotInput.css.ts

 }

 .editor__tool--active {
  background: var(--button-primary-background, var(--accent-background));
-  color: var(--button-primary-foreground, var(--accent-foreground));


was this wrong before, not matching docs? now it's fixed?

ryan953 · 2025-02-20T22:52:04Z

packages/feedback/src/screenshot/components/ScreenshotEditorv2.tsx

+  if (sentryFeedback) {
+    strokeColor =
+      getComputedStyle(sentryFeedback).getPropertyValue('--button-primary-background') ||
+      getComputedStyle(sentryFeedback).getPropertyValue('--accent-background');
+  }


Suggested change

if (sentryFeedback) {

strokeColor =

getComputedStyle(sentryFeedback).getPropertyValue('--button-primary-background') ||

getComputedStyle(sentryFeedback).getPropertyValue('--accent-background');

}

if (sentryFeedback) {

const computedStyle = getComputedStyle(sentryFeedback);

strokeColor =

computedStyle.getPropertyValue('--button-primary-background') ||

computedStyle.getPropertyValue('--accent-background');

}

also, not sure if this is the best way to do this, but it works

billyvg

mostly nits about naming/comments

packages/feedback/src/screenshot/components/ScreenshotEditor.tsx

billyvg · 2025-02-21T20:51:53Z

packages/feedback/src/screenshot/components/ScreenshotEditor.tsx

-            <Annotations action={action} imageBuffer={imageBuffer} annotatingRef={annotatingRef} />
+          <div class="editor__canvas-container" ref={measurementRef}>
+            <canvas ref={screenshotRef}></canvas>
+            <canvas class="editor__canvas-annotate" ref={graywashRef} onMouseDown={handleMouseDown}></canvas>


It would be nice if the class names and ref name were more aligned. Also, graywash makes me think it's only used to draw the backdrop, but handleMouseDown looks like it's also handling the drawing of the boxes/rects. maybe this doesn't matter as much if we had some code comments about what the different refs are do.

packages/feedback/src/screenshot/components/ScreenshotEditor.tsx

billyvg · 2025-02-21T20:58:16Z

packages/feedback/src/screenshot/components/ScreenshotEditor.tsx

+      };
+
+      const handleMouseUp = (e: MouseEvent): void => {
+        setCurrentRect(undefined);


can you add a comment why we're setting current rect as undefined?

billyvg · 2025-02-21T21:01:50Z

packages/feedback/src/screenshot/components/ScreenshotEditor.tsx

+      }
+    }, [currentRect]);
+
+    const drawBuffer = hooks.useCallback((): void => {


What's the purpose of the buffer?

updated with a comment

packages/feedback/src/screenshot/components/ScreenshotEditor.tsx

billyvg · 2025-02-21T21:05:44Z

packages/feedback/src/screenshot/components/ScreenshotEditor.tsx

+    const [drawCommands, setDrawCommands] = hooks.useState<Rect[]>([]);
+    const [currentRect, setCurrentRect] = hooks.useState<Rect | undefined>(undefined);
+    const measurementRef = hooks.useRef<HTMLDivElement>(null);
+    const screenshotRef = hooks.useRef<HTMLCanvasElement>(null);
+    const graywashRef = hooks.useRef<HTMLCanvasElement>(null);
+    const rectDivRef = hooks.useRef<HTMLDivElement>(null);
+    const [imageSource, setimageSource] = hooks.useState<HTMLCanvasElement | null>(null);
+    const [displayEditor, setdisplayEditor] = hooks.useState<boolean>(true);
+    const [scaleFactor, setScaleFactor] = hooks.useState<number>(1);


It might help to have comments explaining the purposes these (where it makes sense)

e.g. what is the purpose of the graywashRef

billyvg · 2025-02-21T21:06:19Z

packages/feedback/src/screenshot/components/ScreenshotEditor.tsx

+
+      // draws outline around rectangle in the same colour as the submit button
+      let strokeColor;
+      const sentryFeedback = DOCUMENT.getElementById('sentry-feedback');


is sentry-feedback id always guaranteed to exist?

It's the id attribute of the div that contains the feedback widget. Good catch tho, this can actually be customized

Allows you to edit screenshots with highlight and hide boxes, with the ability to delete individual boxes and resize the page. This will replace the previous cropping and pen annotation. ![image](https://github.com/user-attachments/assets/9cf9dbfa-b65b-47fc-8933-bafe33687eb8) Related to getsentry/sentry#69786

wip: need to fix border & resizing

c6a13d8

c298lee added 6 commits February 17, 2025 01:02

Merge branch 'develop' into cl/annotations-v2

b52d5a7

buggy resizing & screenshotting

2c0fd8c

back to prev drawBuffer plus move scaling out

50ca3ea

shrinking works

0ba53a6

working resize & delete

8908648

no bugs hopefully

dc273c3