diff --git a/src/features/packets/PacketExpansion.tsx b/src/features/packets/PacketExpansion.tsx index d609c88..043d10d 100644 --- a/src/features/packets/PacketExpansion.tsx +++ b/src/features/packets/PacketExpansion.tsx @@ -2,7 +2,6 @@ import { useCallback, useMemo } from "react"; import type { PacketSummary } from "../../types/api"; import { formatPropagation } from "../../lib/formatters"; import { Timestamp } from "../../components/Timestamp"; -import { CopyLinkButton } from "../../components/CopyLinkButton"; import { usePacketDetail } from "./usePacketDetail"; import { ObservationTable } from "./ObservationTable"; import { buildPacketPaths } from "../map/packet-path"; @@ -35,10 +34,13 @@ export function PacketExpansion({ packet, onOpenAnalyzer, onViewPath, selectedOb // than showing a blank (0-row) skeleton while it loads. const noObservations = packet.observationCount === 0; const emptyState =
No observations
; - // null drops ?analyze, so a link copied while the drawer is open still restores just the row - const copyParams = useCallback( - () => ({ tab: "Packets", hash: packet.packetHash, analyze: null }), - [packet.packetHash], + // Picking an observation is the way into the analyzer — it opens on the one you clicked. + const handleSelectObservation = useCallback( + (id: number) => { + onSelectObservation(id); + onOpenAnalyzer(); + }, + [onSelectObservation, onOpenAnalyzer], ); return ( @@ -53,9 +55,6 @@ export function PacketExpansion({ packet, onOpenAnalyzer, onViewPath, selectedOb first last spread {formatPropagation(spread)} - -
@@ -91,7 +89,7 @@ export function PacketExpansion({ packet, onOpenAnalyzer, onViewPath, selectedOb ) : data && data.observations.length === 0 ? ( emptyState ) : data ? ( - + ) : null}
diff --git a/src/features/packets/PacketTableHeader.tsx b/src/features/packets/PacketTableHeader.tsx index 5366f45..97ae3da 100644 --- a/src/features/packets/PacketTableHeader.tsx +++ b/src/features/packets/PacketTableHeader.tsx @@ -13,6 +13,7 @@ export function PacketTableHeader() { Route Obs Hops + Hash Size Src → Dst IATA Age diff --git a/src/features/packets/PacketTableRow.tsx b/src/features/packets/PacketTableRow.tsx index ff67b15..95a430f 100644 --- a/src/features/packets/PacketTableRow.tsx +++ b/src/features/packets/PacketTableRow.tsx @@ -19,7 +19,8 @@ interface PacketTableRowProps { // expansion instead, which frees the wide column for the packet's endpoints. export function PacketTableRow({ packet, expanded, isFresh, onToggle }: PacketTableRowProps) { // ?? not ||, so a legitimate 0-hop direct packet still shows its count - const hopCount = packet.latestObserver?.pathLength?.hopCount; + const pathLength = packet.latestObserver?.pathLength; + const na = n/a; return (
{packet.scope}} ×{packet.observationCount} - - {hopCount ?? n/a} - + {pathLength?.hopCount ?? na} + {pathLength?.hashSize ?? na} - {packet.latestObserver?.iata ?? n/a} + {packet.latestObserver?.iata ?? {na}} diff --git a/src/features/packets/PacketVirtualList.tsx b/src/features/packets/PacketVirtualList.tsx index 2cf218b..4cc15ad 100644 --- a/src/features/packets/PacketVirtualList.tsx +++ b/src/features/packets/PacketVirtualList.tsx @@ -55,7 +55,7 @@ export function PacketVirtualList({ count: packets.length, getScrollElement: () => parentRef.current, // a collapsed row: one grid line on desktop, a taller card below md. Expanded rows are remeasured. - estimateSize: () => (isMobile ? 64 : 43), + estimateSize: () => (isMobile ? 64 : 37), overscan: 10, getItemKey: (index) => packets[index]?.packetHash ?? index, }); @@ -121,7 +121,8 @@ export function PacketVirtualList({ transform: `translateY(${virtualRow.start}px)`, }} > -
+ {/* cards need breathing room; table rows butt up so the whole strip is a click target */} +
{isMobile ? ( { describe("opening the analyzer from an expanded row", () => { // Regression: handleAnalyze used to reset selectedObservationId on every open, so picking an - // observation inside the expanded row and then opening the analyzer landed on observations[0] - // instead of the one clicked. + // observation inside the expanded row landed the analyzer on observations[0] instead of the one + // clicked. Selecting an observation is now what opens the analyzer, so the two happen together. it("keeps the observation selected in the expanded row", async () => { render(); fireEvent.click(await screen.findByRole("button", { name: "AA11" })); fireEvent.click(await screen.findByText("Observer Three")); - fireEvent.click(screen.getByRole("button", { name: "Open analyzer" })); const drawer = await screen.findByTestId("packet-analyzer-drawer"); expect(within(drawer).getByText("Observer Three")).toBeInTheDocument(); diff --git a/tests/features/packets/PacketExpansion.test.tsx b/tests/features/packets/PacketExpansion.test.tsx index cbbf9c0..889a7df 100644 --- a/tests/features/packets/PacketExpansion.test.tsx +++ b/tests/features/packets/PacketExpansion.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, vi, beforeEach } from "vitest"; import { render, screen, fireEvent } from "@testing-library/react"; import { PacketExpansion } from "../../../src/features/packets/PacketExpansion"; import type { PacketSummary, Observation, PacketDetail } from "../../../src/types/api"; @@ -119,69 +119,58 @@ describe("PacketExpansion", () => { expect(onSelectObservation).toHaveBeenCalledWith(1); }); - it("disables both action buttons while loading", () => { - usePacketDetail.mockReturnValue({ isLoading: true }); - render(); - expect(screen.getByRole("button", { name: "Open analyzer" })).toBeDisabled(); - expect(screen.getByRole("button", { name: "View path on map" })).toBeDisabled(); - }); - - it("disables both action buttons on error", () => { - usePacketDetail.mockReturnValue({ isError: true, refetch: vi.fn() }); - render(); - expect(screen.getByRole("button", { name: "Open analyzer" })).toBeDisabled(); - expect(screen.getByRole("button", { name: "View path on map" })).toBeDisabled(); - }); - - it("enables both action buttons once the fetch resolves with a drawable path", () => { + // Clicking an observation opens the analyzer, so a dedicated button would be a second way to do + // the same thing. Copy Link lives in the analyzer popup only. + it("offers neither an Open analyzer nor a Copy link button", () => { + usePacketDetail.mockReturnValue({ data: detailWithPath() }); + render(); + expect(screen.queryByRole("button", { name: "Open analyzer" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Copy row link" })).not.toBeInTheDocument(); + }); + + it("opens the analyzer on the clicked observation, selecting it first", () => { + const onOpenAnalyzer = vi.fn(); + const onSelectObservation = vi.fn(); + usePacketDetail.mockReturnValue({ data: { packetHash: "AA11", header: header(), observations: [obs(1), obs(2)] } }); + render(); + + fireEvent.click(screen.getByText("Observer 2")); + + expect(onSelectObservation).toHaveBeenCalledWith(2); + expect(onOpenAnalyzer).toHaveBeenCalledTimes(1); + }); + + it("disables View path on map while loading", () => { + usePacketDetail.mockReturnValue({ isLoading: true }); + render(); + expect(screen.getByRole("button", { name: "View path on map" })).toBeDisabled(); + }); + + it("disables View path on map on error", () => { + usePacketDetail.mockReturnValue({ isError: true, refetch: vi.fn() }); + render(); + expect(screen.getByRole("button", { name: "View path on map" })).toBeDisabled(); + }); + + it("enables View path on map once the fetch resolves with a drawable path", () => { usePacketDetail.mockReturnValue({ data: detailWithPath() }); render(); - expect(screen.getByRole("button", { name: "Open analyzer" })).not.toBeDisabled(); expect(screen.getByRole("button", { name: "View path on map" })).not.toBeDisabled(); }); it("disables View path on map when the loaded detail has no resolvable path", () => { usePacketDetail.mockReturnValue({ data: { packetHash: "AA11", header: header(), observations: [obs(1)] } }); render(); - expect(screen.getByRole("button", { name: "Open analyzer" })).not.toBeDisabled(); const viewPathBtn = screen.getByRole("button", { name: "View path on map" }); expect(viewPathBtn).toBeDisabled(); expect(viewPathBtn).toHaveAttribute("title", "No resolved path to map"); }); - it("calls onOpenAnalyzer and onViewPath when their buttons are clicked", () => { - const onOpenAnalyzer = vi.fn(); + it("calls onViewPath when its button is clicked", () => { const onViewPath = vi.fn(); usePacketDetail.mockReturnValue({ data: detailWithPath() }); - render(); - fireEvent.click(screen.getByRole("button", { name: "Open analyzer" })); + render(); fireEvent.click(screen.getByRole("button", { name: "View path on map" })); - expect(onOpenAnalyzer).toHaveBeenCalledTimes(1); expect(onViewPath).toHaveBeenCalledTimes(1); }); }); - -describe("PacketExpansion copy link", () => { - const writeText = vi.fn(); - - beforeEach(() => { - Object.defineProperty(navigator, "clipboard", { value: { writeText }, writable: true, configurable: true }); - writeText.mockClear(); - usePacketDetail.mockReturnValue({ data: detailWithPath() }); - }); - - afterEach(() => window.history.replaceState({}, "", "/")); - - it("copies a link to the expanded row with the analyzer stripped", () => { - window.history.replaceState({}, "", "/?tab=Packets&hash=AA11&analyze=1&iata=YVR"); - render(); - - fireEvent.click(screen.getByRole("button", { name: "Copy row link" })); - - const copied = new URL(writeText.mock.calls[0]![0] as string); - expect(copied.searchParams.get("tab")).toBe("Packets"); - expect(copied.searchParams.get("hash")).toBe("AA11"); - expect(copied.searchParams.has("analyze")).toBe(false); // the drawer must not tag along - expect(copied.searchParams.get("iata")).toBe("YVR"); // unrelated params survive - }); -}); diff --git a/tests/features/packets/PacketTableHeader.test.tsx b/tests/features/packets/PacketTableHeader.test.tsx index 0f85043..de254c0 100644 --- a/tests/features/packets/PacketTableHeader.test.tsx +++ b/tests/features/packets/PacketTableHeader.test.tsx @@ -6,7 +6,7 @@ import { GRID_TEMPLATE } from "../../../src/features/packets/packet-grid"; describe("PacketTableHeader", () => { it("declares every column heading", () => { render(); - for (const h of ["Hash", "Type", "Route", "Obs", "Hops", "Src → Dst", "IATA", "Age"]) { + for (const h of ["Hash", "Type", "Route", "Obs", "Hops", "Hash Size", "Src → Dst", "IATA", "Age"]) { expect(screen.getByText(h)).toBeInTheDocument(); } }); @@ -28,9 +28,9 @@ describe("PacketTableHeader", () => { expect(el.style.gridTemplateColumns).toBe(GRID_TEMPLATE); }); - it("has exactly 9 cells, one per row column including the chevron spacer", () => { + it("has exactly 10 cells, one per row column including the chevron spacer", () => { const { container } = render(); - expect(container.firstElementChild?.children).toHaveLength(9); + expect(container.firstElementChild?.children).toHaveLength(10); }); // Regression: the header and the rows are two independent grids. A `ch` track resolves against diff --git a/tests/features/packets/PacketTableRow.test.tsx b/tests/features/packets/PacketTableRow.test.tsx index d359771..27435ae 100644 --- a/tests/features/packets/PacketTableRow.test.tsx +++ b/tests/features/packets/PacketTableRow.test.tsx @@ -16,10 +16,10 @@ const node = (name: string): ResolvedHop => ({ // pathLength is what makes buildPathSummary produce endpoints at all, so it is always present here. const observer = ( - over: { hopCount?: number } & Partial> = {}, + over: { hopCount?: number; hashSize?: number } & Partial> = {}, ): LatestObserver => { - const { hopCount = 2, ...rest } = over; - return { id: "abcdef1234", iata: "YVR", pathLength: { raw: "1e", hashSize: 1, hopCount }, ...rest }; + const { hopCount = 2, hashSize = 1, ...rest } = over; + return { id: "abcdef1234", iata: "YVR", pathLength: { raw: "1e", hashSize, hopCount }, ...rest }; }; describe("PacketTableRow", () => { @@ -42,10 +42,16 @@ describe("PacketTableRow", () => { expect(screen.queryByText("latest")).not.toBeInTheDocument(); }); - it("falls back to n/a in the hops, endpoint and IATA cells when there is no observer", () => { + it("falls back to n/a in the hops, hash size, endpoint and IATA cells when there is no observer", () => { render( {}} />); const row = within(screen.getByRole("button")); - expect(row.getAllByText("n/a")).toHaveLength(3); + expect(row.getAllByText("n/a")).toHaveLength(4); + }); + + it("shows the hash size alongside the hop count", () => { + render( {}} />); + expect(screen.getByText("5")).toBeInTheDocument(); + expect(screen.getByText("3")).toBeInTheDocument(); }); it("no longer shows the observer, which moved into the expansion", () => { diff --git a/tests/features/packets/PacketVirtualList.test.tsx b/tests/features/packets/PacketVirtualList.test.tsx index 3ead972..96f8028 100644 --- a/tests/features/packets/PacketVirtualList.test.tsx +++ b/tests/features/packets/PacketVirtualList.test.tsx @@ -141,9 +141,11 @@ describe("PacketVirtualList expansion", () => { const handlers = makeHandlers(); render(); - fireEvent.click(screen.getByRole("button", { name: "Open analyzer" })); + // clicking an observation is what opens the analyzer now — there is no button for it + fireEvent.click(screen.getByText("o1")); fireEvent.click(screen.getByRole("button", { name: "View path on map" })); + expect(handlers.onSelectObservation).toHaveBeenCalledWith(1); expect(handlers.onOpenAnalyzer).toHaveBeenCalledTimes(1); expect(handlers.onViewPath).toHaveBeenCalledTimes(1); });