Skip to content

Conversation

@mydea
Copy link
Member

@mydea mydea commented Jul 19, 2023

Not sure if this actually fixes the problem with sometimes incorrect click count, but it def. makes sense to have this safeguard, I'd say.

@mydea mydea added Type: Bug Package: replay Issues related to the Sentry Replay SDK labels Jul 19, 2023
@mydea mydea requested a review from billyvg July 19, 2023 15:15
@mydea mydea self-assigned this Jul 19, 2023
@github-actions
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 21.95 KB (-0.01% 🔽)
@sentry/browser - ES5 CDN Bundle (minified) 69.13 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 20.28 KB (-0.01% 🔽)
@sentry/browser - ES6 CDN Bundle (minified) 60.38 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 21.9 KB (0%)
@sentry/browser - Webpack (minified) 71.51 KB (0%)
@sentry/react - Webpack (gzipped + minified) 21.92 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 50.76 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 30.33 KB (+0.01% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 28.2 KB (0%)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 49.45 KB (+0.03% 🔺)
@sentry/replay - Webpack (gzipped + minified) 43.18 KB (+0.05% 🔺)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 69.58 KB (+0.03% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 61.86 KB (+0.04% 🔺)


const newClick: Click = {
timestamp: breadcrumb.timestamp,
timestamp: timestampToS(breadcrumb.timestamp),
Copy link
Member

Choose a reason for hiding this comment

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

should we use full name here?

Suggested change
timestamp: timestampToS(breadcrumb.timestamp),
timestamp: timestampToSeconds(breadcrumb.timestamp),

Copy link
Member Author

Choose a reason for hiding this comment

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

I used this to be parallel to timestampToMs, but 🤷 no strong feelings? WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah ToS just looks a bit funny to me, nbd

@mydea mydea merged commit 96fdbf4 into develop Jul 20, 2023
@mydea mydea deleted the fn/multi-click-guard branch July 20, 2023 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Package: replay Issues related to the Sentry Replay SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants