From a8a248fd9d2d2a6c2589df12ef8801aa5a13aa83 Mon Sep 17 00:00:00 2001 From: MrAlders0n <55921894+MrAlders0n@users.noreply.github.com> Date: Thu, 1 Oct 2026 04:09:00 -0400 Subject: [PATCH] Keep the packet drawer and windows open across tabs (#120) --- src/App.tsx | 62 ++++------------- src/features/observers/ObserverPage.tsx | 24 +++---- src/features/observers/observer-navigation.ts | 2 +- src/i18n/locales/en.json | 1 - src/i18n/locales/fr.json | 1 - tests/App.observerInvestigation.test.tsx | 66 ++++++++----------- tests/App.packetUrlContract.test.tsx | 8 +-- .../observers/observer-navigation.test.ts | 6 +- 8 files changed, 58 insertions(+), 112 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index deeac02..0c11974 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,5 +1,5 @@ import React, { useState, useEffect, useCallback, useRef, lazy, Suspense } from "react"; -import { BrowserRouter, Routes, Route, useLocation, useNavigate, useSearchParams, type Location } from "react-router-dom"; +import { BrowserRouter, Routes, Route, useLocation, useNavigate, useSearchParams } from "react-router-dom"; import { useTranslation } from "react-i18next"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { RegionProvider, useRegion, useRegionSelection } from "./hooks/useRegion"; @@ -148,14 +148,10 @@ function TabLoading({ tab }: { tab: string }) { return ; } -function AppInner({ observerVisit, onObserverDashboard, onReturn, onExitVisit }: { - observerVisit?: Location; onObserverDashboard: (id: string) => void; - onReturn: () => void; onExitVisit: () => void; -}) { - const { t } = useTranslation(); +function AppInner() { const [searchParams, setSearchParams] = useSearchParams(); + const navigate = useNavigate(); const isMobile = useIsMobile(); - const location = useLocation(); // The URL is the single source of truth for the active tab — back/forward just work, and an // unknown ?tab value falls back to Packets instead of rendering a blank pane. // "Stats" was renamed to "Analytics"; keep old ?tab=Stats links working. @@ -170,11 +166,10 @@ function AppInner({ observerVisit, onObserverDashboard, onReturn, onExitVisit }: const [selectedObservationId, setSelectedObservationId] = useState(null); const [selectedNodeId, setSelectedNodeId] = useState(() => searchParams.get("node")); const [panels, setPanels] = useState([]); - const scene = JSON.stringify([activeTab, searchParams.get("route"), searchParams.get("hash"), searchParams.get("node"), searchParams.get("observer"), searchParams.get("analyze")]); + const scene = JSON.stringify([searchParams.get("hash"), searchParams.get("node"), searchParams.get("analyze")]); const [panelScene, setPanelScene] = useState(scene); - // Browser Back to another entity ends its panels; dashboard visits freeze this location instead. + // Moving to another packet or node ends its panels; switching tabs keeps them (desktop). if (panelScene !== scene) { setPanelScene(scene); setPanels([]); } - const dashboardFocus = useRef(null); const openPanel = useCallback((target: InvestigationTarget) => { const key = target.kind === "packet" ? `packet:${target.hash.toLowerCase()}:${target.observationId ?? ""}` : target.kind === "path" ? `path:${target.detail.packetHash.toLowerCase()}:${target.selectedKey ?? "all"}` : `${target.kind}:${target.id}`; setPanels(previous => { @@ -189,10 +184,6 @@ function AppInner({ observerVisit, onObserverDashboard, onReturn, onExitVisit }: const [pathLink] = useState(() => ({ path: searchParams.get("path"), hash: searchParams.get("hash") })); const { data: analyzerDetail, isLoading: analyzerLoading } = usePacketDetail(analyzerHash); - useEffect(() => { - if (!observerVisit && dashboardFocus.current?.isConnected) dashboardFocus.current.focus(); - }, [observerVisit]); - const closePanel = (index: number) => { setPanels(previous => previous.slice(0, index)); if (panels[index]?.target.kind === "path" && searchParams.has("path")) setSearchParams(previous => { @@ -220,11 +211,9 @@ function AppInner({ observerVisit, onObserverDashboard, onReturn, onExitVisit }: }, [setSearchParams]); const handleTabChange = (tab: string, mapFocus?: { lat: number; lng: number }) => { - setPanels([]); - dashboardFocus.current = null; - onExitVisit(); // On mobile a detail panel (and the analyzer) fills the screen, so leaving its tab must close it; - // desktop side panels persist across tabs. Cross-nav (onViewObserver) re-sets its selection after this. + // desktop side panels persist across tabs. Map and Analytics start without carried-over windows. + if (isMobile || tab === "Map" || tab === "Analytics") setPanels([]); if (isMobile) { setSelectedObservationId(null); setSelectedNodeId(null); @@ -265,9 +254,7 @@ function AppInner({ observerVisit, onObserverDashboard, onReturn, onExitVisit }: const clearSelection = useCallback(() => { setSelectedNodeId(null); setPanels([]); - dashboardFocus.current = null; - onExitVisit(); - }, [onExitVisit, setSelectedNodeId, setPanels]); + }, [setSelectedNodeId, setPanels]); // Closing a detail panel drops its deep-link param so a reload can't reopen it (mirrors the packet // analyzer's ?analyze cleanup). Selecting a different node/observer doesn't touch the URL — the panel's @@ -286,9 +273,8 @@ function AppInner({ observerVisit, onObserverDashboard, onReturn, onExitVisit }: }, [dropSelectionParam, setSelectedNodeId]); const handleViewObserverStats = useCallback((id: string) => { - dashboardFocus.current = document.activeElement !== document.body ? document.activeElement as HTMLElement | null : null; - onObserverDashboard(id); - }, [onObserverDashboard]); + navigate({ search: "?" + observerDestination(searchParams, id).toString() }); + }, [navigate, searchParams]); useEffect(() => { // Region slugs can't be expanded yet (region details load async) — connect with the directly @@ -322,7 +308,7 @@ function AppInner({ observerVisit, onObserverDashboard, onReturn, onExitVisit }: return ( - } /> + } /> - +
- {/* Preserve measurable geometry: display:none makes virtualized lists lose their offset. */} -
}> {tabContent[activeTab]}
- {analyzerHash && (activeTab === "Packets" || activeTab === "Channels") && ( + {analyzerHash && activeTab !== "Map" && activeTab !== "Analytics" && ( closePanel(index)} onOpen={openPanel} onObserverDashboard={handleViewObserverStats} onViewOnMap={activeTab === "Map" ? undefined : handleViewOnMap} />)}
- {observerVisit && } />} -
); } -// A dashboard visit keeps one originating screen under its original location context. A reload has -// no in-memory origin, so copied/canonical dashboard URLs remain standalone destinations. -function AppNavigation() { - const location = useLocation(); - const navigate = useNavigate(); - const [origin, setOrigin] = useState(null); - const visiting = origin != null && location.state?.beaconObserverReturnKey === origin.key && new URLSearchParams(location.search).get("tab") === "Observers"; - if (origin && !visiting && location.key !== origin.key) setOrigin(null); - const exitVisit = useCallback(() => setOrigin(null), []); - const openDashboard = (id: string) => { - setOrigin(location); - navigate({ search: "?" + observerDestination(new URLSearchParams(location.search), id).toString() }, { state: { beaconObserverReturnKey: location.key } }); - }; - return navigate(-1)} onExitVisit={exitVisit} />} />; -} - export function App() { return ( - + diff --git a/src/features/observers/ObserverPage.tsx b/src/features/observers/ObserverPage.tsx index b85bdd7..1e38d71 100644 --- a/src/features/observers/ObserverPage.tsx +++ b/src/features/observers/ObserverPage.tsx @@ -1,5 +1,5 @@ -import { lazy, Suspense, useState, useEffect, useMemo, useRef } from "react"; -import { useLocation, useSearchParams } from "react-router-dom"; +import { lazy, Suspense, useState, useMemo } from "react"; +import { useSearchParams } from "react-router-dom"; import { useQuery } from "@tanstack/react-query"; import { useTranslation } from "react-i18next"; import { getBrokers } from "../../api/client"; @@ -19,13 +19,8 @@ const ObserverTab = lazy(() => import("../stats/ObserverTab").then(m => ({ defau const NAV_CHIP = "inline-flex items-center gap-1.5 rounded-sm border border-border bg-bg-raised px-2 py-0.5 font-mono text-[11px] text-text-normal transition-colors hover:border-text-dim hover:text-text-bright"; -export function ObserverPage({ wsManager, onReturn, returnLabel }: { wsManager: WsManager; onReturn?: () => void; returnLabel?: string }) { +export function ObserverPage({ wsManager }: { wsManager: WsManager }) { const [params, setParams] = useSearchParams(); - const location = useLocation(); - const returnButton = useRef(null); - useEffect(() => { returnButton.current?.focus(); }, []); - // Dashboard adjustments belong to this visit; Back returns to the original investigation. - const visitOptions = onReturn ? { replace: true, state: location.state } : undefined; const { t } = useTranslation(); const now = useTick(60_000); const id = params.get("observer"); @@ -50,7 +45,7 @@ export function ObserverPage({ wsManager, onReturn, returnLabel }: { wsManager: const share = new URL(window.location.pathname, window.location.origin); share.search = params.toString(); if (id) share.searchParams.set("range", range); if (comparing && until != null) share.searchParams.set("compareUntil", String(until)); - const select = (observer: string | null) => setParams(observerDestination(params, observer, range), visitOptions); + const select = (observer: string | null) => setParams(observerDestination(params, observer, range)); const compare = (observer: string) => { // eslint-disable-next-line react-hooks/purity -- Capture time when the user invokes this event callback. const clickedAt = Date.now(); @@ -60,9 +55,9 @@ export function ObserverPage({ wsManager, onReturn, returnLabel }: { wsManager: next.set("compareWith", observer); next.set("compareUntil", String(comparing ? until ?? hourAt(clickedAt) : hourAt(clickedAt))); return next; - }, visitOptions); + }); }; - const closeCompare = () => setParams(old => { const next = new URLSearchParams(old); next.delete("compareWith"); next.delete("compareUntil"); return next; }, visitOptions); + const closeCompare = () => setParams(old => { const next = new URLSearchParams(old); next.delete("compareWith"); next.delete("compareUntil"); return next; }); const refreshCompare = () => { const clickedAt = Date.now(); setActionTime(clickedAt); @@ -70,20 +65,17 @@ export function ObserverPage({ wsManager, onReturn, returnLabel }: { wsManager: const next = new URLSearchParams(old); next.set("compareUntil", String(hourAt(clickedAt))); return next; - }, { replace: true, state: location.state }); + }, { replace: true }); return hourAt(clickedAt); }; return
- {onReturn &&
- -
} b.name) ?? []} scopeFilter={scope} onScopeChange={setScope} scopeOptions={scopeOptions} trailing={{t("observerPage.range")} - setParams(observerDestination(params, id, observerRange(v)), visitOptions)} /> + setParams(observerDestination(params, id, observerRange(v)))} /> } />
diff --git a/src/features/observers/observer-navigation.ts b/src/features/observers/observer-navigation.ts index b33fee7..75e2a4f 100644 --- a/src/features/observers/observer-navigation.ts +++ b/src/features/observers/observer-navigation.ts @@ -5,7 +5,7 @@ export const observerRange = (value: string | null): StatsRange => value === "24 export function observerDestination(params: URLSearchParams, id: string | null, range = observerRange(params.get("range"))) { const next = new URLSearchParams(params); next.set("tab", "Observers"); - for (const key of ["statsTab", "observerId", "node", "hash", "analyze", "path", "observation", "route", "routeIata", "routeRange", "routeSince", "routeUntil", "compareA", "compareB", "compareSince"]) next.delete(key); + for (const key of ["statsTab", "observerId", "node", "route", "routeIata", "routeRange", "routeSince", "routeUntil", "compareA", "compareB", "compareSince"]) next.delete(key); if (id) { next.set("observer", id); next.set("range", range); } else { next.delete("observer"); next.delete("range"); next.delete("compareWith"); next.delete("compareUntil"); } return next; diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 0daa97b..0c8c5dc 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -309,7 +309,6 @@ "recording": "Loading observer…", "hour": "hour", "day": "day", - "returnTo": "Back to {{page}}", "directory": "All observers", "sidebarSearch": "Search observers", "noMatches": "No matches", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index b1dd242..a919723 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -313,7 +313,6 @@ "recording": "Chargement de l’observateur…", "hour": "heure", "day": "jour", - "returnTo": "Retour à « {{page}} »", "directory": "Tous les observateurs", "sidebarSearch": "Rechercher les observateurs", "noMatches": "Aucun résultat", diff --git a/tests/App.observerInvestigation.test.tsx b/tests/App.observerInvestigation.test.tsx index 983675d..2b1f0e3 100644 --- a/tests/App.observerInvestigation.test.tsx +++ b/tests/App.observerInvestigation.test.tsx @@ -51,7 +51,7 @@ const click = (name: string) => { const button = screen.getByRole("button", { na const escape = () => fireEvent.keyDown(screen.getByRole("dialog"), { key: "Escape" }); const browserBack = async () => { await act(async () => { window.history.back(); }); await waitFor(() => expect(window.location.search).toContain("tab=Routes")); }; -describe("observer investigation return", () => { +describe("observer investigation windows", () => { it("dismisses only the observer with Escape and returns keyboard focus", () => { render(); click("Route observer"); escape(); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); @@ -77,32 +77,35 @@ describe("observer investigation return", () => { expect(within(screen.getByRole("dialog")).getByRole("heading")).toHaveTextContent("Observer o1"); escape(); expect(within(screen.getByRole("dialog")).getByRole("heading")).toHaveTextContent("Packet aa report 8"); }); - it("returns from a changed dashboard window to the same route, filter, scroll and observer", async () => { - render(); - fireEvent.change(screen.getByLabelText("Route filter"), { target: { value: "roof" } }); - screen.getByTestId("route-scroll").scrollTop = 180; - click("Route observer"); click("Open dashboard"); + it("opens the dashboard as a normal tab with every window left open", async () => { + render(); click("Route packet"); click("Packet observer"); click("Open dashboard"); expect(await screen.findByRole("heading", { level: 1 })).toHaveTextContent("Dashboard o1 7d"); expect(window.location.search).toContain("tab=Observers&observer=o1"); - expect(screen.getByTestId("origin-url")).toHaveTextContent("route=full-route"); - fireEvent.click(within(screen.getByRole("group", { name: "Time range" })).getByRole("button", { name: "24h" })); - click("Compare with…"); - expect(window.location.search).toContain("compareWith="); - click("Back to Routes"); - await waitFor(() => expect(window.location.search).toContain("tab=Routes")); - expect(screen.getByLabelText("Route filter")).toHaveValue("roof"); - expect(screen.getByTestId("route-scroll").scrollTop).toBe(180); - await waitFor(() => expect(screen.getByRole("button", { name: "Open dashboard" })).toHaveFocus()); - escape(); expect(screen.getByRole("button", { name: "Route observer" })).toHaveFocus(); + expect(screen.queryByRole("button", { name: "Back to Routes" })).not.toBeInTheDocument(); + expect(within(screen.getByRole("dialog")).getByRole("heading")).toHaveTextContent("Observer o1"); + escape(); expect(within(screen.getByRole("dialog")).getByRole("heading")).toHaveTextContent("Packet aa"); }); - it("supports browser Back/Forward without rebuilding the originating screen", async () => { - render(); fireEvent.change(screen.getByLabelText("Route filter"), { target: { value: "keep" } }); - click("Route observer"); click("Open dashboard"); + it("returns to the previous tab with browser Back", async () => { + render(); click("Route observer"); click("Open dashboard"); await screen.findByRole("heading", { level: 1 }); - await browserBack(); expect(screen.getByLabelText("Route filter")).toHaveValue("keep"); - await act(async () => { window.history.forward(); }); - expect(await screen.findByRole("heading", { level: 1 })).toHaveTextContent("Dashboard o1"); - await browserBack(); expect(screen.getByLabelText("Route filter")).toHaveValue("keep"); + await browserBack(); expect(screen.getByTestId("route-origin")).toBeInTheDocument(); + }); + it("keeps the packet drawer and stacked windows across tabs, but not into Analytics", async () => { + await import("../src/features/stats/StatsOverview"); + window.history.replaceState({}, "", "/?tab=Routes&hash=aa&analyze=1"); + render(); + expect(screen.getByRole("heading", { name: /Packet aa/ })).toBeInTheDocument(); + click("Packet node"); + click("Observer tab"); + expect(await screen.findByRole("heading", { level: 2, name: /Packet aa/ })).toBeInTheDocument(); + expect(within(screen.getByRole("dialog")).getByRole("heading")).toHaveTextContent("Node n1"); + click("Analytics tab"); + await screen.findByRole("button", { name: "Leaderboard observer" }); + expect(screen.queryByRole("heading", { name: /Packet aa/ })).not.toBeInTheDocument(); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + click("Route tab"); + expect(await screen.findByRole("heading", { name: /Packet aa/ })).toBeInTheDocument(); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); it("opens the Observers dashboard from the leaderboard", async () => { await import("../src/features/stats/StatsOverview"); @@ -122,23 +125,10 @@ describe("observer investigation return", () => { expect(window.location.search).toContain("tab=Observers"); expect(window.location.search).not.toContain("statsTab"); }); - it("does not invent a return destination for a reloaded/shared dashboard", async () => { - window.history.replaceState({ usr: { beaconObserverReturnKey: "old" } }, "", "/?tab=Observers&observer=o2&range=24h"); - render(); expect(await screen.findByRole("heading", { level: 1 })).toHaveTextContent("Dashboard o2 24h"); - expect(screen.queryByRole("button", { name: "Back to Routes" })).not.toBeInTheDocument(); - click("Back to observers"); expect(screen.queryByRole("heading", { level: 1 })).not.toBeInTheDocument(); - expect(screen.getByText("Observer directory")).toBeInTheDocument(); - }); - it("ends the retained investigation when the operator changes tabs or regions", async () => { + it("closes the stacked windows when the region changes", async () => { render(); click("Route observer"); click("Open dashboard"); await screen.findByRole("heading", { level: 1 }); click("Change region"); - await waitFor(() => expect(screen.queryByRole("button", { name: "Back to Routes" })).not.toBeInTheDocument()); - expect(screen.queryByTestId("route-origin")).not.toBeInTheDocument(); - click("Route tab"); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - }); - it("translates the return action", async () => { - await i18n.changeLanguage("fr"); render(); click("Route observer"); click("Open dashboard"); - expect(await screen.findByRole("button", { name: /Retour à « Routes »/ })).toBeInTheDocument(); + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); }); it("clears comparison params from both the observer dashboard and analytics compare tabs on tab change", () => { window.history.replaceState({}, "", "/?tab=Observers&observer=o1&compareWith=o2&compareUntil=1700000000000&compareA=x&compareB=y&compareSince=1"); diff --git a/tests/App.packetUrlContract.test.tsx b/tests/App.packetUrlContract.test.tsx index 9f706f8..dc3fbbd 100644 --- a/tests/App.packetUrlContract.test.tsx +++ b/tests/App.packetUrlContract.test.tsx @@ -141,7 +141,7 @@ afterEach(() => { }); describe("Packets deep links", () => { - it("returns from an observer dashboard to the same packet, report and filters", async () => { + it("keeps the packet drawer beside the observer dashboard and returns to the same packet, report and filters", async () => { setMobile(false); window.history.pushState({}, "", "/?tab=Packets&hash=AA11&analyze=1&observation=1&iata=YOW&q=missing"); render(); @@ -150,10 +150,10 @@ describe("Packets deep links", () => { fireEvent.click(within(section).getByRole("button", { name: "Inspect observer" })); fireEvent.click(await screen.findByRole("button", { name: "Open observer dashboard" })); expect(await screen.findByRole("heading", { name: "Observer dashboard" })).toBeInTheDocument(); - expect(new URLSearchParams(window.location.search).has("observation")).toBe(false); + expect(new URLSearchParams(window.location.search).get("observation")).toBe("1"); + expect(screen.getByTestId("packet-analyzer-drawer")).toBeInTheDocument(); act(() => window.history.back()); - // The originating drawer now stays mounted while hidden; wait for history, not its existence. - await waitFor(() => expect(new URLSearchParams(window.location.search).get("hash")).toBe("AA11")); + await waitFor(() => expect(new URLSearchParams(window.location.search).get("tab")).toBe("Packets")); expect(await screen.findByTestId("packet-analyzer-drawer")).toBeInTheDocument(); const params = new URLSearchParams(window.location.search); expect(params.get("hash")).toBe("AA11"); expect(params.get("observation")).toBe("1"); diff --git a/tests/features/observers/observer-navigation.test.ts b/tests/features/observers/observer-navigation.test.ts index 05cfa9d..bd28e75 100644 --- a/tests/features/observers/observer-navigation.test.ts +++ b/tests/features/observers/observer-navigation.test.ts @@ -15,9 +15,9 @@ describe("observerDestination", () => { expect(p.has("statsTab")).toBe(false); }); - it("drops unrelated investigation selectors and defaults the range to 7d", () => { - const p = observerDestination(new URLSearchParams("hash=abc&analyze=1&node=n&path=p"), "b"); - expect(p.toString()).toBe("tab=Observers&observer=b&range=7d"); + it("keeps the open packet analyzer, drops other tabs' selectors and defaults the range to 7d", () => { + const p = observerDestination(new URLSearchParams("hash=abc&analyze=1&observation=5&path=p&node=n&route=r"), "b"); + expect(Object.fromEntries(p)).toEqual({ hash: "abc", analyze: "1", observation: "5", path: "p", tab: "Observers", observer: "b", range: "7d" }); }); it("drops the analytics compare params so they don't ride along into the dashboard URL", () => {