Skip to content

fix: Wait for the correct clientWidth/clientHeight when showing Feedback Screenshot previews #16648

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 1 commit into from
Jun 20, 2025

Conversation

ryan953
Copy link
Member

@ryan953 ryan953 commented Jun 18, 2025

Before we would see a blank screenshot in the feedback modal when using firefox. After resizing the screenshot would appear.

I debugged and foundout that the clientWidth/clientHeight wasn't being set right away, therefore the elements were on the page properly, and contained the correct pixel information drawn into the canvas, but the canvas wasn't sized correctly on the page, so you couldn't see anything.

This triggers a resize after a tick, giving the browser a chance first set the width/height before we measure it and set the scale factor.

Before:
SCR-20250618-kgwr

After:

SCR-20250618-kgrm

Fixes REPLAY-420

@ryan953 ryan953 requested a review from a team as a code owner June 18, 2025 18:24
Copy link
Contributor

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.99 kB - -
@sentry/browser - with treeshaking flags 23.76 kB - -
@sentry/browser (incl. Tracing) 38.8 kB - -
@sentry/browser (incl. Tracing, Replay) 76.93 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 70.02 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 81.69 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 93.78 kB +0.03% +20 B 🔺
@sentry/browser (incl. Feedback) 40.74 kB +0.05% +18 B 🔺
@sentry/browser (incl. sendFeedback) 28.7 kB - -
@sentry/browser (incl. FeedbackAsync) 33.59 kB - -
@sentry/react 25.76 kB - -
@sentry/react (incl. Tracing) 40.79 kB - -
@sentry/vue 28.36 kB - -
@sentry/vue (incl. Tracing) 40.67 kB - -
@sentry/svelte 24.01 kB - -
CDN Bundle 25.5 kB - -
CDN Bundle (incl. Tracing) 38.88 kB - -
CDN Bundle (incl. Tracing, Replay) 74.79 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 80.25 kB - -
CDN Bundle - uncompressed 74.5 kB - -
CDN Bundle (incl. Tracing) - uncompressed 115.32 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 229.37 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 242.2 kB - -
@sentry/nextjs (client) 42.43 kB - -
@sentry/sveltekit (client) 39.29 kB - -
@sentry/node 150.76 kB - -
@sentry/node - without tracing 98.55 kB - -
@sentry/aws-serverless 124.3 kB - -

View base workflow run

@ryan953 ryan953 requested a review from a team June 20, 2025 18:29
Copy link
Member

@billyvg billyvg left a comment

Choose a reason for hiding this comment

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

is there a relevant event we could listen for instead of relying on next tick?

@ryan953
Copy link
Member Author

ryan953 commented Jun 20, 2025

is there a relevant event we could listen for instead of relying on next tick?

It's using useLayoutEffect which i was assuming would delay things enough so that the width/height of the dom node is set... but that's not the case. The docs say that it runs "after all DOM mutations but before the browser has a chance to paint".
Maybe calling something like getBoundingClientRect would force it to measure things and return the correct width/heights right away, but what ends up happening with the solution i have is that we run handleResize a few times in a row.

@ryan953 ryan953 merged commit 797ebd1 into develop Jun 20, 2025
130 checks passed
@ryan953 ryan953 deleted the ryan953/fix-feedback-resize branch June 20, 2025 18:41
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.

2 participants