Merge pull request #522 from danditomaso/fix-node-details-styling

fix: styling issues in NodeDialog & TraceRoute components

src/components/Dialog/DialogManager.tsx

@@ -6,7 +6,7 @@ import { PkiBackupDialog } from "@components/Dialog/PKIBackupDialog.tsx";
 import { QRDialog } from "@components/Dialog/QRDialog.tsx";
 import { RebootDialog } from "@components/Dialog/RebootDialog.tsx";
 import { ShutdownDialog } from "@components/Dialog/ShutdownDialog.tsx";
-import { NodeDetailsDialog } from "@components/Dialog/NodeDetailsDialog.tsx";
+import { NodeDetailsDialog } from "@components/Dialog/NodeDetailsDialog/NodeDetailsDialog.tsx";
 import { UnsafeRolesDialog } from "@components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.tsx";
 
 export const DialogManager = () => {

src/components/Dialog/NodeDetailsDialog.tsx

@@ -1,190 +0,0 @@
-import { useAppStore } from "../../core/stores/appStore.ts";
-import { useDevice } from "../../core/stores/deviceStore.ts";
-import {
-  Accordion,
-  AccordionContent,
-  AccordionItem,
-  AccordionTrigger,
-} from "../UI/Accordion.tsx";
-import {
-  Dialog,
-  DialogClose,
-  DialogContent,
-  DialogFooter,
-  DialogHeader,
-  DialogTitle,
-} from "../UI/Dialog.tsx";
-import { Protobuf } from "@meshtastic/core";
-import { numberToHexUnpadded } from "@noble/curves/abstract/utils";
-import { DeviceImage } from "../generic/DeviceImage.tsx";
-import { TimeAgo } from "../generic/TimeAgo.tsx";
-import { Uptime } from "../generic/Uptime.tsx";
-
-export interface NodeDetailsDialogProps {
-  open: boolean;
-  onOpenChange: (open: boolean) => void;
-}
-
-export const NodeDetailsDialog = ({
-  open,
-  onOpenChange,
-}: NodeDetailsDialogProps) => {
-  const { nodes } = useDevice();
-  const { nodeNumDetails } = useAppStore();
-  const device: Protobuf.Mesh.NodeInfo = nodes.get(nodeNumDetails);
-
-  return device
-    ? (
-      <Dialog open={open} onOpenChange={onOpenChange}>
-        <DialogContent>
-          <DialogClose />
-          <DialogHeader>
-            <DialogTitle>
-              Node Details for {device.user?.longName ?? "UNKNOWN"} (
-              {device.user?.shortName ?? "UNK"})
-            </DialogTitle>
-          </DialogHeader>
-          <DialogFooter>
-            <div className="w-full">
-              <DeviceImage
-                className="w-32 h-32 mx-auto rounded-lg border-4 border-slate-200 dark:border-slate-800"
-                deviceType={Protobuf.Mesh
-                  .HardwareModel[device.user?.hwModel ?? 0]}
-              />
-              <div className="bg-slate-100 dark:bg-slate-800 p-3 rounded-lg mt-3">
-                <p className="text-lg font-semibold text-slate-900 dark:text-slate-50">
-                  Details:
-                </p>
-                <p>
-                  Hardware:{" "}
-                  {Protobuf.Mesh.HardwareModel[device.user?.hwModel ?? 0]}
-                </p>
-                <p>Node Number: {device.num}</p>
-                <p>Node HEX: !{numberToHexUnpadded(device.num)}</p>
-                <p>
-                  Role: {Protobuf.Config.Config_DeviceConfig_Role[
-                    device.user?.role ?? 0
-                  ]}
-                </p>
-                <p>
-                  Last Heard: {device.lastHeard === 0
-                    ? (
-                      "Never"
-                    )
-                    : <TimeAgo timestamp={device.lastHeard * 1000} />}
-                </p>
-              </div>
-
-              {device.position
-                ? (
-                  <div className="mt-5 bg-slate-100 dark:bg-slate-800 p-3 rounded-lg mt-3">
-                    <p className="text-lg font-semibold text-slate-900 dark:text-slate-50">
-                      Position:
-                    </p>
-                    {device.position.latitudeI && device.position.longitudeI
-                      ? (
-                        <p>
-                          Coordinates:{" "}
-                          <a
-                            className="text-blue-500 dark:text-blue-400"
-                            href={`https://www.openstreetmap.org/?mlat=${device.position.latitudeI / 1e7
-                              }&mlon=${device.position.longitudeI / 1e7
-                              }&layers=N`}
-                            target="_blank"
-                            rel="noreferrer"
-                          >
-                            {device.position.latitudeI / 1e7},{" "}
-                            {device.position.longitudeI / 1e7}
-                          </a>
-                        </p>
-                      )
-                      : null}
-                    {device.position.altitude
-                      ? <p>Altitude: {device.position.altitude}m</p>
-                      : null}
-                  </div>
-                )
-                : null}
-
-              {device.deviceMetrics
-                ? (
-                  <div className="mt-5 bg-slate-100 dark:bg-slate-800 p-3 rounded-lg mt-3">
-                    <p className="text-lg font-semibold text-slate-900 dark:text-slate-50">
-                      Device Metrics:
-                    </p>
-                    {device.deviceMetrics.airUtilTx
-                      ? (
-                        <p>
-                          Air TX utilization:{" "}
-                          {device.deviceMetrics.airUtilTx.toFixed(2)}%
-                        </p>
-                      )
-                      : null}
-                    {device.deviceMetrics.channelUtilization
-                      ? (
-                        <p>
-                          Channel utilization:{" "}
-                          {device.deviceMetrics.channelUtilization.toFixed(2)}%
-                        </p>
-                      )
-                      : null}
-                    {device.deviceMetrics.batteryLevel
-                      ? (
-                        <p>
-                          Battery level:{" "}
-                          {device.deviceMetrics.batteryLevel.toFixed(2)}%
-                        </p>
-                      )
-                      : null}
-                    {device.deviceMetrics.voltage
-                      ? (
-                        <p>
-                          Voltage: {device.deviceMetrics.voltage.toFixed(2)}V
-                        </p>
-                      )
-                      : null}
-                    {device.deviceMetrics.uptimeSeconds
-                      ? (
-                        <p>
-                          Uptime:{" "}
-                          <Uptime
-                            seconds={device.deviceMetrics.uptimeSeconds}
-                          />
-                        </p>
-                      )
-                      : null}
-                  </div>
-                )
-                : null}
-
-              {device
-                ? (
-                  <div className="mt-5 w-full max-w-[464px] bg-slate-100 dark:bg-slate-800 p-3 rounded-lg mt-3">
-                    <Accordion
-                      className="AccordionRoot"
-                      type="single"
-                      collapsible
-                    >
-                      <AccordionItem className="AccordionItem" value="item-1">
-                        <AccordionTrigger>
-                          <p className="text-lg font-semibold text-slate-900 dark:text-slate-50">
-                            All Raw Metrics:
-                          </p>
-                        </AccordionTrigger>
-                        <AccordionContent className="overflow-x-scroll">
-                          <pre className="text-xs w-full">
-                            {JSON.stringify(device, null, 2)}
-                          </pre>
-                        </AccordionContent>
-                      </AccordionItem>
-                    </Accordion>
-                  </div>
-                )
-                : null}
-            </div>
-          </DialogFooter>
-        </DialogContent>
-      </Dialog>
-    )
-    : null;
-};

src/components/Dialog/NodeDetailsDialog/NodeDetailsDialog.test.tsx

@@ -0,0 +1,73 @@
+import { describe, it, vi, expect, beforeEach, Mock } from "vitest";
+import { render, screen } from "@testing-library/react";
+import { NodeDetailsDialog } from "@components/Dialog/NodeDetailsDialog/NodeDetailsDialog.tsx";
+import { useDevice } from "@core/stores/deviceStore.ts";
+import { useAppStore } from "@core/stores/appStore.ts";
+
+vi.mock("@core/stores/deviceStore");
+vi.mock("@core/stores/appStore");
+
+describe("NodeDetailsDialog", () => {
+  const mockDevice = {
+    num: 1234,
+    user: {
+      longName: "Test Node",
+      shortName: "TN",
+      hwModel: 1,
+      role: 1,
+    },
+    lastHeard: 1697500000,
+    position: {
+      latitudeI: 450000000,
+      longitudeI: -750000000,
+      altitude: 200,
+    },
+    deviceMetrics: {
+      airUtilTx: 50.123,
+      channelUtilization: 75.456,
+      batteryLevel: 88.789,
+      voltage: 4.2,
+      uptimeSeconds: 3600,
+    },
+  };
+
+  beforeEach(() => {
+    // Reset mocks before each test
+    vi.resetAllMocks();
+
+    (useDevice as Mock).mockReturnValue({
+      nodes: new Map([[1234, mockDevice]]),
+    });
+
+    (useAppStore as unknown as Mock).mockReturnValue({
+      nodeNumDetails: 1234,
+    });
+  });
+
+  it("renders node details correctly", () => {
+    render(<NodeDetailsDialog open={true} onOpenChange={() => { }} />);
+
+    expect(screen.getByText(/Node Details for Test Node/i)).toBeInTheDocument();
+
+    expect(screen.getByText("Node Number: 1234")).toBeInTheDocument();
+
+    expect(screen.getByText(/Air TX utilization: 50.12%/i)).toBeInTheDocument();
+    expect(screen.getByText(/Channel utilization: 75.46%/i)).toBeInTheDocument();
+    expect(screen.getByText(/Battery level: 88.79%/i)).toBeInTheDocument();
+    expect(screen.getByText(/Voltage: 4.20V/i)).toBeInTheDocument();
+    expect(screen.getByText(/Uptime:/i)).toBeInTheDocument();
+    expect(screen.getByText(/Coordinates:/i)).toBeInTheDocument();
+    expect(screen.getByText("45, -75")).toBeInTheDocument();
+    expect(screen.getByText(/Altitude: 200m/i)).toBeInTheDocument();
+    expect(screen.getByText(/Role:/i)).toBeInTheDocument();
+  });
+
+  it("renders null if device is not found", () => {
+    (useDevice as Mock).mockReturnValue({
+      nodes: new Map(),
+    });
+
+    render(<NodeDetailsDialog open={true} onOpenChange={() => { }} />);
+    expect(screen.queryByText(/Node Details for/i)).not.toBeInTheDocument();
+  });
+});

src/components/Dialog/NodeDetailsDialog/NodeDetailsDialog.tsx

@@ -0,0 +1,177 @@
+import { useAppStore } from "@core/stores/appStore.ts";
+import { useDevice } from "@core/stores/deviceStore.ts";
+import {
+  Accordion,
+  AccordionContent,
+  AccordionItem,
+  AccordionTrigger,
+} from "@components/UI/Accordion.tsx";
+import {
+  Dialog,
+  DialogClose,
+  DialogContent,
+  DialogFooter,
+  DialogHeader,
+  DialogTitle,
+} from "@components/UI/Dialog.tsx";
+import { Protobuf } from "@meshtastic/core";
+import { numberToHexUnpadded } from "@noble/curves/abstract/utils";
+import { DeviceImage } from "@components/generic/DeviceImage.tsx";
+import { TimeAgo } from "@components/generic/TimeAgo.tsx";
+import { Uptime } from "@components/generic/Uptime.tsx";
+
+export interface NodeDetailsDialogProps {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+}
+
+export const NodeDetailsDialog = ({
+  open,
+  onOpenChange,
+}: NodeDetailsDialogProps) => {
+  const { nodes } = useDevice();
+  const { nodeNumDetails } = useAppStore();
+
+  const device = nodes.get(nodeNumDetails);
+
+  if (!device) return null;
+
+  const deviceMetricsMap = [
+    {
+      key: "airUtilTx",
+      label: "Air TX utilization",
+      value: device.deviceMetrics?.airUtilTx,
+      format: (val: number) => `${val.toFixed(2)}%`,
+    },
+    {
+      key: "channelUtilization",
+      label: "Channel utilization",
+      value: device.deviceMetrics?.channelUtilization,
+      format: (val: number) => `${val.toFixed(2)}%`,
+    },
+    {
+      key: "batteryLevel",
+      label: "Battery level",
+      value: device.deviceMetrics?.batteryLevel,
+      format: (val: number) => `${val.toFixed(2)}%`,
+    },
+    {
+      key: "voltage",
+      label: "Voltage",
+      value: device.deviceMetrics?.voltage,
+      format: (val: number) => `${val.toFixed(2)}V`,
+    },
+  ];
+
+  return (
+    <Dialog open={open} onOpenChange={onOpenChange}>
+      <DialogContent >
+        <DialogClose />
+        <DialogHeader>
+          <DialogTitle>
+            Node Details for {device.user?.longName ?? "UNKNOWN"} (
+            {device.user?.shortName ?? "UNK"})
+          </DialogTitle>
+        </DialogHeader>
+        <DialogFooter>
+          <div className="w-full">
+            <div className="flex flex-col">
+              <DeviceImage
+                className="w-32 h-32 mx-auto rounded-lg border-4 border-slate-200 dark:border-slate-800"
+                deviceType={
+                  Protobuf.Mesh.HardwareModel[device.user?.hwModel ?? 0]
+                }
+              />
+              <div className="bg-slate-100 text-slate-900 dark:text-slate-100 dark:bg-slate-800 p-3 rounded-lg mt-3">
+                <p className="text-lg font-semibold">Details:</p>
+                <p>
+                  Hardware:{" "}
+                  {Protobuf.Mesh.HardwareModel[device.user?.hwModel ?? 0]}
+                </p>
+                <p>Node Number: {device.num}</p>
+                <p>Node Hex: !{numberToHexUnpadded(device.num)}</p>
+                <p>
+                  Role:{" "}
+                  {
+                    Protobuf.Config.Config_DeviceConfig_Role[
+                    device.user?.role ?? 0
+                    ]
+                  }
+                </p>
+                <p>
+                  Last Heard:{" "}
+                  {device.lastHeard === 0 ? "Never" : <TimeAgo timestamp={device.lastHeard * 1000} />}
+                </p>
+              </div>
+
+              {device.position && (
+                <div className="text-slate-900 dark:text-slate-100 bg-slate-100 dark:bg-slate-800 p-3 rounded-lg mt-3">
+                  <p className="text-lg font-semibold">Position:</p>
+                  {device.position.latitudeI && device.position.longitudeI && (
+                    <p>
+                      Coordinates:{" "}
+                      <a
+                        className="text-blue-500 dark:text-blue-400"
+                        href={`https://www.openstreetmap.org/?mlat=${device.position.latitudeI / 1e7
+                          }&mlon=${device.position.longitudeI / 1e7
+                          }&layers=N`}
+                        target="_blank"
+                        rel="noreferrer"
+                      >
+                        {device.position.latitudeI / 1e7},{" "}
+                        {device.position.longitudeI / 1e7}
+                      </a>
+                    </p>
+                  )}
+                  {device.position.altitude && (
+                    <p>Altitude: {device.position.altitude}m</p>
+                  )}
+                </div>
+              )}
+
+              {device.deviceMetrics && (
+                <div className="text-slate-900 dark:text-slate-100 bg-slate-100 dark:bg-slate-800 p-3 rounded-lg mt-3">
+                  <p className="text-lg font-semibold text-slate-900 dark:text-slate-50">
+                    Device Metrics:
+                  </p>
+                  {deviceMetricsMap.map(
+                    (metric) =>
+                      metric.value !== undefined && (
+                        <p key={metric.key}>
+                          {metric.label}: {metric.format(metric.value)}
+                        </p>
+                      )
+                  )}
+                  {device.deviceMetrics.uptimeSeconds && (
+                    <p>
+                      Uptime:{" "}
+                      <Uptime seconds={device.deviceMetrics.uptimeSeconds} />
+                    </p>
+                  )}
+                </div>
+              )}
+
+            </div>
+
+            <div className="text-slate-900 dark:text-slate-100 w-full max-w-[464px] bg-slate-100 dark:bg-slate-800 p-3 rounded-lg mt-3">
+              <Accordion className="AccordionRoot" type="single" collapsible>
+                <AccordionItem className="AccordionItem" value="item-1">
+                  <AccordionTrigger>
+                    <p className="text-lg font-semibold text-slate-900 dark:text-slate-50">
+                      All Raw Metrics:
+                    </p>
+                  </AccordionTrigger>
+                  <AccordionContent className="overflow-x-scroll">
+                    <pre className="text-xs w-full">
+                      {JSON.stringify(device, null, 2)}
+                    </pre>
+                  </AccordionContent>
+                </AccordionItem>
+              </Accordion>
+            </div>
+          </div>
+        </DialogFooter>
+      </DialogContent>
+    </Dialog>
+  );
+};

src/components/PageComponents/Messages/TraceRoute.test.tsx

@@ -0,0 +1,95 @@
+import { describe, it, expect, vi, beforeEach, Mock } from "vitest";
+import { render, screen } from "@testing-library/react";
+import { TraceRoute } from "@components/PageComponents/Messages/TraceRoute.tsx";
+import { useDevice } from "@core/stores/deviceStore.ts";
+
+vi.mock("@core/stores/deviceStore");
+
+describe("TraceRoute", () => {
+  const mockNodes = new Map([
+    [
+      1,
+      { num: 1, user: { longName: "Node A" } },
+    ],
+    [
+      2,
+      { num: 2, user: { longName: "Node B" } },
+    ],
+    [
+      3,
+      { num: 3, user: { longName: "Node C" } },
+    ],
+  ]);
+
+  beforeEach(() => {
+    vi.resetAllMocks();
+    (useDevice as Mock).mockReturnValue({
+      nodes: mockNodes,
+    });
+  });
+
+  it("renders the route to destination with SNR values", () => {
+    render(
+      <TraceRoute
+        from={{ user: { longName: "Source Node" } } as any}
+        to={{ user: { longName: "Destination Node" } } as any}
+        route={[1, 2]}
+        snrTowards={[10, 20, 30]}
+      />
+    );
+
+    expect(screen.getByText("Route to destination:")).toBeInTheDocument();
+    expect(screen.getByText("Destination Node")).toBeInTheDocument();
+
+    expect(screen.getByText("Node A")).toBeInTheDocument();
+    expect(screen.getByText("Node B")).toBeInTheDocument();
+
+    expect(screen.getAllByText(/↓/)).toHaveLength(3); // startNode + 2 hops
+    expect(screen.getByText("↓ 10dB")).toBeInTheDocument();
+    expect(screen.getByText("↓ 20dB")).toBeInTheDocument();
+    expect(screen.getByText("↓ 30dB")).toBeInTheDocument();
+    expect(screen.getByText("Source Node")).toBeInTheDocument();
+  });
+
+  it("renders the route back when provided", () => {
+    render(
+      <TraceRoute
+        from={{ user: { longName: "Source Node" } } as any}
+        to={{ user: { longName: "Destination Node" } } as any}
+        route={[1]}
+        snrTowards={[15, 25]}
+        routeBack={[3]}
+        snrBack={[35, 45]}
+      />
+    );
+
+    expect(screen.getByText("Route back:")).toBeInTheDocument();
+    expect(screen.getByText("Node C")).toBeInTheDocument();
+    expect(screen.getByText("↓ 35dB")).toBeInTheDocument();
+    expect(screen.getByText("↓ 45dB")).toBeInTheDocument();
+  });
+
+  it("renders '??' for missing SNR values", () => {
+    render(
+      <TraceRoute
+        from={{ user: { longName: "Source" } } as any}
+        to={{ user: { longName: "Dest" } } as any}
+        route={[1]}
+      />
+    );
+
+    expect(screen.getAllByText("↓ ??dB").length).toBeGreaterThan(0);
+  });
+
+  it("renders hop hex if node is not found", () => {
+    render(
+      <TraceRoute
+        from={{ user: { longName: "Source" } } as any}
+        to={{ user: { longName: "Dest" } } as any}
+        route={[99]}
+      />
+    );
+
+    expect(screen.getByText(/^!63$/)).toBeInTheDocument(); // 99 in hex 
+  });
+});

src/components/PageComponents/Messages/TraceRoute.tsx

@@ -11,6 +11,33 @@ export interface TraceRouteProps {
   snrBack?: Array<number>;
 }
 
+interface RoutePathProps {
+  title: string;
+  startNode?: Protobuf.Mesh.NodeInfo;
+  endNode?: Protobuf.Mesh.NodeInfo;
+  path: number[];
+  snr?: number[];
+}
+
+const RoutePath = ({ title, startNode, endNode, path, snr }: RoutePathProps) => {
+  const { nodes } = useDevice();
+
+  return (
+    <span className="ml-4 border-l-2 border-l-background-primary pl-2 text-slate-900 dark:text-slate-900">
+      <p className="font-semibold">{title}</p>
+      <p>{startNode?.user?.longName}</p>
+      <p>↓ {snr?.[0] ?? "??"}dB</p>
+      {path.map((hop, i) => (
+        <span key={nodes.get(hop)?.num ?? hop}>
+          <p>{nodes.get(hop)?.user?.longName ?? `!${numberToHexUnpadded(hop)}`}</p>
+          <p>↓ {snr?.[i + 1] ?? "??"}dB</p>
+        </span>
+      ))}
+      <p>{endNode?.user?.longName}</p>
+    </span>
+  );
+};
+
 export const TraceRoute = ({
   from,
   to,
@@ -19,43 +46,24 @@ export const TraceRoute = ({
   snrTowards,
   snrBack,
 }: TraceRouteProps) => {
-  const { nodes } = useDevice();
-
   return (
     <div className="ml-5 flex">
-      <span className="ml-4 border-l-2 border-l-background-primary pl-2 text-text-primary">
-        <p className="font-semibold">Route to destination:</p>
-        <p>{to?.user?.longName}</p>
-        <p>↓ {snrTowards?.[0] ? snrTowards[0] : "??"}dB</p>
-        {route.map((hop, i) => (
-          <span key={nodes.get(hop)?.num}>
-            <p>
-              {nodes.get(hop)?.user?.longName ?? `!${numberToHexUnpadded(hop)}`}
-            </p>
-            <p>↓ {snrTowards?.[i + 1] ? snrTowards[i + 1] : "??"}dB</p>
-          </span>
-        ))}
-        {from?.user?.longName}
-      </span>
-      {routeBack
-        ? (
-          <span className="ml-4 border-l-2 border-l-background-primary pl-2 text-text-primary">
-            <p className="font-semibold">Route back:</p>
-            <p>{from?.user?.longName}</p>
-            <p>↓ {snrBack?.[0] ? snrBack[0] : "??"}dB</p>
-            {routeBack.map((hop, i) => (
-              <span key={nodes.get(hop)?.num}>
-                <p>
-                  {nodes.get(hop)?.user?.longName ??
-                    `!${numberToHexUnpadded(hop)}`}
-                </p>
-                <p>↓ {snrBack?.[i + 1] ? snrBack[i + 1] : "??"}dB</p>
-              </span>
-            ))}
-            {to?.user?.longName}
-          </span>
-        )
-        : null}
+      <RoutePath
+        title="Route to destination:"
+        startNode={to}
+        endNode={from}
+        path={route}
+        snr={snrTowards}
+      />
+      {routeBack && (
+        <RoutePath
+          title="Route back:"
+          startNode={from}
+          endNode={to}
+          path={routeBack}
+          snr={snrBack}
+        />
+      )}
     </div>
   );
 };