-
Notifications
You must be signed in to change notification settings - Fork 24
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
recording: add screenshot
option for session replay instead of wireframe
#127
Conversation
thread.quit() | ||
} | ||
|
||
return RRWireframe( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if something goes wrong, it means it's gonna be an empty image so the screen will be back blank/white (or a placeholder?).
the other option is to not send the mutation update at all so it will keep the last image in the replay, technically better but wrong?
screenshot
option for session replay instead of wireframe
|
||
try { | ||
var success = false | ||
PixelCopy.request(window, bitmap, { copyResult -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not found anything faster than that as discussed in the daily.
// await for 1s max | ||
latch.await(1000, TimeUnit.MILLISECONDS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where the thread waits for the copying to finish, 1s is a lot but it should never take that long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing how little code is involved! Clever to reuse the image from wireframe mode
base64 = bitmap.base64() | ||
} | ||
} catch (e: Throwable) { | ||
config.logger.log("Session Replay PixelCopy failed: $e.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could add a custom event to the recording here so that we see when this is happening
(maybe with a debounce/throttle so we don't spam ourselves)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Defaults to 500ms | ||
*/ | ||
@PostHogExperimental | ||
public var debouncerDelayMs: Long = 500, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could nitpick and suggest we call this frames per second (so debounce delay of 500ms === an FPS of 2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but no strong opinion
💡 Motivation and Context
Adds a screenshot with high-fidelity UI instead of the wireframe option.
Adds more overhead to the app.
Enables Session replay for apps using Jetpack Compose.
The mutation updates seem to be buggy, waiting for @pauldambra so we can pair up on that, won't merge before this.
💚 How did you test it?
Running the sample and some open source apps
📝 Checklist