From 08a36248a073fbcc136b53aa41f372ffd624be83 Mon Sep 17 00:00:00 2001
From: Keith Smith <keithsmith@darksingularity.org>
Date: Fri, 28 Aug 2026 19:17:22 -0600
Subject: [PATCH] Replace video expand's Fullscreen API with a lightbox (#65)

requestFullscreen() silently did nothing in the Electron desktop build
(it worked fine in a regular browser). Swap it for a VideoLightbox
component mirroring the existing ImageLightbox overlay, which has no
such platform dependency.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 frontend/src/components/MessageList.tsx   | 38 +++++++++++++----
 frontend/src/components/VideoLightbox.css |  6 +++
 frontend/src/components/VideoLightbox.tsx | 52 +++++++++++++++++++++++
 3 files changed, 87 insertions(+), 9 deletions(-)
 create mode 100644 frontend/src/components/VideoLightbox.css
 create mode 100644 frontend/src/components/VideoLightbox.tsx

diff --git a/frontend/src/components/MessageList.tsx b/frontend/src/components/MessageList.tsx
index 90dbb2a..3a8fcdd 100644
--- a/frontend/src/components/MessageList.tsx
+++ b/frontend/src/components/MessageList.tsx
@@ -10,6 +10,7 @@ import { ImageLightbox } from './ImageLightbox'
 import { LinkPreviewCard } from './LinkPreviewCard'
 import { MessageContent } from './MessageContent'
 import { UserAvatar } from './UserAvatar'
