From 5881b8ae1c02a8cb2a0e0c6234e27a6597423a51 Mon Sep 17 00:00:00 2001 From: NikitaMGrimm <65914808+NikitaMGrimm@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:58:47 +0200 Subject: [PATCH 1/4] fix(mobile): file preview videos keep their own aspect ratio --- .../src/components/MediaVideoPlayer.tsx | 45 ++++++++++++++++++- .../files/WorkspaceFileVideoPreview.tsx | 1 + apps/mobile/src/lib/videoFrameFit.test.ts | 33 ++++++++++++++ apps/mobile/src/lib/videoFrameFit.ts | 14 ++++++ 4 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 apps/mobile/src/lib/videoFrameFit.test.ts create mode 100644 apps/mobile/src/lib/videoFrameFit.ts diff --git a/apps/mobile/src/components/MediaVideoPlayer.tsx b/apps/mobile/src/components/MediaVideoPlayer.tsx index bc5bd51888d9..592353761b7c 100644 --- a/apps/mobile/src/components/MediaVideoPlayer.tsx +++ b/apps/mobile/src/components/MediaVideoPlayer.tsx @@ -9,6 +9,8 @@ import { AppText } from "./ui/AppText"; import { SymbolView } from "./ui/AppSymbol"; import { VideoThumbnailImage } from "./VideoThumbnailImage"; import { useMediaActions } from "../state/mediaActions"; +import { fitVideoFrame } from "../lib/videoFrameFit"; +import { loadVideoThumbnail } from "../lib/videoThumbnails"; import { type MediaActionsSource } from "../lib/mediaActionsSource"; import { useFullscreenVideoOpen, useMediaAudioSessionHold } from "../lib/videoAudioSession"; import { MediaActionsMenu } from "./MediaActionsMenu"; @@ -116,6 +118,8 @@ interface MediaVideoPlayerProps { readonly thumbnailVisible?: boolean; readonly unavailable?: boolean; readonly expanded?: boolean; + /** Fill the parent and size the frame to the video's shape instead of a fixed 16:9 slot. */ + readonly fitToVideo?: boolean; readonly autoPlay?: boolean; readonly paused?: boolean; readonly actionsSource?: MediaActionsSource; @@ -126,12 +130,37 @@ function MediaVideoPlayerContent(props: MediaVideoPlayerProps) { const [playbackUri, setPlaybackUri] = useState(props.expanded ? props.uri : null); // Keep an opened player mounted while signing or reconnecting temporarily has no usable URL. if (playbackUri === null && props.expanded && props.uri !== null) setPlaybackUri(props.uri); + const [videoSize, setVideoSize] = useState<{ width: number; height: number } | null>(null); + const [bounds, setBounds] = useState<{ width: number; height: number } | null>(null); + const sizeSource = props.fitToVideo && videoSize === null ? props.uri : null; + useEffect(() => { + if (sizeSource === null) return; + // Thumbnails are oriented like the displayed frame, unlike VideoTrack.size. Loading here rather + // than from the thumbnail image keeps working after Play unmounts it; the shared cache dedupes. + const controller = new AbortController(); + void loadVideoThumbnail( + props.thumbnailKey, + async () => ({ uri: sizeSource, dispose: () => undefined }), + controller.signal, + ).then((thumbnail) => { + if (thumbnail && !controller.signal.aborted) { + setVideoSize({ width: thumbnail.width, height: thumbnail.height }); + } + }); + return () => controller.abort(); + }, [props.thumbnailKey, sizeSource]); + const fittedFrame = + props.fitToVideo && videoSize && bounds ? fitVideoFrame(videoSize, bounds, 480) : null; - return ( + const frame = ( {playbackUri ? ( ); + if (!props.fitToVideo) return frame; + return ( + { + const { width, height } = event.nativeEvent.layout; + setBounds({ width, height }); + }} + > + {frame} + + ); } export function MediaVideoPlayer(props: MediaVideoPlayerProps) { diff --git a/apps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx b/apps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx index 72b31f061e96..7ee4a818954d 100644 --- a/apps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx +++ b/apps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx @@ -19,6 +19,7 @@ export function WorkspaceFileVideoPreview(props: { resolvePlaybackUri={props.resolvePlaybackUri} name={props.name} thumbnailKey={props.thumbnailKey} + fitToVideo actionsSource={props.source?.actionsSource} /> diff --git a/apps/mobile/src/lib/videoFrameFit.test.ts b/apps/mobile/src/lib/videoFrameFit.test.ts new file mode 100644 index 000000000000..065434a8c262 --- /dev/null +++ b/apps/mobile/src/lib/videoFrameFit.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { fitVideoFrame } from "./videoFrameFit"; + +describe("fitVideoFrame", () => { + const bounds = { width: 360, height: 600 }; + + it("keeps landscape videos at the full capped width", () => { + expect(fitVideoFrame({ width: 1920, height: 1080 }, bounds, 480)).toEqual({ + width: 360, + height: 202.5, + }); + }); + + it("limits portrait videos by the available height", () => { + expect(fitVideoFrame({ width: 1080, height: 1920 }, bounds, 480)).toEqual({ + width: 337.5, + height: 600, + }); + }); + + it("caps wide bounds at the inline width", () => { + expect(fitVideoFrame({ width: 1080, height: 1080 }, { width: 800, height: 900 }, 480)).toEqual({ + width: 480, + height: 480, + }); + }); + + it("returns null before the video or bounds have a size", () => { + expect(fitVideoFrame({ width: 0, height: 0 }, bounds, 480)).toBeNull(); + expect(fitVideoFrame({ width: 1080, height: 1920 }, { width: 0, height: 0 }, 480)).toBeNull(); + }); +}); diff --git a/apps/mobile/src/lib/videoFrameFit.ts b/apps/mobile/src/lib/videoFrameFit.ts new file mode 100644 index 000000000000..667132df9895 --- /dev/null +++ b/apps/mobile/src/lib/videoFrameFit.ts @@ -0,0 +1,14 @@ +interface Size { + readonly width: number; + readonly height: number; +} + +/** Largest frame with the video's aspect ratio that fits the bounds and the inline width cap. */ +export function fitVideoFrame(video: Size, bounds: Size, maxWidth: number): Size | null { + if (video.width <= 0 || video.height <= 0 || bounds.width <= 0 || bounds.height <= 0) { + return null; + } + const ratio = video.width / video.height; + const width = Math.min(bounds.width, maxWidth, bounds.height * ratio); + return { width, height: width / ratio }; +} From d1bfc628a4fd10c9cca33ff6649d371341fea750 Mon Sep 17 00:00:00 2001 From: Nikita Maximilian Grimm Date: Mon, 19 Oct 2026 21:56:08 +0200 Subject: [PATCH 2/4] fix(mobile): tapping Play no longer restarts the Files preview size probe --- apps/mobile/src/components/MediaVideoPlayer.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/mobile/src/components/MediaVideoPlayer.tsx b/apps/mobile/src/components/MediaVideoPlayer.tsx index 592353761b7c..55cd6b8c7df4 100644 --- a/apps/mobile/src/components/MediaVideoPlayer.tsx +++ b/apps/mobile/src/components/MediaVideoPlayer.tsx @@ -1,7 +1,7 @@ import { useIsFocused } from "@react-navigation/native"; import { useEvent } from "expo"; import { useVideoPlayer, VideoView } from "expo-video"; -import { useEffect, useEffectEvent, useRef, useState } from "react"; +import { useEffect, useEffectEvent, useLayoutEffect, useRef, useState } from "react"; import { AppState, Pressable, View } from "react-native"; import { AppActivityIndicator } from "./ui/AppActivityIndicator"; @@ -133,10 +133,11 @@ function MediaVideoPlayerContent(props: MediaVideoPlayerProps) { const [videoSize, setVideoSize] = useState<{ width: number; height: number } | null>(null); const [bounds, setBounds] = useState<{ width: number; height: number } | null>(null); const sizeSource = props.fitToVideo && videoSize === null ? props.uri : null; - useEffect(() => { + // Layout effect so this request queues ahead of the poster's passive one. Otherwise tapping Play + // unmounts the poster, aborts the extraction it started, and this load begins again from scratch. + useLayoutEffect(() => { if (sizeSource === null) return; - // Thumbnails are oriented like the displayed frame, unlike VideoTrack.size. Loading here rather - // than from the thumbnail image keeps working after Play unmounts it; the shared cache dedupes. + // Thumbnails are oriented like the displayed frame, unlike VideoTrack.size. const controller = new AbortController(); void loadVideoThumbnail( props.thumbnailKey, From 1882dc19e1d36ea0bf5e12c3577a54255a3ef89c Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Mon, 26 Oct 2026 06:44:19 +0000 Subject: [PATCH 3/4] fix(mobile): explain native frame dimension synchronization --- apps/mobile/src/components/MediaVideoPlayer.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/mobile/src/components/MediaVideoPlayer.tsx b/apps/mobile/src/components/MediaVideoPlayer.tsx index b754e4f72b6c..bd92db176ff7 100644 --- a/apps/mobile/src/components/MediaVideoPlayer.tsx +++ b/apps/mobile/src/components/MediaVideoPlayer.tsx @@ -170,8 +170,8 @@ function MediaVideoPlayerContent(props: MediaVideoPlayerProps) { const [videoSize, setVideoSize] = useState<{ width: number; height: number } | null>(null); const [bounds, setBounds] = useState<{ width: number; height: number } | null>(null); const sizeSource = props.fitToVideo && videoSize === null ? props.uri : null; - // Layout effect so this request queues ahead of the poster's passive one. Otherwise tapping Play - // unmounts the poster, aborts the extraction it started, and this load begins again from scratch. + // Queue before the poster so early Play cannot cancel the extraction that supplies frame dimensions. + // oxlint-disable-next-line t3code/react-effect-budget -- Load native thumbnail dimensions independently of the poster's lifetime. useLayoutEffect(() => { if (sizeSource === null) return; // Thumbnails are oriented like the displayed frame, unlike VideoTrack.size. From 2321ea038dc992052244b40f7d88d60f21b92de7 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Fri, 30 Oct 2026 17:46:45 +0000 Subject: [PATCH 4/4] fix(mobile): cancel file video sizing when leaving the screen --- .../src/components/MediaVideoPlayer.tsx | 4 +- .../src/lib/useMediaVideoPlayback.test.tsx | 43 +++++++++++++------ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/apps/mobile/src/components/MediaVideoPlayer.tsx b/apps/mobile/src/components/MediaVideoPlayer.tsx index ac331dcac35c..ea901f244133 100644 --- a/apps/mobile/src/components/MediaVideoPlayer.tsx +++ b/apps/mobile/src/components/MediaVideoPlayer.tsx @@ -1,5 +1,6 @@ import { useEvent } from "expo"; import { useVideoPlayer, VideoView } from "expo-video"; +import { useIsFocused } from "@react-navigation/native"; import { useEffect, useEffectEvent, useLayoutEffect, useRef, useState } from "react"; import { Pressable, View } from "react-native"; @@ -149,12 +150,13 @@ interface MediaVideoPlayerProps { function MediaVideoPlayerContent(props: MediaVideoPlayerProps) { const mediaActions = useMediaActions(props.actionsSource); + const isFocused = useIsFocused(); const [playbackUri, setPlaybackUri] = useState(props.expanded ? props.uri : null); // Keep an opened player mounted while signing or reconnecting temporarily has no usable URL. if (playbackUri === null && props.expanded && props.uri !== null) setPlaybackUri(props.uri); const [videoSize, setVideoSize] = useState<{ width: number; height: number } | null>(null); const [bounds, setBounds] = useState<{ width: number; height: number } | null>(null); - const sizeSource = props.fitToVideo && videoSize === null ? props.uri : null; + const sizeSource = props.fitToVideo && isFocused && videoSize === null ? props.uri : null; // Queue before the poster so early Play cannot cancel the extraction that supplies frame dimensions. // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- loads native thumbnail dimensions independently of the poster's lifetime useLayoutEffect(() => { diff --git a/apps/mobile/src/lib/useMediaVideoPlayback.test.tsx b/apps/mobile/src/lib/useMediaVideoPlayback.test.tsx index c52563b4b4e2..261017c49b06 100644 --- a/apps/mobile/src/lib/useMediaVideoPlayback.test.tsx +++ b/apps/mobile/src/lib/useMediaVideoPlayback.test.tsx @@ -119,6 +119,7 @@ import { PullRequestMediaContext, PullRequestMediaVisibleContext, } from "../features/pull-requests/PullRequestMedia"; +import { MediaVideoPlayer } from "../components/MediaVideoPlayer"; function createPlayer() { let playing = false; @@ -149,7 +150,7 @@ function createPlayer() { }; } -function setup(kind: "thread" | "PR", compact = false) { +function setup(kind: "thread" | "PR" | "files", compact = false) { (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; mocks.focused = true; mocks.appState = "active"; @@ -166,10 +167,20 @@ function setup(kind: "thread" | "PR", compact = false) { const environmentId = EnvironmentId.make("test"); return { render(visible = true) { + if (kind === "files") mocks.focused = visible; act(() => root.render( - {kind === "thread" ? ( + {kind === "files" ? ( + + ) : kind === "thread" ? ( { - using screen = setup("PR", compact); +it.each([ + ["PR", false], + ["PR", true], + ["files", false], +] as const)( + "stops inactive %s thumbnail requests (compact: %s) and restarts on return", + async (kind, compact) => { + using screen = setup(kind, compact); mocks.thumbnail.mockReturnValue(new Promise(() => {})); screen.render(false); expect(mocks.thumbnail).not.toHaveBeenCalled(); await act(async () => screen.render(true)); - expect(mocks.thumbnail).toHaveBeenCalledTimes(1); - const signal = mocks.thumbnail.mock.calls[0]![2]; - expect(signal.aborted).toBe(false); - const aborted = new Promise((resolve) => signal.addEventListener("abort", resolve)); + const requests = kind === "files" ? 2 : 1; + expect(mocks.thumbnail).toHaveBeenCalledTimes(requests); + const aborted = Promise.all( + mocks.thumbnail.mock.calls.map(([, , signal]) => { + expect(signal.aborted).toBe(false); + return new Promise((resolve) => signal.addEventListener("abort", resolve)); + }), + ); screen.render(false); await act(() => aborted); - expect(mocks.thumbnail).toHaveBeenCalledTimes(1); + expect(mocks.thumbnail).toHaveBeenCalledTimes(requests); await act(async () => screen.render(true)); - expect(mocks.thumbnail).toHaveBeenCalledTimes(2); + expect(mocks.thumbnail).toHaveBeenCalledTimes(requests * 2); }, );