Skip to content

Commit

Permalink
fix(replay): Mobile replays look off when orientation changes (#83510)
Browse files Browse the repository at this point in the history
We showed the previous video to fix the video flickering in Safari.
However, if the orientation has changed, you can see both replay videos.
We can't show the previous replay in this case. This also fixes an issue
where video clips other than the one that's seeked to are firing off the
`seeking` event, and if that clip has a different orientation, it causes
the visible replay clip to have the wrong dimensions.

Before:


https://github.com/user-attachments/assets/51e434aa-7de2-4eaf-b991-cf5dad144974

After:


https://github.com/user-attachments/assets/bf1f41da-21a1-4865-90d2-f67ad0a679e8


Unfortunately on Safari, if the clips have gaps in them and the
orientation has changed, these gaps will be visible. There is nothing
feasible that we can do on the player side to fix this.
After on Safari:


https://github.com/user-attachments/assets/8dc124a0-f5cf-444d-b27f-6341ac010196

Fixes #83425

---------

Co-authored-by: Ryan Albrecht <[email protected]>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Jan 15, 2025
1 parent bc2fa9c commit 7a072af
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
1 change: 1 addition & 0 deletions static/app/components/replays/player/styles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {css, type Theme} from '@emotion/react';
// Base styles, to make the Replayer instance work
export const baseReplayerCss = css`
.replayer-wrapper {
/* Videos have z-index, so we need a z-index here so user interactions is on top of the video */
z-index: 1000000;
user-select: none;
}
Expand Down
12 changes: 10 additions & 2 deletions static/app/components/replays/videoReplayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,10 @@ export class VideoReplayer {

const handleSeeking = event => {
// Centers the video when seeking (and video is not playing)
this._callbacks.onLoaded!(event);
// Only call this for the segment that's being seeked to
if (index === this._currentIndex) {
this._callbacks.onLoaded!(event);
}
};

el.addEventListener('ended', handleEnded);
Expand Down Expand Up @@ -408,7 +411,12 @@ export class VideoReplayer {

for (const [index, videoElem] of this._videos) {
// On safari, some clips have a ~1 second gap in the beginning so we also need to show the previous video to hide this gap
if (index === (this._currentIndex || 0) - 1) {
// Edge case: Don't show previous video if size is different (eg. orientation changes)
if (
index === (this._currentIndex || 0) - 1 &&
videoElem.videoHeight === nextVideo.videoHeight &&
videoElem.videoWidth === nextVideo.videoWidth
) {
if (videoElem.duration) {
// we need to set the previous video to the end so that it's shown in case the next video has a gap at the beginning
// setting it to the end of the video causes the 'ended' bug in Chrome so we set it to 1 ms before the video ends
Expand Down

0 comments on commit 7a072af

Please sign in to comment.