+import { VideoLightbox } from './VideoLightbox'
 import './MessageList.css'
 
 export function FileAttachmentIcon() {
@@ -70,23 +71,25 @@ const PLAYABLE_VIDEO_CONTENT_TYPES = new Set(['video/mp4', 'video/webm', 'video/
 interface VideoAttachmentProps {
   file: MessageFileInfo
   roomId: string
+  onExpand: () => void
 }
 
 // Plays inline via the browser's own <video controls> (no custom overlay
 // needed for play/pause/volume/seek) -- the one thing it doesn't give a
-// small inline player is an obvious way to go bigger, so this adds an
-// explicit expand button on top calling the standard Fullscreen API
-// directly on the video element, rather than building a whole second
-// lightbox component just to re-embed the same <video>.
-function VideoAttachment({ file, roomId }: VideoAttachmentProps) {
-  const videoRef = useRef<HTMLVideoElement>(null)
+// small inline player is an obvious way to go bigger. The expand button
+// opens a VideoLightbox (matching how images already expand) rather than
+// calling the Fullscreen API directly on the video element -- that API is
+// unreliable in embedded/packaged contexts (e.g. the Electron desktop
+// build), where a rejected requestFullscreen() promise just does nothing
+// with no visible error.
+function VideoAttachment({ file, roomId, onExpand }: VideoAttachmentProps) {
   return (
     <div className="message-video-wrap">
-      <video ref={videoRef} src={getRoomFileUrl(roomId, file.id)} controls className="message-video" />
+      <video src={getRoomFileUrl(roomId, file.id)} controls className="message-video" />
       <button
         type="button"
         className="message-video-expand"
-        onClick={() => videoRef.current?.requestFullscreen()}
+        onClick={onExpand}
         aria-label="Expand video"
       >
         <svg width="14" height="14" viewBox="0 0 20 20" fill="none" aria-hidden="true">
@@ -133,6 +136,7 @@ export function MessageList({
   const [editingId, setEditingId] = useState<string | null>(null)
   const [draft, setDraft] = useState('')
   const [lightboxSrc, setLightboxSrc] = useState<string | null>(null)
+  const [videoLightbox, setVideoLightbox] = useState<{ src: string; filename: string } | null>(null)
   const [reactingId, setReactingId] = useState<string | null>(null)
   const [reactionPlacement, setReactionPlacement] = useState<'above' | 'below'>('below')
   const [previewFile, setPreviewFile] = useState<MessageFileInfo | null>(null)
@@ -259,7 +263,16 @@ export function MessageList({
                     />
                   )}
                   {msg.file && PLAYABLE_VIDEO_CONTENT_TYPES.has(msg.file.content_type) && (
-                    <VideoAttachment file={msg.file} roomId={roomId} />
+                    <VideoAttachment
+                      file={msg.file}
+                      roomId={roomId}
+                      onExpand={() =>
+                        setVideoLightbox({
+                          src: getRoomFileUrl(roomId, msg.file!.id),
+                          filename: msg.file!.filename,
+                        })
+                      }
+                    />
                   )}
                   {msg.file && !PLAYABLE_VIDEO_CONTENT_TYPES.has(msg.file.content_type) && (
                     <FileAttachmentCard
@@ -362,6 +375,13 @@ export function MessageList({
       })}
       <div ref={bottomRef} />
       {lightboxSrc && <ImageLightbox src={lightboxSrc} onClose={() => setLightboxSrc(null)} />}
+      {videoLightbox && (
+        <VideoLightbox
+          src={videoLightbox.src}
+          filename={videoLightbox.filename}
+          onClose={() => setVideoLightbox(null)}
+        />
+      )}
       {previewFile && (
         <FilePreviewModal
           roomId={roomId}
diff --git a/frontend/src/components/VideoLightbox.css b/frontend/src/components/VideoLightbox.css
new file mode 100644
index 0000000..18bed7b
--- /dev/null
+++ b/frontend/src/components/VideoLightbox.css
@@ -0,0 +1,6 @@
+.video-lightbox-video {
+  max-width: 100%;
+  max-height: 100%;
+  object-fit: contain;
+  border-radius: var(--radius);
+}
diff --git a/frontend/src/components/VideoLightbox.tsx b/frontend/src/components/VideoLightbox.tsx
new file mode 100644
index 0000000..843279f
--- /dev/null
+++ b/frontend/src/components/VideoLightbox.tsx
@@ -0,0 +1,52 @@
+import { useEscapeKey } from '../hooks/useEscapeKey'
+import './ImageLightbox.css'
+import './VideoLightbox.css'
+
+interface VideoLightboxProps {
+  src: string
+  filename: string
+  onClose: () => void
+}
+
+// #65 follow-up: requestFullscreen() on the inline <video> looked right in
+// testing but silently did nothing in production -- the Fullscreen API can
+// reject for reasons that don't show up as a visible error (permission
+// policy, a standalone/installed PWA window disallowing it entirely), and
+// nothing was catching that rejection. A lightbox has no such dependency --
+// it's the same "expand" ImageLightbox already gives images, reusing its
+// overlay/action-bar chrome directly (see the shared classNames below).
+export function VideoLightbox({ src, filename, onClose }: VideoLightboxProps) {
+  useEscapeKey(onClose)
+
+  return (
+    <div className="image-lightbox" onClick={onClose}>
+      <div className="image-lightbox-actions" onClick={(e) => e.stopPropagation()}>
+        <a href={src} download={filename} className="image-lightbox-download" aria-label="Download">
+          <svg width="15" height="15" viewBox="0 0 20 20" fill="none" aria-hidden="true">
+            <path
+              d="M10 3v10m0 0-4-4m4 4 4-4M4 16h12"
+              stroke="currentColor"
+              strokeWidth="1.6"
+              strokeLinecap="round"
+              strokeLinejoin="round"
+            />
+          </svg>
+        </a>
+        <button type="button" className="image-lightbox-close" onClick={onClose} aria-label="Close">
+          ×
+        </button>
+      </div>
+      {/* stopPropagation -- without it, clicking the video to play/pause
+          (or seek, or hit any native control) also bubbles up to the
+          overlay's onClose, closing the lightbox on the very interaction
+          it exists to allow. */}
+      <video
+        src={src}
+        controls
+        autoPlay
+        className="video-lightbox-video"
+        onClick={(e) => e.stopPropagation()}
+      />
+    </div>
+  )
+}