Skip to content

Commit b974b88

Browse files
shadowusrshadowusr
authored
fix: handle incorrect player aspect ratios and timings (#770)
* fix: fix incorrect aspect ratio on certain mobile screens * fix: handle edge cases when timing is off timeline
1 parent b3ef6d6 commit b974b88

2 files changed

Lines changed: 56 additions & 21 deletions

File tree

‎lib/static/new-ui/features/suites/components/SnapshotsPlayer/Timeline.tsx‎

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -48,23 +48,37 @@ export function Timeline({
4848

4949
const progressRef = useRef<HTMLDivElement | null>(null);
5050

51+
const safeTotalTime = Number.isFinite(totalTime) && totalTime > 0 ? totalTime : 0;
52+
const getBoundedTime = (time: number): number => {
53+
if (!Number.isFinite(time) || safeTotalTime === 0) {
54+
return 0;
55+
}
56+
57+
return clamp(time, 0, safeTotalTime);
58+
};
59+
const getProgressPercent = (time: number): number => safeTotalTime === 0 ? 0 : getBoundedTime(time) / safeTotalTime * 100;
60+
5161
const getTimeFromMouseEvent = (e: MouseEvent | React.MouseEvent): number => {
52-
if (!progressRef.current || totalTime === 0) {
62+
if (!progressRef.current || safeTotalTime === 0) {
5363
return 0;
5464
}
5565

5666
const rect = progressRef.current.getBoundingClientRect();
67+
if (rect.width <= 0) {
68+
return 0;
69+
}
70+
5771
const offsetX = e.clientX - rect.left;
5872
const ratio = Math.max(0, Math.min(1, offsetX / rect.width));
5973

60-
return ratio * totalTime;
74+
return ratio * safeTotalTime;
6175
};
6276

6377
useEffect(() => {
6478
if (!isScrubbing && !isHovering) {
65-
setDisplayTime(currentTime);
79+
setDisplayTime(getBoundedTime(currentTime));
6680
}
67-
}, [currentTime, isScrubbing, isHovering]);
81+
}, [currentTime, isScrubbing, isHovering, safeTotalTime]);
6882

6983
// Disable text selection while scrubbing
7084
useEffect(() => {
@@ -118,7 +132,7 @@ export function Timeline({
118132
document.removeEventListener('mousemove', handleMouseMove);
119133
document.removeEventListener('mouseup', handleMouseUp);
120134
};
121-
}, [isScrubbing, totalTime]);
135+
}, [isScrubbing, safeTotalTime]);
122136

123137
const onTimelineMouseDown = (e: React.MouseEvent): void => {
124138
const clickedTime = getTimeFromMouseEvent(e);
@@ -147,17 +161,23 @@ export function Timeline({
147161
setIsHovering(false);
148162
// If not scrubbing, revert the knob to real player time
149163
if (!isScrubbing) {
150-
setDisplayTime(currentTime);
164+
setDisplayTime(getBoundedTime(currentTime));
151165
}
152166
onMouseLeave?.();
153167
};
154168

155-
const highlightStartTime = highlightState.highlightStartTime - playerStartTimestamp;
156-
const highlightEndTime = highlightState.highlightEndTime - playerStartTimestamp;
157-
const progressPercent = clamp(((isHovering && !isScrubbing ? currentTime : displayTime) / totalTime) * 100, 0, 100) * Number(!isSnapshotMissing);
158-
const highlightRegionWidth = highlightState.isActive ? clamp((highlightEndTime - highlightStartTime) / totalTime * 100, 0, 100) * Number(!isSnapshotMissing) : 0;
159-
const leftKnobPosition = clamp(((highlightState.isActive ? highlightStartTime : displayTime) / totalTime) * 100, 0, 100) * Number(!isSnapshotMissing);
160-
const rightKnobPosition = clamp(((highlightState.isActive ? highlightEndTime : displayTime) / totalTime) * 100, 0, 100) * Number(!isSnapshotMissing);
169+
const rawHighlightStartTime = highlightState.highlightStartTime - playerStartTimestamp;
170+
const rawHighlightEndTime = highlightState.highlightEndTime - playerStartTimestamp;
171+
const highlightStartTime = getBoundedTime(Math.min(rawHighlightStartTime, rawHighlightEndTime));
172+
const highlightEndTime = getBoundedTime(Math.max(rawHighlightStartTime, rawHighlightEndTime));
173+
const visibleTimelineMultiplier = Number(!isSnapshotMissing);
174+
const progressPercent = getProgressPercent(isHovering && !isScrubbing ? currentTime : displayTime) * visibleTimelineMultiplier;
175+
const highlightStartPercent = getProgressPercent(highlightStartTime);
176+
const highlightEndPercent = getProgressPercent(highlightEndTime);
177+
const highlightRegionWidth = highlightState.isActive ? Math.max(highlightEndPercent - highlightStartPercent, 0) * visibleTimelineMultiplier : 0;
178+
const leftKnobPosition = (highlightState.isActive ? highlightStartPercent : getProgressPercent(displayTime)) * visibleTimelineMultiplier;
179+
const rightKnobPosition = (highlightState.isActive ? highlightEndPercent : getProgressPercent(displayTime)) * visibleTimelineMultiplier;
180+
const loadingProgressPercent = (Number.isFinite(downloadProgress) ? clamp(downloadProgress, 0, 100) : 0) * visibleTimelineMultiplier;
161181

162182
const containerClasses = classNames(
163183
styles.container,
@@ -175,7 +195,7 @@ export function Timeline({
175195
return (
176196
<div className={containerClasses}>
177197
<div className={styles.playerTime}>
178-
{isSnapshotMissing ? '––:––' : formatTime(displayTime)}
198+
{isSnapshotMissing ? '––:––' : formatTime(getBoundedTime(displayTime))}
179199
</div>
180200

181201
<div
@@ -197,7 +217,7 @@ export function Timeline({
197217
<div
198218
className={styles.playerProgress}
199219
style={{
200-
width: `${(isLoading ? downloadProgress : progressPercent).toFixed(2)}%`
220+
width: `${(isLoading ? loadingProgressPercent : progressPercent).toFixed(2)}%`
201221
}}
202222
>
203223
<div className={styles.progressPulse} />
@@ -237,7 +257,7 @@ export function Timeline({
237257
</div>
238258

239259
<div className={styles.playerTime}>
240-
{isSnapshotMissing ? '––:––' : formatTime(totalTime)}
260+
{isSnapshotMissing ? '––:––' : formatTime(safeTotalTime)}
241261
</div>
242262
</div>
243263
);

‎lib/static/new-ui/features/suites/components/SnapshotsPlayer/index.tsx‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -488,18 +488,31 @@ export function SnapshotsPlayer({isSnapshotBroken = false}: {isSnapshotBroken?:
488488
}, [dispatch, isSnapshotMissing]);
489489

490490
const currentPlayerHighlightState = useSelector(state => state.app.snapshotsPlayer);
491+
const getPlayerTimeInBounds = (time: number): number => {
492+
let maxPlayerTime = MIN_PLAYER_TIME;
493+
try {
494+
maxPlayerTime = Math.max(playerRef.current?.getMetaData().totalTime ?? MIN_PLAYER_TIME, MIN_PLAYER_TIME);
495+
} catch { /* */ }
496+
497+
if (!Number.isFinite(time)) {
498+
return MIN_PLAYER_TIME;
499+
}
500+
501+
return Math.min(Math.max(time, MIN_PLAYER_TIME), maxPlayerTime);
502+
};
491503

492504
useEffect(() => {
493505
if (isSnapshotMissing) {
494506
return;
495507
}
496508

497-
if (currentPlayerHighlightState.isActive) {
498-
playerRef.current?.pause(currentPlayerHighlightState.highlightEndTime - playerRef.current?.getMetaData().startTime);
509+
if (currentPlayerHighlightState.isActive && playerRef.current) {
510+
const playerStartTime = playerRef.current.getMetaData().startTime;
511+
playerRef.current.pause(getPlayerTimeInBounds(currentPlayerHighlightState.highlightEndTime - playerStartTime));
499512
setIsPlaying(false);
500513
cancelTimeTicking();
501514
} else if (!isPlaying) {
502-
playerRef.current?.pause(currentPlayerTime > 0 ? currentPlayerTime : MIN_PLAYER_TIME);
515+
playerRef.current?.pause(getPlayerTimeInBounds(currentPlayerTime > 0 ? currentPlayerTime : MIN_PLAYER_TIME));
503516
}
504517
}, [currentPlayerHighlightState, isSnapshotMissing]);
505518

@@ -508,7 +521,7 @@ export function SnapshotsPlayer({isSnapshotBroken = false}: {isSnapshotBroken?:
508521
return;
509522
}
510523

511-
const newPlayerTime = Math.max(currentPlayerHighlightState.goToTime - playerRef.current.getMetaData().startTime, MIN_PLAYER_TIME);
524+
const newPlayerTime = getPlayerTimeInBounds(currentPlayerHighlightState.goToTime - playerRef.current.getMetaData().startTime);
512525
setCurrentPlayerTime(newPlayerTime);
513526
playerRef.current.pause(newPlayerTime);
514527
}, [currentPlayerHighlightState.goToTime, isSnapshotMissing]);
@@ -520,14 +533,16 @@ export function SnapshotsPlayer({isSnapshotBroken = false}: {isSnapshotBroken?:
520533
colorScheme: iframeColorScheme
521534
};
522535

536+
const isPlayerWidthConstrained = playerWidth / playerHeight >= maxPlayerSize.width / maxPlayerSize.height;
537+
523538
const playerStyle: React.CSSProperties = {
524539
aspectRatio: `${playerWidth} / ${playerHeight}`,
525540
maxWidth: `min(${playerWidth}px, 100%)`,
526541
maxHeight: '100%',
527542
transition: 'opacity .5s ease',
528543
position: 'absolute',
529-
height: playerHeight > playerWidth ? '100%' : 'auto',
530-
width: playerHeight > playerWidth ? 'auto' : '100%'
544+
height: isPlayerWidthConstrained ? 'auto' : '100%',
545+
width: isPlayerWidthConstrained ? '100%' : 'auto'
531546
};
532547

533548
const replayerContainerStyle: React.CSSProperties = {

0 commit comments

Comments
 (0)