feat: Add breadcrumbs when visibility changes#94
Conversation
This also changes the timeout of a session when page is hidden from 15 seconds to the session timeout of 15 minutes. This will reduce the number of short-lived sessions when users tab around, as well as make the timeout more consistent.
| }, | ||
| "volta": { | ||
| "node": "14.19.0", | ||
| "node": "16.15.1", |
src/index.ts
Outdated
| // Not all blurs will result in `document.visibilityState` being hidden. | ||
| // e.g. opening a new window, but new window does not completely overlap | ||
| // previous window | ||
| message: `Page is ${document.visibilityState}`, |
There was a problem hiding this comment.
I think this should be:
| message: `Page is ${document.visibilityState}`, | |
| message: `Page is blurred`, |
The way it is right now we could have a breadcrumb list like this, where the first two events would be really confusing to read:
[
[ ui.blur, "Page is visible" ],
...
[ ui.focus, "Page is visible" ],
...
[ ui.change_visibility, "Page is hidden" ],
...
[ ui.blur, "Page is hidden" ],
[ ui.change_visibility, "Page is visible" ],
...
[ ui.focus, "Page is hidden" ],
]
There was a problem hiding this comment.
I agree it's a bit noisy and confusing, but the data itself can be interesting (seeing that a user switches away from the window but with content still in view?) -- it's likely this will be more confusing than helpful, in which case we should just kill the message completely.
There was a problem hiding this comment.
talked offline, killing the message seems easiest because we still have the breadcrumb category which turns into a title. It's not ambiguous.
Also, I guess, translations are easier if we don't rely on the sdk to make user friendly text.
src/index.ts
Outdated
| // Not all blurs will result in `document.visibilityState` being hidden. | ||
| // e.g. opening a new window, but new window does not completely overlap | ||
| // previous window | ||
| message: `Page is ${document.visibilityState}`, |
There was a problem hiding this comment.
talked offline, killing the message seems easiest because we still have the breadcrumb category which turns into a title. It's not ambiguous.
Also, I guess, translations are easier if we don't rely on the sdk to make user friendly text.
| import { SentryReplay } from '@'; | ||
| import { | ||
| SESSION_IDLE_DURATION, | ||
| VISIBILITY_CHANGE_TIMEOUT, |
There was a problem hiding this comment.
keep this import because the fix is split out into #98 ?
| record.takeFullSnapshot(true); | ||
| } | ||
|
|
||
| updateLastActivity(lastActivity: number = new Date().getTime()) { |
There was a problem hiding this comment.
couldn't set that default in python!
We handled the blur case but not the focus case in #94
…y#94) Adds breadcrumbs when user blurs/focuses the page. 
This also changes the timeout of a session when page is hidden from
15 seconds to the session timeout of 15 minutes. This will reduce the
number of short-lived sessions when users tab around, as well as make
the timeout more consistent.