wip: feedback screenshot annotations #11175
Conversation
size-limit report 📦
|
| const gray100 = '#F0ECF3'; | ||
|
|
||
| style.textContent = ` | ||
| .canvas { |
There was a problem hiding this comment.
I'd be careful with these class names as they can conflict with other css we are injecting into the shadow dom. I tried to use BEM naming convention for the other components
There was a problem hiding this comment.
thanks for bringing that up, we would need to modify this for screenshots & cropping too!
| const surface200 = '#FAF9FB'; | ||
| const gray100 = '#F0ECF3'; |
There was a problem hiding this comment.
Let's be consistent with file naming, ScreenshotEditor vs imageEditorWrapper (I think we should have it match the exported component)
| export interface Rect { | ||
| height: number; | ||
| width: number; | ||
| x: number; | ||
| y: number; | ||
| } |
| const maxWidth = containerElement.getBoundingClientRect().width; | ||
| const maxHeight = containerElement.getBoundingClientRect().height; |
There was a problem hiding this comment.
Let's have an intermediate var to store getBoundingClientRect() so it's only called once
| useEffect(() => { | ||
| resizeCanvas(); | ||
| WINDOW.addEventListener('resize', resizeCanvas); | ||
| return () => { | ||
| WINDOW.removeEventListener('resize', resizeCanvas); | ||
| }; | ||
| }, [resizeCanvas]); |
There was a problem hiding this comment.
You'll want to throttle or debounce this otherwise many resize events end up getting fired when the window resizes and it will slowdown the browser due to the number of events it has to handle and forced reflows inside of resizeCanvas
|
@c298lee I just came across this PR through triage party. Do you want to continue working on this or are you fine if we close it? |
|
@Lms24 Let's backlog it for now, in the future, this might get picked up again! |
DO NOT MERGE:
This is currently all code copied over from the hackweek project and will be modified. This is just a reference for a working version of annotations
Screen.Recording.2024-03-15.at.1.01.55.PM.mov
Relates to getsentry/sentry#63749