Commit 2c1c189
feat(feedback): Implement highlighting and hiding controls for screenshots (#15951)
| Editing | In Sentry |
| --- | --- |
| <img width="1408" alt="SCR-20250402-pfuj"
src="https://github.com/user-attachments/assets/896a1efe-74ac-42ca-8f51-b7024dee768c"
/> | <img width="1421" alt="SCR-20250402-pgft"
src="https://github.com/user-attachments/assets/37337f95-9e4d-477f-935b-2545466bc455"
/>
I should write that there are like 3 separate systems inside the
`ScreenshotEditor` component, all working with the same data and helpers
really, but each easy to spot and understand on their own.
The end-goal is to have the correct thing rendered into `outputBuffer`
(which is an argument to the component, but it's used for outputting
data) and also render into the `foreground` and `background`.
1. The first is the `useLayoutEffect` and `useEffect` calls. These
basically depend on the `screenshot` prop & `drawCommands` state. When
those change we're basically re-sizing and re-rendering into our
canvases.
2. The second system is all the stuff inside of `handleMouseDown`. We
call `getBoundingClientRect` and use offsetX/offsetY for a consistent
frame if reference when doing mouse coordinates. From in here we don't
update any react state until onMouseUp happens, so the re-rendering
onMouseMove is really fast to execute, and really easy to understand.
3. The last system to look at is is the `drawCommands` stuff and
`scaleFactor`. All the drawCommands in state/memory are using an x/y
coordinate system based on the size of the original screenshot. So when
we call paintForeground it's really easy. But when we want to add
`<div>` elements to the page we have to scale the sizes. This helps with
window resize, and helps to consolidate when/where we convert from one
x/y coordinate system to another.
The resulting feedbacks are in the correct dpi, and have boxes draw
using the same DPI.
| Case | Img |
| --- | --- |
| No boxes drawn ever | <img width="485" alt="SCR-20250402-jidz"
src="https://github.com/user-attachments/assets/3f5d177a-4652-40c1-be89-bfa2ff4f7399"
/> |
| Drew some, then removed them | <img width="452"
alt="SCR-20250402-jifp"
src="https://github.com/user-attachments/assets/c24fc144-d0f1-4b64-af6c-199eff0a3fee"
/> |
| Some boxes | <img width="452" alt="SCR-20250402-jigm"
src="https://github.com/user-attachments/assets/a3af82b1-16a8-4a78-bac4-5d83f687a59f"
/> |
Related to #15424
Iteration on top of
develop...ryan953/feedback-highlight-mask-ref
Fixes getsentry/sentry#69786
---------
Co-authored-by: Billy Vong <[email protected]>1 parent 35e78c6 commit 2c1c189
File tree
13 files changed
+404
-744
lines changed- packages
- core/src/types-hoist/feedback
- feedback/src
- core
- screenshot
- components
13 files changed
+404
-744
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | 60 | | |
70 | 61 | | |
71 | 62 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
84 | 84 | | |
85 | 85 | | |
86 | 86 | | |
87 | | - | |
88 | 87 | | |
89 | 88 | | |
90 | 89 | | |
| |||
159 | 158 | | |
160 | 159 | | |
161 | 160 | | |
162 | | - | |
163 | | - | |
164 | 161 | | |
165 | 162 | | |
166 | 163 | | |
| |||
Lines changed: 0 additions & 91 deletions
This file was deleted.
0 commit comments