Skip to content

Commit bf91f4c

Browse files
committed
fix(media): forward MediaViewer's ref under React 18
MediaViewer read ref from its props, which only React 19 passes to a function component; under React 18, inside the peer range, a host's ref never reached the dialog. MediaViewer is now a forwardRef component that hands the ref to the <dialog>, and its props type leaves ref to forwardRef.
1 parent 40b73d2 commit bf91f4c

2 files changed

Lines changed: 35 additions & 15 deletions

File tree

‎src/ui/media-viewer.test.tsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { act, StrictMode, useState } from "react";
1+
import { act, createRef, StrictMode, useState } from "react";
22
import type * as React from "react";
33
import { createRoot, type Root } from "react-dom/client";
44
import { afterEach, describe, expect, test } from "bun:test";
@@ -181,6 +181,16 @@ describe("MediaViewer", () => {
181181
expect(document.activeElement).toBe(tile);
182182
});
183183

184+
test("passes its ref to the dialog", () => {
185+
const ref = createRef<HTMLDialogElement>();
186+
function WithRef() {
187+
const viewer = useMediaViewer({ items: ITEMS, defaultIndex: 0 });
188+
return <MediaViewer ref={ref} viewer={viewer} />;
189+
}
190+
render(<WithRef />);
191+
expect(ref.current).toBe(dialog()!);
192+
});
193+
184194
test("holds the page still while open, then restores its overflow", () => {
185195
const page = document.documentElement;
186196
page.style.setProperty("overflow", "scroll", "important");

‎src/ui/media-viewer.tsx‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"use client";
22

33
import { ChevronLeft, ChevronRight, Download, X } from "lucide-react";
4-
import { useCallback, useLayoutEffect, useRef } from "react";
4+
import { forwardRef, useCallback, useLayoutEffect, useRef } from "react";
55
import type * as React from "react";
66

77
import type { MediaViewerState } from "../hooks/use-media-viewer.js";
@@ -29,7 +29,7 @@ export type MediaViewerItem = {
2929
};
3030

3131
export type MediaViewerProps = Omit<
32-
React.ComponentProps<"dialog">,
32+
React.ComponentPropsWithoutRef<"dialog">,
3333
"children" | "open"
3434
> & {
3535
/** From `useMediaViewer`: which item is open, and how to step and close. */
@@ -50,21 +50,30 @@ const CONTROL =
5050
* opened. Escape pressed inside a PDF goes to the browser's PDF viewer, not
5151
* the dialog; Tab leaves the PDF and reaches Close.
5252
*
53-
* Renders nothing while `viewer` is closed.
53+
* Renders nothing while `viewer` is closed. A `ref` gets the `<dialog>`
54+
* while it is open.
5455
*/
55-
export function MediaViewer({ viewer, ...props }: MediaViewerProps) {
56-
const { item, index } = viewer;
57-
if (item === null || index === null) return null;
58-
return (
59-
<MediaViewerDialog viewer={viewer} item={item} index={index} {...props} />
60-
);
61-
}
56+
export const MediaViewer = forwardRef<HTMLDialogElement, MediaViewerProps>(
57+
function MediaViewer({ viewer, ...props }, ref) {
58+
const { item, index } = viewer;
59+
if (item === null || index === null) return null;
60+
return (
61+
<MediaViewerDialog
62+
viewer={viewer}
63+
item={item}
64+
index={index}
65+
forwardedRef={ref}
66+
{...props}
67+
/>
68+
);
69+
},
70+
);
6271

6372
function MediaViewerDialog({
6473
viewer,
6574
item,
6675
index,
67-
ref,
76+
forwardedRef,
6877
className,
6978
onCancel,
7079
onClick,
@@ -73,6 +82,7 @@ function MediaViewerDialog({
7382
}: MediaViewerProps & {
7483
readonly item: MediaViewerItem;
7584
readonly index: number;
85+
readonly forwardedRef: React.ForwardedRef<HTMLDialogElement>;
7686
}) {
7787
const dialogRef = useRef<HTMLDialogElement | null>(null);
7888
const closeRef = useRef<HTMLButtonElement | null>(null);
@@ -81,10 +91,10 @@ function MediaViewerDialog({
8191
const setDialog = useCallback(
8292
(element: HTMLDialogElement | null) => {
8393
dialogRef.current = element;
84-
if (typeof ref === "function") ref(element);
85-
else if (ref !== null && ref !== undefined) ref.current = element;
94+
if (typeof forwardedRef === "function") forwardedRef(element);
95+
else if (forwardedRef !== null) forwardedRef.current = element;
8696
},
87-
[ref],
97+
[forwardedRef],
8898
);
8999

90100
useLayoutEffect(() => {

0 commit comments

Comments
 (0)