Skip to content

fix: polish mobile lightbox swipe playback #42

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.

Sign up for GitHub

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

Merged
Simplereally merged 1 commit into from
Mar 10, 2026
Merged

fix: polish mobile lightbox swipe playback #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 83 additions & 1 deletion components/images/image-lightbox.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,14 @@ vi.mock("@/components/ui/media-player", () => ({
url,
alt,
contentType,
controls = true,
onLoadedMetadata,
onLoad,
}: {
url: string;
alt: string;
contentType?: string;
controls?: boolean;
onLoadedMetadata?: ReactEventHandler<HTMLVideoElement>;
onLoad?: ReactEventHandler<HTMLImageElement>;
}) => {
Expand All @@ -193,6 +195,8 @@ vi.mock("@/components/ui/media-player", () => ({
src={url}
aria-label={alt}
data-testid="video-player"
data-controls={controls ? "true" : "false"}
controls={controls}
onLoadedMetadata={onLoadedMetadata}
/>
);
Expand Down Expand Up @@ -428,6 +432,17 @@ describe("ImageLightbox - Prompt Library Integration", () => {
expect(onClose).toHaveBeenCalled();
});

it("closes when the lightbox whitespace is clicked", async () => {
const user = userEvent.setup();
const onClose = vi.fn();

render(<ImageLightbox image={mockImage} isOpen={true} onClose={onClose} />);

await user.click(screen.getByTestId("lightbox-surface"));

expect(onClose).toHaveBeenCalledTimes(1);
});

it("passes onClose as onInsertComplete to PromptLibrary", async () => {
const user = userEvent.setup();
const onClose = vi.fn();
Expand Down Expand Up @@ -869,6 +884,44 @@ describe("ImageLightbox - Mobile Swipe Navigation", () => {
});
});

it("accepts swipe gestures from the outer lightbox area", () => {
const onNext = vi.fn();

render(
<ImageLightbox
image={mockImage}
isOpen={true}
onClose={vi.fn()}
mediaNavigation={{
hasNext: true,
hasPrevious: true,
onNext,
onPrevious: vi.fn(),
}}
/>,
);

const swipeRegion = screen.getByTestId("lightbox-swipe-region");
fireEvent.pointerDown(swipeRegion, {
pointerId: 1,
pointerType: "touch",
isPrimary: true,
clientX: 24,
clientY: 120,
});
fireEvent.pointerUp(swipeRegion, {
pointerId: 1,
pointerType: "touch",
isPrimary: true,
clientX: 20,
clientY: 0,
});

return waitFor(() => {
expect(onNext).toHaveBeenCalledTimes(1);
});
});

it("navigates to the previous media on downward touch swipe", () => {
const onPrevious = vi.fn();

Expand Down Expand Up @@ -907,6 +960,35 @@ describe("ImageLightbox - Mobile Swipe Navigation", () => {
});
});

it("hides native video controls during mobile swipe navigation", () => {
const mockVideo = {
url: "https://example.com/test-video.mp4",
prompt: "A beautiful video",
model: "veo",
contentType: "video/mp4",
};

render(
<ImageLightbox
image={mockVideo}
isOpen={true}
onClose={vi.fn()}
mediaNavigation={{
hasNext: true,
hasPrevious: true,
hideVideoControls: true,
onNext: vi.fn(),
onPrevious: vi.fn(),
}}
/>,
);

expect(screen.getByTestId("video-player")).toHaveAttribute(
"data-controls",
"false",
);
});

it("drags the media with the finger before release", () => {
render(
<ImageLightbox
Expand Down Expand Up @@ -1000,7 +1082,7 @@ describe("ImageLightbox - Mobile Swipe Navigation", () => {
onClose={vi.fn()}
mediaNavigation={{
hasNext: false,
hasPrevious: false,
hasPrevious: true,
onNext: vi.fn(),
onPrevious: vi.fn(),
}}
Expand Down
41 changes: 27 additions & 14 deletions components/images/image-lightbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ interface ImageLightboxProps {
mediaNavigation?: {
hasNext: boolean;
hasPrevious: boolean;
hideVideoControls?: boolean;
onNext: () => void;
onPrevious: () => void;
};
Expand Down Expand Up @@ -269,6 +270,15 @@ export function ImageLightbox({
handleCloseAttempt();
}, [handleCloseAttempt]);

const handleLightboxSurfaceClick = React.useCallback(
(event: React.MouseEvent<HTMLDivElement>) => {
if (event.target === event.currentTarget) {
handleBackdropCloseAttempt();
}
},
[handleBackdropCloseAttempt],
);

// ==========================================
// Pane action handlers (used in compare mode)
// ==========================================
Expand Down Expand Up @@ -448,9 +458,17 @@ export function ImageLightbox({
...swipeGestureHandlers
} = swipeNavigationHandlers;
const isSwipeInteractionActive = isSwipeDragging || isSwipeAnimating;
const showMobileSwipeVideoControls = mediaNavigation?.hideVideoControls !== true;
const lightboxBackdropStyle = isSwipeNavigationEnabled
? swipeOverlayStyle
: { backgroundColor: "rgba(0, 0, 0, 0.8)" };
const lightboxSwipeRegionProps = isSwipeNavigationEnabled
? {
"data-testid": "lightbox-swipe-region",
style: { touchAction: swipeTouchAction as React.CSSProperties["touchAction"] },
...swipeGestureHandlers,
}
: {};

return (
<>
Expand All @@ -472,14 +490,14 @@ export function ImageLightbox({
className="w-full h-full backdrop-blur-md cursor-default flex items-center justify-center animate-in fade-in duration-150"
style={lightboxBackdropStyle}
>
<div className="relative w-full h-full">
<div
className="relative w-full h-full"
data-testid="lightbox-surface"
onClick={handleLightboxSurfaceClick}
{...lightboxSwipeRegionProps}
>
{isVideo ? (
<div
className="relative w-full h-full"
data-testid="lightbox-swipe-region"
style={{ touchAction: swipeTouchAction }}
{...swipeGestureHandlers}
>
<div className="relative w-full h-full">
<button
type="button"
aria-label="Close lightbox"
Expand All @@ -501,7 +519,7 @@ export function ImageLightbox({
url={displayImage.url}
alt={displayImage.prompt || "Generated video"}
contentType={displayImage.contentType}
controls={true}
controls={showMobileSwipeVideoControls}
autoPlay={true}
loop={true}
muted={true}
Expand Down Expand Up @@ -531,12 +549,7 @@ export function ImageLightbox({
/>
</div>
) : (
<div
className="w-full h-full"
data-testid="lightbox-swipe-region"
style={{ touchAction: swipeTouchAction }}
{...swipeGestureHandlers}
>
<div className="w-full h-full">
<div
className="w-full h-full"
style={swipeMediaStyle}
Expand Down
1 change: 1 addition & 0 deletions components/studio/layout/studio-shell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ export function StudioShell({
return {
hasNext: currentLightboxGalleryIndex < galleryImages.length - 1,
hasPrevious: currentLightboxGalleryIndex > 0,
hideVideoControls: true,
onNext: handleNextLightboxMedia,
onPrevious: handlePreviousLightboxMedia,
};
Expand Down
80 changes: 77 additions & 3 deletions hooks/use-media-player.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe("useMediaPlayer", () => {
describe("handleError", () => {
it("sets hasError to true on error", () => {
const { result } = renderHook(() =>
useMediaPlayer({ url: "https://example.com/video.mp4", isVideo: true })
useMediaPlayer({ url: "https://example.com/image.jpg", isVideo: false })
)

expect(result.current.hasError).toBe(false)
Expand All @@ -104,8 +104,8 @@ describe("useMediaPlayer", () => {
const onError = vi.fn()
const { result } = renderHook(() =>
useMediaPlayer({
url: "https://example.com/video.mp4",
isVideo: true,
url: "https://example.com/image.jpg",
isVideo: false,
onError,
})
)
Expand Down Expand Up @@ -166,6 +166,80 @@ describe("useMediaPlayer", () => {
expect(result.current.hasError).toBe(false)
expect(onError).not.toHaveBeenCalled()
})

it("does not surface transient video errors during source swaps", async () => {
vi.useFakeTimers()

const onError = vi.fn()
const { result } = renderHook(() =>
useMediaPlayer({
url: "https://example.com/video.mp4",
isVideo: true,
onError,
})
)

const mockVideo = {
src: "https://example.com/video.mp4",
currentSrc: "https://example.com/video.mp4",
error: null,
networkState: HTMLMediaElement.NETWORK_LOADING,
} as unknown as HTMLVideoElement
;(result.current.videoRef as React.MutableRefObject<HTMLVideoElement | null>).current = mockVideo

act(() => {
result.current.handleError({
currentTarget: mockVideo,
} as unknown as React.SyntheticEvent<HTMLVideoElement>)
})

expect(result.current.hasError).toBe(false)

await act(async () => {
vi.advanceTimersByTime(500)
})

expect(result.current.hasError).toBe(false)
expect(onError).not.toHaveBeenCalled()
vi.useRealTimers()
})

it("surfaces persistent video errors after a grace period", async () => {
vi.useFakeTimers()

const onError = vi.fn()
const { result } = renderHook(() =>
useMediaPlayer({
url: "https://example.com/video.mp4",
isVideo: true,
onError,
})
)

const mockVideo = {
src: "https://example.com/video.mp4",
currentSrc: "https://example.com/video.mp4",
error: { code: 4 } as MediaError,
networkState: HTMLMediaElement.NETWORK_NO_SOURCE,
} as unknown as HTMLVideoElement
;(result.current.videoRef as React.MutableRefObject<HTMLVideoElement | null>).current = mockVideo

act(() => {
result.current.handleError({
currentTarget: mockVideo,
} as unknown as React.SyntheticEvent<HTMLVideoElement>)
})

expect(result.current.hasError).toBe(false)

await act(async () => {
vi.advanceTimersByTime(500)
})

expect(result.current.hasError).toBe(true)
expect(onError).toHaveBeenCalledTimes(1)
vi.useRealTimers()
})
})

describe("handleVideoLoadedData", () => {
Expand Down
Loading
Loading