From e51667e4cb7d85f5ec19130b90dd370d47136d2e Mon Sep 17 00:00:00 2001 From: Adam Shiervani Date: Thu, 27 Feb 2025 12:36:08 +0100 Subject: [PATCH] refactor(ui): Replace react-router-dom navigation with custom navigation hook This commit introduces a new custom navigation hook `useDeviceUiNavigation` to replace direct usage of `useNavigate` across multiple components: - Removed direct `useNavigate` imports in various components - Added `navigateTo` method from new navigation hook - Updated navigation calls in ActionBar, MountPopover, UpdateInProgressStatusCard, and other routes - Simplified navigation logic and prepared for potential future navigation enhancements - Removed console logs and unnecessary comments --- ui/src/components/ActionBar.tsx | 7 +-- ui/src/components/Modal.tsx | 1 - .../components/UpdateInProgressStatusCard.tsx | 9 +-- ui/src/components/popovers/MountPopover.tsx | 7 ++- ui/src/hooks/stores.ts | 8 +-- ui/src/hooks/useAppNavigation.ts | 58 +++++++++++++++++++ ui/src/routes/devices.$id.mount.tsx | 4 +- .../devices.$id.settings.general._index.tsx | 8 +-- .../devices.$id.settings.general.update.tsx | 18 ++---- .../devices.$id.settings.security._index.tsx | 11 ++-- ...vices.$id.settings.security.local-auth.tsx | 19 ++++-- ui/src/routes/devices.$id.tsx | 23 +++----- 12 files changed, 105 insertions(+), 68 deletions(-) create mode 100644 ui/src/hooks/useAppNavigation.ts diff --git a/ui/src/components/ActionBar.tsx b/ui/src/components/ActionBar.tsx index 41fe09d..9f299de 100644 --- a/ui/src/components/ActionBar.tsx +++ b/ui/src/components/ActionBar.tsx @@ -17,13 +17,14 @@ import MountPopopover from "./popovers/MountPopover"; import { Fragment, useCallback, useRef } from "react"; import { CommandLineIcon } from "@heroicons/react/20/solid"; import ExtensionPopover from "./popovers/ExtensionPopover"; -import { useNavigate } from "react-router-dom"; +import { useDeviceUiNavigation } from "../hooks/useAppNavigation"; export default function Actionbar({ requestFullscreen, }: { requestFullscreen: () => Promise; }) { + const { navigateTo } = useDeviceUiNavigation(); const virtualKeyboard = useHidStore(state => state.isVirtualKeyboardEnabled); const setVirtualKeyboard = useHidStore(state => state.setVirtualKeyboardEnabled); @@ -54,8 +55,6 @@ export default function Actionbar({ [setDisableFocusTrap], ); - const navigate = useNavigate(); - return (
navigate("settings")} + onClick={() => navigateTo("/settings")} />
diff --git a/ui/src/components/Modal.tsx b/ui/src/components/Modal.tsx index 5e597fd..b6dac20 100644 --- a/ui/src/components/Modal.tsx +++ b/ui/src/components/Modal.tsx @@ -13,7 +13,6 @@ const Modal = React.memo(function Modal({ open: boolean; onClose: () => void; }) { - console.log("Modal", open); return ( @@ -31,10 +31,7 @@ export default function UpdateInProgressStatusCard() { className="pointer-events-auto" theme="light" text="View Details" - onClick={() => { - // TODO: this wont work in cloud mode - navigate("/settings/general/update"); - }} + onClick={() => navigateTo("/settings/general/update")} /> diff --git a/ui/src/components/popovers/MountPopover.tsx b/ui/src/components/popovers/MountPopover.tsx index 340afe2..e39a750 100644 --- a/ui/src/components/popovers/MountPopover.tsx +++ b/ui/src/components/popovers/MountPopover.tsx @@ -16,7 +16,8 @@ import { import { useJsonRpc } from "@/hooks/useJsonRpc"; import notifications from "../../notifications"; import { useClose } from "@headlessui/react"; -import { useLocation, useNavigate } from "react-router-dom"; +import { useLocation } from "react-router-dom"; +import { useDeviceUiNavigation } from "@/hooks/useAppNavigation"; const MountPopopover = forwardRef((_props, ref) => { const diskDataChannelStats = useRTCStore(state => state.diskDataChannelStats); @@ -187,7 +188,7 @@ const MountPopopover = forwardRef((_props, ref) => { syncRemoteVirtualMediaState(); }, [syncRemoteVirtualMediaState, location.pathname]); - const navigate = useNavigate(); + const { navigateTo } = useDeviceUiNavigation(); return ( @@ -307,7 +308,7 @@ const MountPopopover = forwardRef((_props, ref) => { text="Add New Media" onClick={() => { setModalView("mode"); - navigate("mount"); + navigateTo("/mount"); }} LeadingIcon={LuPlus} /> diff --git a/ui/src/hooks/stores.ts b/ui/src/hooks/stores.ts index 5e98971..ba6a4eb 100644 --- a/ui/src/hooks/stores.ts +++ b/ui/src/hooks/stores.ts @@ -517,9 +517,7 @@ export const useUpdateStore = create(set => ({ })); interface UsbConfigModalState { - modalView: - | "updateUsbConfig" - | "updateUsbConfigSuccess"; + modalView: "updateUsbConfig" | "updateUsbConfigSuccess"; errorMessage: string | null; setModalView: (view: UsbConfigModalState["modalView"]) => void; setErrorMessage: (message: string | null) => void; @@ -548,14 +546,10 @@ interface LocalAuthModalState { | "creationSuccess" | "deleteSuccess" | "updateSuccess"; - errorMessage: string | null; setModalView: (view: LocalAuthModalState["modalView"]) => void; - setErrorMessage: (message: string | null) => void; } export const useLocalAuthModalStore = create(set => ({ modalView: "createPassword", - errorMessage: null, setModalView: view => set({ modalView: view }), - setErrorMessage: message => set({ errorMessage: message }), })); diff --git a/ui/src/hooks/useAppNavigation.ts b/ui/src/hooks/useAppNavigation.ts new file mode 100644 index 0000000..aca7f71 --- /dev/null +++ b/ui/src/hooks/useAppNavigation.ts @@ -0,0 +1,58 @@ +import { useNavigate, useParams, NavigateOptions } from "react-router-dom"; +import { isOnDevice } from "../main"; +import { useCallback, useMemo } from "react"; + +/** + * Hook that provides context-aware navigation and path generation + * that works in both cloud and device modes. + * + * In cloud mode, paths are prefixed with /devices/:id + * In device mode, paths start from the root + * Relative paths (starting with . or ..) are preserved in both modes + * Supports all React Router navigation options + */ +export function useDeviceUiNavigation() { + const navigate = useNavigate(); + const params = useParams(); + + // Get the device ID from params + const deviceId = useMemo(() => params.id, [params.id]); + + // Function to generate the correct path + const getPath = useCallback( + (path: string): string => { + // Check if it's a relative path (starts with . or ..) + const isRelativePath = path.startsWith(".") || path === ""; + + // If it's a relative path, don't modify it + if (isRelativePath) return path; + + // Ensure absolute path starts with a slash + const normalizedPath = path.startsWith("/") ? path : `/${path}`; + + if (isOnDevice) { + return normalizedPath; + } else { + if (!deviceId) { + console.error("No device ID found in params when generating path"); + throw new Error("No device ID found in params when generating path"); + } + return `/devices/${deviceId}${normalizedPath}`; + } + }, + [deviceId], + ); + + // Function to navigate to the correct path with all options + const navigateTo = useCallback( + (path: string, options?: NavigateOptions) => { + navigate(getPath(path), options); + }, + [getPath, navigate], + ); + + return { + navigateTo, + getPath, + }; +} diff --git a/ui/src/routes/devices.$id.mount.tsx b/ui/src/routes/devices.$id.mount.tsx index 56fd1a4..ec289bc 100644 --- a/ui/src/routes/devices.$id.mount.tsx +++ b/ui/src/routes/devices.$id.mount.tsx @@ -93,9 +93,7 @@ export function Dialog({ onClose }: { onClose: () => void }) { clearMountMediaState(); syncRemoteVirtualMediaState() - .then(() => { - navigate(".."); - }) + .then(() => navigate("..")) .catch(err => { triggerError(err instanceof Error ? err.message : String(err)); }) diff --git a/ui/src/routes/devices.$id.settings.general._index.tsx b/ui/src/routes/devices.$id.settings.general._index.tsx index 7c7b61c..79b9f4d 100644 --- a/ui/src/routes/devices.$id.settings.general._index.tsx +++ b/ui/src/routes/devices.$id.settings.general._index.tsx @@ -13,10 +13,11 @@ import { CLOUD_APP } from "../ui.config"; import notifications from "../notifications"; import { isOnDevice } from "../main"; import Checkbox from "../components/Checkbox"; +import { useDeviceUiNavigation } from "../hooks/useAppNavigation"; export default function SettingsGeneralRoute() { const [send] = useJsonRpc(); - const navigate = useNavigate(); + const { navigateTo } = useDeviceUiNavigation(); const [devChannel, setDevChannel] = useState(false); const [autoUpdate, setAutoUpdate] = useState(true); @@ -135,10 +136,7 @@ export default function SettingsGeneralRoute() { size="SM" theme="light" text="Check for Updates" - onClick={() => { - // TODO: this wont work in cloud mode - navigate("./update"); - }} + onClick={() => navigateTo("./update")} /> diff --git a/ui/src/routes/devices.$id.settings.general.update.tsx b/ui/src/routes/devices.$id.settings.general.update.tsx index 0dc0d95..648efc7 100644 --- a/ui/src/routes/devices.$id.settings.general.update.tsx +++ b/ui/src/routes/devices.$id.settings.general.update.tsx @@ -7,6 +7,7 @@ import { UpdateState, useUpdateStore } from "@/hooks/stores"; import notifications from "@/notifications"; import { CheckCircleIcon } from "@heroicons/react/20/solid"; import LoadingSpinner from "@/components/LoadingSpinner"; +import { useDeviceUiNavigation } from "@/hooks/useAppNavigation"; export default function SettingsGeneralUpdateRoute() { const navigate = useNavigate(); @@ -36,15 +37,7 @@ export default function SettingsGeneralUpdateRoute() { { /* TODO: Migrate to using URLs instead of the global state. To simplify the refactoring, we'll keep the global state for now. */ } - return ( - { - // TODO: This wont work in cloud mode - navigate(".."); - }} - onConfirmUpdate={onConfirmUpdate} - /> - ); + return navigate("..")} onConfirmUpdate={onConfirmUpdate} />; } export interface SystemVersionInfo { @@ -61,7 +54,7 @@ export function Dialog({ onClose: () => void; onConfirmUpdate: () => void; }) { - const navigate = useNavigate(); + const { navigateTo } = useDeviceUiNavigation(); const [versionInfo, setVersionInfo] = useState(null); const { modalView, setModalView, otaState } = useUpdateStore(); @@ -113,10 +106,7 @@ export function Dialog({ {modalView === "updating" && ( { - // TODO: This wont work in cloud mode - navigate("/"); - }} + onMinimizeUpgradeDialog={() => navigateTo("/")} /> )} diff --git a/ui/src/routes/devices.$id.settings.security._index.tsx b/ui/src/routes/devices.$id.settings.security._index.tsx index 9961e73..d2856e7 100644 --- a/ui/src/routes/devices.$id.settings.security._index.tsx +++ b/ui/src/routes/devices.$id.settings.security._index.tsx @@ -1,10 +1,11 @@ import { SectionHeader } from "@/components/SectionHeader"; import { SettingsItem } from "./devices.$id.settings"; -import { useNavigate, useLoaderData } from "react-router-dom"; +import { useLoaderData } from "react-router-dom"; import { Button } from "../components/Button"; import { DEVICE_API } from "../ui.config"; import api from "../api"; import { LocalDevice } from "./devices.$id"; +import { useDeviceUiNavigation } from "../hooks/useAppNavigation"; export const loader = async () => { const status = await api @@ -15,7 +16,7 @@ export const loader = async () => { export default function SettingsSecurityIndexRoute() { const { authMode } = useLoaderData() as LocalDevice; - const navigate = useNavigate(); + const { navigateTo } = useDeviceUiNavigation(); return (
@@ -35,7 +36,7 @@ export default function SettingsSecurityIndexRoute() { theme="light" text="Disable Protection" onClick={() => { - navigate("local-auth", { state: { init: "deletePassword" } }); + navigateTo("./local-auth", { state: { init: "deletePassword" } }); }} /> ) : ( @@ -44,7 +45,7 @@ export default function SettingsSecurityIndexRoute() { theme="light" text="Enable Password" onClick={() => { - navigate("local-auth", { state: { init: "createPassword" } }); + navigateTo("./local-auth", { state: { init: "createPassword" } }); }} /> )} @@ -60,7 +61,7 @@ export default function SettingsSecurityIndexRoute() { theme="light" text="Change Password" onClick={() => { - navigate("local-auth", { state: { init: "updatePassword" } }); + navigateTo("./local-auth", { state: { init: "updatePassword" } }); }} /> diff --git a/ui/src/routes/devices.$id.settings.security.local-auth.tsx b/ui/src/routes/devices.$id.settings.security.local-auth.tsx index d9bf938..702bfba 100644 --- a/ui/src/routes/devices.$id.settings.security.local-auth.tsx +++ b/ui/src/routes/devices.$id.settings.security.local-auth.tsx @@ -3,32 +3,33 @@ import { Button } from "@components/Button"; import { InputFieldWithLabel } from "@/components/InputField"; import api from "@/api"; import { useLocalAuthModalStore } from "@/hooks/stores"; -import { useLocation, useNavigate } from "react-router-dom"; +import { useLocation, useRevalidator } from "react-router-dom"; +import { useDeviceUiNavigation } from "@/hooks/useAppNavigation"; export default function LocalAuthRoute() { - const navigate = useNavigate(); const { setModalView } = useLocalAuthModalStore(); - + const { navigateTo } = useDeviceUiNavigation(); const location = useLocation(); const init = location.state?.init; useEffect(() => { if (!init) { - navigate(".."); + navigateTo(".."); } else { setModalView(init); } - }, [init, navigate, setModalView]); + }, [init, navigateTo, setModalView]); { /* TODO: Migrate to using URLs instead of the global state. To simplify the refactoring, we'll keep the global state for now. */ } - return navigate("..")} />; + return navigateTo("..")} />; } export function Dialog({ onClose }: { onClose: () => void }) { const { modalView, setModalView } = useLocalAuthModalStore(); const [error, setError] = useState(null); + const revalidator = useRevalidator(); const handleCreatePassword = async (password: string, confirmPassword: string) => { if (password === "") { @@ -45,6 +46,8 @@ export function Dialog({ onClose }: { onClose: () => void }) { const res = await api.POST("/auth/password-local", { password }); if (res.ok) { setModalView("creationSuccess"); + // The rest of the app needs to revalidate the device authMode + revalidator.revalidate(); } else { const data = await res.json(); setError(data.error || "An error occurred while setting the password"); @@ -82,6 +85,8 @@ export function Dialog({ onClose }: { onClose: () => void }) { if (res.ok) { setModalView("updateSuccess"); + // The rest of the app needs to revalidate the device authMode + revalidator.revalidate(); } else { const data = await res.json(); setError(data.error || "An error occurred while changing the password"); @@ -101,6 +106,8 @@ export function Dialog({ onClose }: { onClose: () => void }) { const res = await api.DELETE("/auth/local-password", { password }); if (res.ok) { setModalView("deleteSuccess"); + // The rest of the app needs to revalidate the device authMode + revalidator.revalidate(); } else { const data = await res.json(); setError(data.error || "An error occurred while disabling the password"); diff --git a/ui/src/routes/devices.$id.tsx b/ui/src/routes/devices.$id.tsx index 9d89840..bc3c856 100644 --- a/ui/src/routes/devices.$id.tsx +++ b/ui/src/routes/devices.$id.tsx @@ -38,6 +38,7 @@ import Terminal from "@components/Terminal"; import { CLOUD_API, DEVICE_API } from "@/ui.config"; import Modal from "../components/Modal"; import { motion, AnimatePresence } from "motion/react"; +import { useDeviceUiNavigation } from "../hooks/useAppNavigation"; interface LocalLoaderResp { authMode: "password" | "noPassword" | null; @@ -322,11 +323,11 @@ export default function KvmIdRoute() { const setHdmiState = useVideoStore(state => state.setHdmiState); const [hasUpdated, setHasUpdated] = useState(false); + const { navigateTo } = useDeviceUiNavigation(); function onJsonRpcRequest(resp: JsonRpcRequest) { if (resp.method === "otherSessionConnected") { - console.log("otherSessionConnected", resp.params); - navigate("other-session"); + navigateTo("/other-session"); } if (resp.method === "usbState") { @@ -350,8 +351,7 @@ export default function KvmIdRoute() { if (otaState.error) { setModalView("error"); - // TODO: this wont work in cloud mode - navigate("update"); + navigateTo("/settings/general/update"); return; } @@ -381,12 +381,9 @@ export default function KvmIdRoute() { // When the update is successful, we need to refresh the client javascript and show a success modal useEffect(() => { if (queryParams.get("updateSuccess")) { - // TODO: this wont work in cloud mode - navigate("./settings/general/update", { - state: { updateSuccess: true }, - }); + navigateTo("/settings/general/update", { state: { updateSuccess: true } }); } - }, [navigate, queryParams, setModalView, setQueryParams]); + }, [navigate, navigateTo, queryParams, setModalView, setQueryParams]); const diskChannel = useRTCStore(state => state.diskChannel)!; const file = useMountMediaStore(state => state.localFile)!; @@ -442,10 +439,8 @@ export default function KvmIdRoute() { const outlet = useOutlet(); const location = useLocation(); const onModalClose = useCallback(() => { - if (location.pathname !== "/other-session") { - navigate(".."); - } - }, [navigate, location.pathname]); + if (location.pathname !== "/other-session") navigateTo(".."); + }, [navigateTo, location.pathname]); return ( <> @@ -497,7 +492,7 @@ export default function KvmIdRoute() { onKeyUp={e => e.stopPropagation()} onKeyDown={e => { e.stopPropagation(); - if (e.key === "Escape") navigate("./"); + if (e.key === "Escape") navigateTo("/"); }} >