From 36da8db6d07d1483d8e27eefe987d0869d3cef24 Mon Sep 17 00:00:00 2001 From: Nicolas Mowen Date: Tue, 25 Nov 2025 10:24:07 -0700 Subject: [PATCH] Revamp the history handling for dialog components --- web/src/components/mobile/MobilePage.tsx | 38 +++---------- .../overlay/detail/SearchDetailDialog.tsx | 2 +- .../overlay/dialog/PlatformAwareDialog.tsx | 7 ++- web/src/components/ui/dialog.tsx | 52 +++++------------ web/src/components/ui/sheet.tsx | 52 +++++------------ web/src/hooks/use-history-back.ts | 57 +++++++++++++++++++ 6 files changed, 103 insertions(+), 105 deletions(-) create mode 100644 web/src/hooks/use-history-back.ts diff --git a/web/src/components/mobile/MobilePage.tsx b/web/src/components/mobile/MobilePage.tsx index a4b80b2cb..4b6c41c7a 100644 --- a/web/src/components/mobile/MobilePage.tsx +++ b/web/src/components/mobile/MobilePage.tsx @@ -13,7 +13,7 @@ import { cn } from "@/lib/utils"; import { isPWA } from "@/utils/isPWA"; import { Button } from "@/components/ui/button"; import { useTranslation } from "react-i18next"; -import { useLocation } from "react-router-dom"; +import { useHistoryBack } from "@/hooks/use-history-back"; const MobilePageContext = createContext<{ open: boolean; @@ -24,15 +24,16 @@ type MobilePageProps = { children: React.ReactNode; open?: boolean; onOpenChange?: (open: boolean) => void; + enableHistoryBack?: boolean; }; export function MobilePage({ children, open: controlledOpen, onOpenChange, + enableHistoryBack = true, }: MobilePageProps) { const [uncontrolledOpen, setUncontrolledOpen] = useState(false); - const location = useLocation(); const open = controlledOpen ?? uncontrolledOpen; const setOpen = useCallback( @@ -46,33 +47,12 @@ export function MobilePage({ [onOpenChange, setUncontrolledOpen], ); - useEffect(() => { - let isActive = true; - - if (open && isActive) { - window.history.pushState({ isMobilePage: true }, "", location.pathname); - } - - const handlePopState = (event: PopStateEvent) => { - if (open && isActive) { - event.preventDefault(); - setOpen(false); - // Delay replaceState to ensure state updates are processed - setTimeout(() => { - if (isActive) { - window.history.replaceState(null, "", location.pathname); - } - }, 0); - } - }; - - window.addEventListener("popstate", handlePopState); - - return () => { - isActive = false; - window.removeEventListener("popstate", handlePopState); - }; - }, [open, setOpen, location.pathname]); + // Handle browser back button to close mobile page + useHistoryBack({ + enabled: enableHistoryBack, + open, + onClose: () => setOpen(false), + }); return ( diff --git a/web/src/components/overlay/detail/SearchDetailDialog.tsx b/web/src/components/overlay/detail/SearchDetailDialog.tsx index 333c1bc9d..467008e92 100644 --- a/web/src/components/overlay/detail/SearchDetailDialog.tsx +++ b/web/src/components/overlay/detail/SearchDetailDialog.tsx @@ -538,7 +538,7 @@ export default function SearchDetailDialog({ {isDesktop && onPrevious && onNext && ( diff --git a/web/src/components/overlay/dialog/PlatformAwareDialog.tsx b/web/src/components/overlay/dialog/PlatformAwareDialog.tsx index e0ec6efa9..5782ba149 100644 --- a/web/src/components/overlay/dialog/PlatformAwareDialog.tsx +++ b/web/src/components/overlay/dialog/PlatformAwareDialog.tsx @@ -113,7 +113,12 @@ export function PlatformAwareSheet({ } return ( - + {trigger} diff --git a/web/src/components/ui/dialog.tsx b/web/src/components/ui/dialog.tsx index 65a861012..f65adbff0 100644 --- a/web/src/components/ui/dialog.tsx +++ b/web/src/components/ui/dialog.tsx @@ -2,6 +2,7 @@ import * as React from "react"; import * as DialogPrimitive from "@radix-ui/react-dialog"; import { X } from "lucide-react"; import { cn } from "@/lib/utils"; +import { useHistoryBack } from "@/hooks/use-history-back"; // Enhanced Dialog with History Support interface HistoryDialogProps extends DialogPrimitive.DialogProps { @@ -15,51 +16,28 @@ const Dialog = ({ ...props }: HistoryDialogProps) => { const [internalOpen, setInternalOpen] = React.useState(open || false); - const historyStateRef = React.useRef void; - }>(null); + // Sync internal state with controlled open prop React.useEffect(() => { if (open !== undefined) { setInternalOpen(open); } }, [open]); - React.useEffect(() => { - if (enableHistoryBack) { - if (internalOpen) { - window.history.pushState({ dialogOpen: true }, ""); + const handleOpenChange = React.useCallback( + (newOpen: boolean) => { + setInternalOpen(newOpen); + onOpenChange?.(newOpen); + }, + [onOpenChange], + ); - const listener = () => { - setInternalOpen(false); - if (onOpenChange) onOpenChange(false); - }; - - historyStateRef.current = { listener }; - window.addEventListener("popstate", listener); - - return () => { - if (internalOpen) { - window.removeEventListener("popstate", listener); - historyStateRef.current = null; - } - }; - } else if (historyStateRef.current) { - window.removeEventListener( - "popstate", - historyStateRef.current.listener, - ); - historyStateRef.current = null; - } - } - }, [enableHistoryBack, internalOpen, onOpenChange]); - - const handleOpenChange = (open: boolean) => { - setInternalOpen(open); - if (onOpenChange) { - onOpenChange(open); - } - }; + // Handle browser back button to close dialog + useHistoryBack({ + enabled: enableHistoryBack, + open: internalOpen, + onClose: () => handleOpenChange(false), + }); return ( { const [internalOpen, setInternalOpen] = React.useState(open || false); - const historyStateRef = React.useRef void; - }>(null); + // Sync internal state with controlled open prop React.useEffect(() => { if (open !== undefined) { setInternalOpen(open); } }, [open]); - React.useEffect(() => { - if (enableHistoryBack) { - if (internalOpen) { - window.history.pushState({ sheetOpen: true }, ""); + const handleOpenChange = React.useCallback( + (newOpen: boolean) => { + setInternalOpen(newOpen); + onOpenChange?.(newOpen); + }, + [onOpenChange], + ); - const listener = () => { - setInternalOpen(false); - if (onOpenChange) onOpenChange(false); - }; - - historyStateRef.current = { listener }; - window.addEventListener("popstate", listener); - - return () => { - if (internalOpen) { - window.removeEventListener("popstate", listener); - historyStateRef.current = null; - } - }; - } else if (historyStateRef.current) { - window.removeEventListener( - "popstate", - historyStateRef.current.listener, - ); - historyStateRef.current = null; - } - } - }, [enableHistoryBack, internalOpen, onOpenChange]); - - const handleOpenChange = (open: boolean) => { - setInternalOpen(open); - if (onOpenChange) { - onOpenChange(open); - } - }; + // Handle browser back button to close sheet + useHistoryBack({ + enabled: enableHistoryBack, + open: internalOpen, + onClose: () => handleOpenChange(false), + }); return ( void; +} + +/** + * Hook that manages browser history for overlay components (dialogs, sheets, etc.) + * When enabled, pressing the browser back button will close the overlay instead of navigating away. + */ +export function useHistoryBack({ + enabled, + open, + onClose, +}: UseHistoryBackOptions): void { + const historyPushedRef = React.useRef(false); + const closedByBackRef = React.useRef(false); + + // Keep onClose in a ref to avoid effect re-runs that cause multiple history pushes + const onCloseRef = React.useRef(onClose); + React.useLayoutEffect(() => { + onCloseRef.current = onClose; + }); + + React.useEffect(() => { + if (!enabled) return; + + if (open) { + // Only push history state if we haven't already (prevents duplicates in strict mode) + if (!historyPushedRef.current) { + window.history.pushState({ overlayOpen: true }, ""); + historyPushedRef.current = true; + } + + const handlePopState = () => { + closedByBackRef.current = true; + historyPushedRef.current = false; + onCloseRef.current(); + }; + + window.addEventListener("popstate", handlePopState); + + return () => { + window.removeEventListener("popstate", handlePopState); + }; + } else { + // Overlay is closing - clean up history if we pushed and it wasn't via back button + if (historyPushedRef.current && !closedByBackRef.current) { + window.history.back(); + } + historyPushedRef.current = false; + closedByBackRef.current = false; + } + }, [enabled, open]); +}