Skip to content

feat(user feedback): Create pen tool button for uf annotations #15102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 21, 2025

Conversation

c298lee
Copy link
Contributor

@c298lee c298lee commented Jan 21, 2025

Adds a proper pen tool button. The icon for the button is from the hackweek project
Unselected:
image

Selected:
image

Relates to #15064

Copy link
Contributor

github-actions bot commented Jan 21, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 22.98 KB added added
@sentry/browser - with treeshaking flags 22.87 KB added added
@sentry/browser (incl. Tracing) 35.68 KB added added
@sentry/browser (incl. Tracing, Replay) 72.47 KB added added
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 66.03 KB added added
@sentry/browser (incl. Tracing, Replay with Canvas) 76.72 KB added added
@sentry/browser (incl. Tracing, Replay, Feedback) 89.22 KB added added
@sentry/browser (incl. Feedback) 39.67 KB added added
@sentry/browser (incl. sendFeedback) 27.61 KB added added
@sentry/browser (incl. FeedbackAsync) 32.39 KB added added
@sentry/react 25.66 KB added added
@sentry/react (incl. Tracing) 38.46 KB added added
@sentry/vue 27.04 KB added added
@sentry/vue (incl. Tracing) 37.43 KB added added
@sentry/svelte 23.11 KB added added
CDN Bundle 24.36 KB added added
CDN Bundle (incl. Tracing) 36 KB added added
CDN Bundle (incl. Tracing, Replay) 70.64 KB added added
CDN Bundle (incl. Tracing, Replay, Feedback) 75.8 KB added added
CDN Bundle - uncompressed 71.16 KB added added
CDN Bundle (incl. Tracing) - uncompressed 106.82 KB added added
CDN Bundle (incl. Tracing, Replay) - uncompressed 217.67 KB added added
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 230.23 KB added added
@sentry/nextjs (client) 38.58 KB added added
@sentry/sveltekit (client) 36.21 KB added added
@sentry/node 156.17 KB added added
@sentry/node - without tracing 97.32 KB added added
@sentry/aws-serverless 106.73 KB added added

Copy link

codecov bot commented Jan 21, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
690 1 689 297
View the top 1 failed tests by shortest run time
transactions.test.ts Should send a transaction for instrumented server actions
Stack Traces | 14s run time
transactions.test.ts:54:5 Should send a transaction for instrumented server actions

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@c298lee c298lee marked this pull request as ready for review January 21, 2025 14:46
@c298lee c298lee requested a review from a team as a code owner January 21, 2025 14:46
@c298lee c298lee requested review from ryan953 and billyvg January 21, 2025 17:48
@c298lee c298lee requested a review from ryan953 January 21, 2025 18:28
Comment on lines 79 to 80
const CropCorner = CropCornerFactory({ h });
const PenIcon = PenIconFactory({ h });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move these two lines up above the line return function ScreenshotEditor....

They should be in the same spot as the line const useTakeScreenshot = useTakeScreenshotFactory({ hooks });

setIsAnnotating(!isAnnotating);
}}
>
<PenIcon></PenIcon>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm surprised prettier doesn't clean this up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch!

@c298lee c298lee merged commit 26d7601 into cl/annotate-draw-tool Jan 21, 2025
5 checks passed
@c298lee c298lee deleted the cl/draw-tool-button branch January 21, 2025 18:38
c298lee added a commit that referenced this pull request Jan 21, 2025
…5062)

- Updates some cropping variable names to make it more clear that it's
only meant for cropping
- The pen tool button is improved in
#15102, which needs
to be merged first
- The drawing tool button needs to be "on" to annotate, but cropping can
happen at any time
- Once the button is "on", drawing happens on mouse down and mouse move,
and on mouse up, the drawing gets "squashed" onto the image. The
"squashing" can be moved to happen at a different time in a future PR if
we want to incorporate undo, selection, or erasing
- The experimental flag must be on to use annotations: `_experiments:
{annotations: true}`


https://github.com/user-attachments/assets/2fac5e56-5caf-454b-b8b3-afabbd2c31b9

Closes #15064

---------

Co-authored-by: Ryan Albrecht <ryan.albrecht@sentry.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants