-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
feat(replay): create a wrapper class to init rrweb player alongside video replayer #69927
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
Scope: Frontend
Automatically applied to PRs that change frontend components
label
Apr 29, 2024
Bundle ReportChanges will increase total bundle size by 261.05kB ⬆️
|
michellewzhang
changed the title
feat(replays): create a wrapper class to init rrweb player alongside video replayer
feat(replay): create a wrapper class to init rrweb player alongside video replayer
May 6, 2024
billyvg
reviewed
May 7, 2024
billyvg
reviewed
May 7, 2024
billyvg
reviewed
May 7, 2024
michellewzhang
commented
May 7, 2024
michellewzhang
force-pushed
the
mz/wrapper-class
branch
from
May 8, 2024 04:11
0e37dbd
to
0554259
Compare
billyvg
reviewed
May 8, 2024
billyvg
reviewed
May 8, 2024
static/app/components/replays/videoReplayerWithInteractions.tsx
Outdated
Show resolved
Hide resolved
billyvg
approved these changes
May 8, 2024
static/app/components/replays/videoReplayerWithInteractions.tsx
Outdated
Show resolved
Hide resolved
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #69817
We have a
videoReplayer
, which uses video events to create the replay playback for mobile replays. However, in order to see the gestures (clicks, mouse movements, etc) we need to initialize an rrweb player too (the one that web replay uses). This PR introduces avideoReplayerWithInteractions
classe which initializes both, so that mobile replays can utilize both players at once.Another key change we had to make was introducing a fake full snapshot event after every meta event in order to trick the rrweb player into thinking we had a node to map the mouse movement to. The rrweb player essentially fails to render any gesture if it doesn't find an element with a matching
id
to theid
inside thepositions
array (see below picture), so we hardcoded the event to haveid: 0
(which is what the SDK is sending for the mobile rrweb events). This workaround should be safe to do since the full snapshot event doesn't affect the video playback at all.Adding a snapshot event after every meta event also fixes the scrubbing bugs we were experiencing.
Fixing mousetails not showing up involved absolutely positioning the
replayer-wrapper
.Screen.Recording.2024-05-08.at.12.03.23.AM.mov