address pr comments

Author: michellewzhang

static/app/components/replays/replayContext.tsx

@@ -15,7 +15,7 @@ import type {
   ReplayPrefs,
 } from 'sentry/components/replays/preferences/replayPreferences';
 import useReplayHighlighting from 'sentry/components/replays/useReplayHighlighting';
-import {WrapperReplayer} from 'sentry/components/replays/wrapperReplayer';
+import {VideoReplayerWithInteractions} from 'sentry/components/replays/videoReplayerWithInteractions';
 import {trackAnalytics} from 'sentry/utils/analytics';
 import clamp from 'sentry/utils/number/clamp';
 import type useInitialOffsetMs from 'sentry/utils/replays/hooks/useInitialTimeOffsetMs';
@@ -481,36 +481,35 @@ function ProviderNonMemo({
 
       // This is a wrapper class that wraps both the VideoReplayer
       // and the rrweb Replayer
-      const inst = new WrapperReplayer(
-        {videoEvents, events: events ?? []},
-        {
-          // video specific
-          videoApiPrefix: `/api/0/projects/${
-            organization.slug
-          }/${projectSlug}/replays/${replay?.getReplay().id}/videos/`,
-          start: startTimestampMs,
-          onFinished: setReplayFinished,
-          onLoaded: event => {
-            const {videoHeight, videoWidth} = event.target;
-            if (!videoHeight || !videoWidth) {
-              return;
-            }
-            setDimensions({
-              height: videoHeight,
-              width: videoWidth,
-            });
-          },
-          onBuffer: buffering => {
-            setVideoBuffering(buffering);
-          },
-          clipWindow,
-          durationMs,
-          // rrweb specific
-          theme,
-          // common to both
-          root,
-        }
-      );
+      const inst = new VideoReplayerWithInteractions({
+        // video specific
+        videoEvents,
+        videoApiPrefix: `/api/0/projects/${
+          organization.slug
+        }/${projectSlug}/replays/${replay?.getReplay().id}/videos/`,
+        start: startTimestampMs,
+        onFinished: setReplayFinished,
+        onLoaded: event => {
+          const {videoHeight, videoWidth} = event.target;
+          if (!videoHeight || !videoWidth) {
+            return;
+          }
+          setDimensions({
+            height: videoHeight,
+            width: videoWidth,
+          });
+        },
+        onBuffer: buffering => {
+          setVideoBuffering(buffering);
+        },
+        clipWindow,
+        durationMs,
+        // rrweb specific
+        theme,
+        events: events ?? [],
+        // common to both
+        root,
+      });
       // `.current` is marked as readonly, but it's safe to set the value from
       // inside a `useEffect` hook.
       // See: https://reactjs.org/docs/hooks-faq.html#is-there-something-like-instance-variables

static/app/components/replays/replayPlayer.tsx

@@ -283,6 +283,7 @@ const SentryPlayerRoot = styled(PlayerRoot)`
     }
   }
 
+  /* Correctly positions the canvas for video replays and show the purple "mousetails" */
   &.video-replayer {
     .replayer-wrapper {
       position: absolute;

static/app/components/replays/wrapperReplayer.tsx renamed to static/app/components/replays/videoReplayerWithInteractions.tsx

@@ -9,46 +9,47 @@ import type {ClipWindow, VideoEvent} from 'sentry/utils/replays/types';
 
 type RootElem = HTMLDivElement | null;
 
-interface WrapperReplayerOptions {
+interface VideoReplayerWithInteractionsOptions {
   durationMs: number;
+  events: eventWithTime[];
   onBuffer: (isBuffering: boolean) => void;
   onFinished: () => void;
   onLoaded: (event: any) => void;
   root: RootElem;
   start: number;
   theme: Theme;
   videoApiPrefix: string;
+  videoEvents: VideoEvent[];
   clipWindow?: ClipWindow;
 }
 
 /**
  * A wrapper replayer that wraps both VideoReplayer and the rrweb Replayer.
  * We need both instances in order to render the video playback alongside gestures.
  */
-export class WrapperReplayer {
+export class VideoReplayerWithInteractions {
   public config: VideoReplayerConfig = {
     skipInactive: false,
     speed: 1.0,
   };
   public iframe = {};
-  public videoInst: VideoReplayer;
-  public rrwebInst: Replayer;
+  public videoReplayer: VideoReplayer;
+  public replayer: Replayer;
 
-  constructor(
-    {videoEvents, events}: {events: eventWithTime[]; videoEvents: VideoEvent[]},
-    {
-      root,
-      start,
-      videoApiPrefix,
-      onBuffer,
-      onFinished,
-      onLoaded,
-      clipWindow,
-      durationMs,
-      theme,
-    }: WrapperReplayerOptions
-  ) {
-    this.videoInst = new VideoReplayer(videoEvents, {
+  constructor({
+    videoEvents,
+    events,
+    root,
+    start,
+    videoApiPrefix,
+    onBuffer,
+    onFinished,
+    onLoaded,
+    clipWindow,
+    durationMs,
+    theme,
+  }: VideoReplayerWithInteractionsOptions) {
+    this.videoReplayer = new VideoReplayer(videoEvents, {
       videoApiPrefix,
       root,
       start,
@@ -61,9 +62,9 @@ export class WrapperReplayer {
 
     root?.classList.add('video-replayer');
 
-    const modifiedEvents: eventWithTime[] = [];
+    const eventsWithSnapshots: eventWithTime[] = [];
     events.forEach((e, idx) => {
-      modifiedEvents.push(e);
+      eventsWithSnapshots.push(e);
       if (e.type === 4) {
         // Create a mock full snapshot event, in order to render rrweb gestures properly
         // Need to add one for every meta event we see
@@ -80,7 +81,6 @@ export class WrapperReplayer {
                   name: 'html',
                   publicId: '',
                   systemId: '',
-                  id: 0,
                 },
                 {
                   type: 2,
@@ -89,23 +89,18 @@ export class WrapperReplayer {
                     lang: 'en',
                   },
                   childNodes: [],
-                  id: 0,
                 },
               ],
               id: 0,
             },
-            initialOffset: {
-              left: 0,
-              top: 0,
-            },
           },
           timestamp: events[idx].timestamp,
         };
-        modifiedEvents.push(fullSnapshotEvent);
+        eventsWithSnapshots.push(fullSnapshotEvent);
       }
     });
 
-    this.rrwebInst = new Replayer(modifiedEvents, {
+    this.replayer = new Replayer(eventsWithSnapshots, {
       root: root as Element,
       blockClass: 'sentry-block',
       mouseTail: {
@@ -121,38 +116,38 @@ export class WrapperReplayer {
   }
 
   public destroy() {
-    this.videoInst.destroy();
-    this.rrwebInst.destroy();
+    this.videoReplayer.destroy();
+    this.replayer.destroy();
   }
 
   /**
    * Returns the current video time, using the video's external timer.
    */
   public getCurrentTime() {
-    return this.videoInst.getCurrentTime();
+    return this.videoReplayer.getCurrentTime();
   }
 
   /**
    * Play both the rrweb and video player.
    */
   public play(videoOffsetMs: number) {
-    this.videoInst.play(videoOffsetMs);
-    this.rrwebInst.play(videoOffsetMs);
+    this.videoReplayer.play(videoOffsetMs);
+    this.replayer.play(videoOffsetMs);
   }
 
   /**
    * Pause both the rrweb and video player.
    */
   public pause(videoOffsetMs: number) {
-    this.videoInst.pause(videoOffsetMs);
-    this.rrwebInst.pause(videoOffsetMs);
+    this.videoReplayer.pause(videoOffsetMs);
+    this.replayer.pause(videoOffsetMs);
   }
 
   /**
    * Equivalent to rrweb's `setConfig()`, but here we only support the `speed` configuration.
    */
   public setConfig(config: Partial<VideoReplayerConfig>): void {
-    this.videoInst.setConfig(config);
-    this.rrwebInst.setConfig(config);
+    this.videoReplayer.setConfig(config);
+    this.replayer.setConfig(config);
   }
 }