From 1f3fb5b7c932cf77439cb11ab308aa2eb5b3b6b2 Mon Sep 17 00:00:00 2001 From: MrAlders0n Date: Mon, 27 Jul 2026 22:06:05 -0400 Subject: [PATCH] Collapse the packet row to a single line with hops and endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header and the rows are separate grids, so the shared track list can't use ch or auto — each resolved them differently and the columns drifted apart. Observer moves into the expansion to free the column. --- src/features/packets/PacketEndpoints.tsx | 47 ++++++++ src/features/packets/PacketExpansion.tsx | 7 ++ src/features/packets/PacketPathLine.tsx | 57 --------- src/features/packets/PacketTableHeader.tsx | 3 +- src/features/packets/PacketTableRow.tsx | 22 ++-- src/features/packets/PacketVirtualList.tsx | 3 +- src/features/packets/packet-grid.ts | 8 +- .../features/packets/PacketEndpoints.test.tsx | 67 ++++++++++ .../features/packets/PacketPathLine.test.tsx | 114 ------------------ .../packets/PacketTableHeader.test.tsx | 19 ++- .../features/packets/PacketTableRow.test.tsx | 67 +++++++--- 11 files changed, 210 insertions(+), 204 deletions(-) create mode 100644 src/features/packets/PacketEndpoints.tsx delete mode 100644 src/features/packets/PacketPathLine.tsx create mode 100644 tests/features/packets/PacketEndpoints.test.tsx delete mode 100644 tests/features/packets/PacketPathLine.test.tsx diff --git a/src/features/packets/PacketEndpoints.tsx b/src/features/packets/PacketEndpoints.tsx new file mode 100644 index 0000000..33e5822 --- /dev/null +++ b/src/features/packets/PacketEndpoints.tsx @@ -0,0 +1,47 @@ +import type { PacketSummary } from "../../types/api"; +import type { PathConfidence } from "../../types/enums"; +import { buildPathSummary, type PathChip } from "./path-summary"; + +// Same three-state vocabulary PathData uses in the analyzer. +const CONFIDENCE_CLASSES: Record = { + high: "bg-green/8 text-green", + ambiguous: "bg-warn/8 text-warn", + none: "bg-text-muted/8 text-text-dim", +}; + +function Chip({ chip }: { chip: PathChip }) { + if (chip.kind === "hex") { + return {chip.label}; + } + if (chip.kind === "unresolved-run") { + return ( + + {chip.count === 1 ? "?" : `?×${chip.count}`} + + ); + } + return ( + + {chip.label} + + ); +} + +const Na = () => n/a; + +// The packet's logical endpoints. beacon-server resolves these on the WS feed only and leaves them +// nil on the REST list, so scrollback rows read n/a — as do payload types with no addressed +// endpoint at all (GRP_TXT/GRP_DATA/TRACE). +export function PacketEndpoints({ packet }: { packet: PacketSummary }) { + const { source, destination } = buildPathSummary(packet); + // One n/a for the pair reads better than "n/a → n/a" on every historical row. + if (!source && !destination) return ; + + return ( + + {source ? : } + → + {destination ? : } + + ); +} diff --git a/src/features/packets/PacketExpansion.tsx b/src/features/packets/PacketExpansion.tsx index 30bb1b7..d609c88 100644 --- a/src/features/packets/PacketExpansion.tsx +++ b/src/features/packets/PacketExpansion.tsx @@ -25,6 +25,7 @@ interface Props { // the per-observer table, which does wait on usePacketDetail. export function PacketExpansion({ packet, onOpenAnalyzer, onViewPath, selectedObservationId, onSelectObservation }: Props) { const { data, isLoading, isError, refetch } = usePacketDetail(packet.packetHash); + const observer = packet.latestObserver; // firstHeardAt/lastHeardAt are epoch ms (same unit Timestamp expects), so the difference is // already in ms for formatPropagation -- no *1000 here. const spread = packet.lastHeardAt - packet.firstHeardAt; @@ -43,6 +44,12 @@ export function PacketExpansion({ packet, onOpenAnalyzer, onViewPath, selectedOb return (
+ + observer{" "} + {observer + ? {observer.displayName ?? observer.id.slice(0, 8)} + : n/a} + first last spread {formatPropagation(spread)} diff --git a/src/features/packets/PacketPathLine.tsx b/src/features/packets/PacketPathLine.tsx deleted file mode 100644 index 427dde6..0000000 --- a/src/features/packets/PacketPathLine.tsx +++ /dev/null @@ -1,57 +0,0 @@ -import type { PacketSummary } from "../../types/api"; -import type { PathConfidence } from "../../types/enums"; -import { buildPathSummary, type PathChip } from "./path-summary"; - -// Same three-state vocabulary PathData uses in the analyzer. -const CONFIDENCE_CLASSES: Record = { - high: "bg-green/8 text-green", - ambiguous: "bg-warn/8 text-warn", - none: "bg-text-muted/8 text-text-dim", -}; - -function Chip({ chip }: { chip: PathChip }) { - if (chip.kind === "hex") { - return {chip.label}; - } - if (chip.kind === "unresolved-run") { - return ( - - {chip.count === 1 ? "?" : `?×${chip.count}`} - - ); - } - return ( - - {chip.label} - - ); -} - -// Line 2 of a packet row: always the latest observation's path, never the selected one. -export function PacketPathLine({ packet }: { packet: PacketSummary }) { - const summary = buildPathSummary(packet); - - if (summary.isNa) { - return n/a; - } - - return ( -
- latest - {summary.hopLabel} - {summary.chips.map((chip, i) => )} - {summary.overflow > 0 && ( - - +{summary.overflow} more - - )} - {(summary.source || summary.destination) && ( - - {summary.source ? : n/a} - → - {summary.destination ? : n/a} - - )} -
- ); -} diff --git a/src/features/packets/PacketTableHeader.tsx b/src/features/packets/PacketTableHeader.tsx index a9bb568..5366f45 100644 --- a/src/features/packets/PacketTableHeader.tsx +++ b/src/features/packets/PacketTableHeader.tsx @@ -12,7 +12,8 @@ export function PacketTableHeader() { Type Route Obs - Observer + Hops + Src → Dst IATA Age
diff --git a/src/features/packets/PacketTableRow.tsx b/src/features/packets/PacketTableRow.tsx index 5433e94..ff67b15 100644 --- a/src/features/packets/PacketTableRow.tsx +++ b/src/features/packets/PacketTableRow.tsx @@ -6,7 +6,7 @@ import { payloadTypeVariant } from "../../components/badge-utils"; import { PAYLOAD_TYPE_NAMES, type PayloadTypeValue } from "../../types/enums"; import type { PacketSummary } from "../../types/api"; import { GRID_TEMPLATE } from "./packet-grid"; -import { PacketPathLine } from "./PacketPathLine"; +import { PacketEndpoints } from "./PacketEndpoints"; interface PacketTableRowProps { packet: PacketSummary; @@ -15,10 +15,11 @@ interface PacketTableRowProps { onToggle: () => void; } -// two-line table row: line 1 is the identity grid (shares GRID_TEMPLATE with the header), line 2 -// is the latest path. The whole row is the expansion click target. +// Single-line table row sharing GRID_TEMPLATE with the sticky header. The observer lives in the +// expansion instead, which frees the wide column for the packet's endpoints. export function PacketTableRow({ packet, expanded, isFresh, onToggle }: PacketTableRowProps) { - const observer = packet.latestObserver; + // ?? not ||, so a legitimate 0-hop direct packet still shows its count + const hopCount = packet.latestObserver?.pathLength?.hopCount; return (
{packet.scope}} ×{packet.observationCount} - - {observer ? (observer.displayName ?? observer.id.slice(0, 8)) : n/a} + + {hopCount ?? n/a} + + + - {observer?.iata ?? n/a} + {packet.latestObserver?.iata ?? n/a} - -
- -
); } diff --git a/src/features/packets/PacketVirtualList.tsx b/src/features/packets/PacketVirtualList.tsx index 8eed3e2..2cf218b 100644 --- a/src/features/packets/PacketVirtualList.tsx +++ b/src/features/packets/PacketVirtualList.tsx @@ -54,7 +54,8 @@ export function PacketVirtualList({ const virtualizer = useVirtualizer({ count: packets.length, getScrollElement: () => parentRef.current, - estimateSize: () => 64, // a collapsed two-line row; expanded ones are remeasured + // a collapsed row: one grid line on desktop, a taller card below md. Expanded rows are remeasured. + estimateSize: () => (isMobile ? 64 : 43), overscan: 10, getItemKey: (index) => packets[index]?.packetHash ?? index, }); diff --git a/src/features/packets/packet-grid.ts b/src/features/packets/packet-grid.ts index 0a5aec0..2845d8f 100644 --- a/src/features/packets/packet-grid.ts +++ b/src/features/packets/packet-grid.ts @@ -1,3 +1,5 @@ -// One track list shared by the sticky header and every row, so columns stay aligned. Line 2 spans -// all of it via grid-column: 1 / -1. -export const GRID_TEMPLATE = "1.25rem minmax(7ch,auto) 6rem 5rem 3rem minmax(8rem,1fr) 3.5rem 5rem"; +// One track list shared by the sticky header and every row, so columns stay aligned. Keep every +// track font- and content-independent: the header and the rows are separate grids, so a `ch` track +// resolves against each one's own font size (9px vs 11px) and an `auto` track against its own text +// ("HASH" vs "4AE77F09") — either silently drifts the two apart. +export const GRID_TEMPLATE = "1.25rem 5rem 6rem 5rem 3rem 3rem minmax(6rem,1fr) 3.5rem 5rem"; diff --git a/tests/features/packets/PacketEndpoints.test.tsx b/tests/features/packets/PacketEndpoints.test.tsx new file mode 100644 index 0000000..037489c --- /dev/null +++ b/tests/features/packets/PacketEndpoints.test.tsx @@ -0,0 +1,67 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { PacketEndpoints } from "../../../src/features/packets/PacketEndpoints"; +import type { LatestObserver, PacketSummary } from "../../../src/types/api"; + +const pkt = (observer?: LatestObserver): PacketSummary => ({ + packetHash: "AA11", payloadType: 1, payloadTypeName: "ADVERT", + routeType: 1, routeTypeName: "FLOOD", + firstHeardAt: 0, lastHeardAt: 0, observationCount: 1, latestObserver: observer, +}); + +const obs = (over: Partial = {}): LatestObserver => ({ + id: "o1", iata: "YVR", pathLength: { raw: "00", hashSize: 1, hopCount: 0 }, ...over, +}); + +describe("PacketEndpoints", () => { + it("renders a single n/a when there is no observer at all", () => { + render(); + expect(screen.getByText("n/a")).toBeInTheDocument(); + }); + + it("renders both endpoints with the arrow glyph between them", () => { + render(); + expect(screen.getByText("SrcNode")).toBeInTheDocument(); + expect(screen.getByText("DstNode")).toBeInTheDocument(); + expect(screen.getByText("→")).toBeInTheDocument(); + }); + + it("shows n/a for a missing endpoint while the present one still renders", () => { + render(); + expect(screen.getByText("SrcNode")).toBeInTheDocument(); + expect(screen.getByText("n/a")).toBeInTheDocument(); + }); + + // The REST list leaves both nil, so this is the common scrollback case — one n/a, no arrow. + it("collapses to a single n/a when both endpoints are absent", () => { + render(); + expect(screen.getByText("n/a")).toBeInTheDocument(); + expect(screen.queryByText("→")).not.toBeInTheDocument(); + }); + + it("tints an ambiguous endpoint with the warn token", () => { + render(); + expect(screen.getByText("Raven").className).toContain("text-warn"); + }); + + it("tints a high-confidence endpoint with the green token", () => { + render(); + expect(screen.getByText("Falcon").className).toContain("text-green"); + }); + + it("renders a bare ? for an endpoint the backend could not resolve", () => { + render(); + expect(screen.getByText("?")).toBeInTheDocument(); + }); +}); diff --git a/tests/features/packets/PacketPathLine.test.tsx b/tests/features/packets/PacketPathLine.test.tsx deleted file mode 100644 index 0843834..0000000 --- a/tests/features/packets/PacketPathLine.test.tsx +++ /dev/null @@ -1,114 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { render, screen } from "@testing-library/react"; -import { PacketPathLine } from "../../../src/features/packets/PacketPathLine"; -import type { PacketSummary } from "../../../src/types/api"; - -const pkt = (over: Partial = {}): PacketSummary => ({ - packetHash: "AA11", payloadType: 1, payloadTypeName: "ADVERT", - routeType: 1, routeTypeName: "FLOOD", - firstHeardAt: 0, lastHeardAt: 0, observationCount: 1, ...over, -}); - -describe("PacketPathLine", () => { - it("renders a single n/a when there is nothing to show", () => { - render(); - expect(screen.getByText("n/a")).toBeInTheDocument(); - }); - - it("renders hop label, hex chips and the overflow count", () => { - render(); - expect(screen.getByText("14 hops")).toBeInTheDocument(); - expect(screen.getByText("00")).toBeInTheDocument(); - expect(screen.getByText("+9 more")).toBeInTheDocument(); - }); - - it("tints an ambiguous hop with the warn token", () => { - render(); - expect(screen.getByText("Raven").className).toContain("text-warn"); - }); - - it("tints a high-confidence hop with the green token", () => { - render(); - expect(screen.getByText("Falcon").className).toContain("text-green"); - }); - - it("renders both endpoints with the arrow glyph between them", () => { - render(); - expect(screen.getByText("SrcNode")).toBeInTheDocument(); - expect(screen.getByText("DstNode")).toBeInTheDocument(); - expect(screen.getByText("→")).toBeInTheDocument(); - }); - - it("shows n/a for a missing endpoint while the present one still renders", () => { - render(); - expect(screen.getByText("SrcNode")).toBeInTheDocument(); - expect(screen.getByText("n/a")).toBeInTheDocument(); - }); - - it("omits the endpoint block entirely when both endpoints are absent", () => { - render(); - expect(screen.queryByText("→")).not.toBeInTheDocument(); - }); - - it("renders a bare ? for a single unresolved hop", () => { - render(); - expect(screen.getByText("?")).toBeInTheDocument(); - }); - - it("collapses a run of unresolved hops into a single ?×N chip", () => { - render(); - expect(screen.getByText("?×3")).toBeInTheDocument(); - }); -}); diff --git a/tests/features/packets/PacketTableHeader.test.tsx b/tests/features/packets/PacketTableHeader.test.tsx index 1b7ecad..0f85043 100644 --- a/tests/features/packets/PacketTableHeader.test.tsx +++ b/tests/features/packets/PacketTableHeader.test.tsx @@ -6,11 +6,16 @@ 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", "Observer", "IATA", "Age"]) { + for (const h of ["Hash", "Type", "Route", "Obs", "Hops", "Src → Dst", "IATA", "Age"]) { expect(screen.getByText(h)).toBeInTheDocument(); } }); + it("no longer heads an observer column, which moved into the expansion", () => { + render(); + expect(screen.queryByText("Observer")).not.toBeInTheDocument(); + }); + it("is hidden below md", () => { const { container } = render(); expect(container.firstElementChild?.className).toContain("hidden"); @@ -23,9 +28,17 @@ describe("PacketTableHeader", () => { expect(el.style.gridTemplateColumns).toBe(GRID_TEMPLATE); }); - it("has exactly 8 cells, one per row column including the chevron spacer", () => { + it("has exactly 9 cells, one per row column including the chevron spacer", () => { const { container } = render(); - expect(container.firstElementChild?.children).toHaveLength(8); + expect(container.firstElementChild?.children).toHaveLength(9); + }); + + // Regression: the header and the rows are two independent grids. A `ch` track resolves against + // each one's own font size (header 9px vs row 11px) and an `auto`/`min-content` track against its + // own content ("HASH" vs "4AE77F09"), so either kind silently drifts the columns apart. + it("sizes every track in font-independent units so both grids resolve identically", () => { + expect(GRID_TEMPLATE).not.toMatch(/\bch\b/); + expect(GRID_TEMPLATE).not.toMatch(/auto|min-content|max-content|fit-content/); }); it("leaves the leading chevron-alignment cell unlabeled and hidden from screen readers", () => { diff --git a/tests/features/packets/PacketTableRow.test.tsx b/tests/features/packets/PacketTableRow.test.tsx index 7a5e7c4..d359771 100644 --- a/tests/features/packets/PacketTableRow.test.tsx +++ b/tests/features/packets/PacketTableRow.test.tsx @@ -1,7 +1,7 @@ import { describe, it, expect, vi } from "vitest"; import { render, screen, fireEvent, within } from "@testing-library/react"; import { PacketTableRow } from "../../../src/features/packets/PacketTableRow"; -import type { PacketSummary } from "../../../src/types/api"; +import type { LatestObserver, PacketSummary, ResolvedHop } from "../../../src/types/api"; const pkt = (over: Partial = {}): PacketSummary => ({ packetHash: "AA11BB22", payloadType: 1, payloadTypeName: "ADVERT", @@ -9,6 +9,19 @@ const pkt = (over: Partial = {}): PacketSummary => ({ firstHeardAt: 1700000000, lastHeardAt: 1700000000, observationCount: 3, ...over, }); +const node = (name: string): ResolvedHop => ({ + confidence: "high", + nodes: [{ id: "n-1", name, publicKey: "aabbccdd" }], +}); + +// pathLength is what makes buildPathSummary produce endpoints at all, so it is always present here. +const observer = ( + over: { hopCount?: number } & Partial> = {}, +): LatestObserver => { + const { hopCount = 2, ...rest } = over; + return { id: "abcdef1234", iata: "YVR", pathLength: { raw: "1e", hashSize: 1, hopCount }, ...rest }; +}; + describe("PacketTableRow", () => { it("exposes one button carrying the expansion state", () => { render( {}} />); @@ -23,27 +36,53 @@ describe("PacketTableRow", () => { expect(onToggle).toHaveBeenCalledOnce(); }); - it("renders line 2 even when there is no path data, so row height is constant", () => { - render( {}} />); - expect(screen.getAllByText("n/a")).toHaveLength(3); + it("is a single line, so the row height stays constant for the virtualizer", () => { + const { container } = render( {}} />); + expect(container.querySelectorAll("button")).toHaveLength(1); + expect(screen.queryByText("latest")).not.toBeInTheDocument(); }); - it("falls back to n/a in the observer and IATA cells when there is no observer", () => { + it("falls back to n/a in the hops, endpoint and IATA cells when there is no observer", () => { render( {}} />); - const line1 = within(screen.getByRole("button")); - expect(line1.getAllByText("n/a")).toHaveLength(2); + const row = within(screen.getByRole("button")); + expect(row.getAllByText("n/a")).toHaveLength(3); }); - it("falls back to the observer id when there is no display name", () => { - render( {}} />); - expect(screen.getByText("abcdef12")).toBeInTheDocument(); + it("no longer shows the observer, which moved into the expansion", () => { + render( {}} />); + expect(screen.queryByText("Cypress Peak")).not.toBeInTheDocument(); + expect(screen.queryByText("abcdef12")).not.toBeInTheDocument(); expect(screen.getByText("YVR")).toBeInTheDocument(); }); - it("prefers the observer display name over the id", () => { - render( {}} />); - expect(screen.getByText("Cypress Peak")).toBeInTheDocument(); - expect(screen.queryByText("abcdef12")).not.toBeInTheDocument(); + it("shows the hop count, which the REST list carries on every row", () => { + render( {}} />); + expect(screen.getByText("3")).toBeInTheDocument(); + }); + + it("renders resolved endpoints when the WS feed supplied them", () => { + render( + {}} + />, + ); + expect(screen.getByText("Laprairie")).toBeInTheDocument(); + expect(screen.getByText("YUL1")).toBeInTheDocument(); + }); + + // The REST list leaves resolvedSource/Destination nil on purpose, so scrollback rows show one n/a + // for the pair rather than "n/a → n/a". + it("collapses the endpoint cell to a single n/a when neither endpoint resolved", () => { + render( {}} />); + expect(screen.getAllByText("n/a")).toHaveLength(1); + }); + + it("still marks the missing half when only one endpoint resolved", () => { + render( {}} />); + expect(screen.getByText("Laprairie")).toBeInTheDocument(); + expect(screen.getAllByText("n/a")).toHaveLength(1); }); it("reflects the expanded state on the button and chevron", () => {