From 163642fbf9695cb8a1a1b25a0459c74e3c7b25c8 Mon Sep 17 00:00:00 2001
From: philon- <philon-@users.noreply.github.com>
Date: Wed, 17 Sep 2025 23:15:38 +0200
Subject: [PATCH] Review fixes

Co-Authored-By: Dan Ditomaso <dan.ditomaso@gmail.com>
---
 .../web/public/i18n/locales/en/common.json    |   3 +-
 .../PageComponents/Map/Layers/NodesLayer.tsx  |   4 +-
 .../PageComponents/Map/Layers/SNRLayer.tsx    |  42 +--
 .../Map/Layers/WaypointLayer.tsx              |   4 +-
 .../Map/Popups/WaypointDetail.tsx             | 258 ++++++++++--------
 5 files changed, 169 insertions(+), 142 deletions(-)

diff --git a/packages/web/public/i18n/locales/en/common.json b/packages/web/public/i18n/locales/en/common.json
index 9c1c0f71..6c25fc9f 100644
--- a/packages/web/public/i18n/locales/en/common.json
+++ b/packages/web/public/i18n/locales/en/common.json
@@ -66,7 +66,8 @@
     "year": { "one": "Year", "plural": "Years" },
     "snr": "SNR",
     "volt": { "one": "Volt", "plural": "Volts", "suffix": "V" },
-    "record": { "one": "Records", "plural": "Records" }
+    "record": { "one": "Records", "plural": "Records" },
+    "degree": { "one": "Degree", "plural": "Degrees", "suffix": "°" }
   },
   "security": {
     "0bit": "Empty",
diff --git a/packages/web/src/components/PageComponents/Map/Layers/NodesLayer.tsx b/packages/web/src/components/PageComponents/Map/Layers/NodesLayer.tsx
index 1a14f4ad..d4f22bc5 100644
--- a/packages/web/src/components/PageComponents/Map/Layers/NodesLayer.tsx
+++ b/packages/web/src/components/PageComponents/Map/Layers/NodesLayer.tsx
@@ -83,7 +83,7 @@ export const NodesLayer = ({
       : undefined;
 
     // Always render all node markers in the cluster
-    nodes.forEach((node, i) => {
+    for (const [i, node] of nodes.entries()) {
       const isHead = i === 0;
 
       rendered.push(
@@ -109,7 +109,7 @@ export const NodesLayer = ({
           }}
         />,
       );
-    });
+    }
 
     if (nodes.length > 1) {
       rendered.push(
diff --git a/packages/web/src/components/PageComponents/Map/Layers/SNRLayer.tsx b/packages/web/src/components/PageComponents/Map/Layers/SNRLayer.tsx
index a4a83461..febb3a3c 100644
--- a/packages/web/src/components/PageComponents/Map/Layers/SNRLayer.tsx
+++ b/packages/web/src/components/PageComponents/Map/Layers/SNRLayer.tsx
@@ -165,6 +165,21 @@ function makeFeature(
   };
 }
 
+function pushIfFeature(
+  a: number,
+  b: number,
+  aPos: LngLat,
+  bPos: LngLat,
+  snr: number,
+  curved: boolean,
+  features: Feature[],
+) {
+  const feat = makeFeature(a, b, aPos, bPos, snr, curved);
+  if (feat) {
+    features.push(feat);
+  }
+}
+
 function generateNeighborLines(
   neighborInfos: NeighborInfos[],
 ): FeatureCollection {
@@ -215,26 +230,15 @@ function generateNeighborLines(
 
     if (pair.ab && pair.ba) {
       // both directions → two arcs
-      const feat1 = makeFeature(pair.a, pair.b, aPos, bPos, pair.ab, true);
-      const feat2 = makeFeature(pair.b, pair.a, bPos, aPos, pair.ba, true);
-
-      if (feat1) {
-        features.push(feat1);
-      }
-      if (feat2) {
-        features.push(feat2);
-      }
-    } else if (pair.ab) {
-      // only a->b, straight
-      const feat = makeFeature(pair.a, pair.b, aPos, bPos, pair.ab, false);
-      if (feat) {
-        features.push(feat);
+      pushIfFeature(pair.a, pair.b, aPos, bPos, pair.ab, true, features);
+      pushIfFeature(pair.b, pair.a, bPos, aPos, pair.ba, true, features);
+    } else {
+      // only one direction → straight
+      if (pair.ab) {
+        pushIfFeature(pair.a, pair.b, aPos, bPos, pair.ab, false, features);
       }
-    } else if (pair.ba) {
-      // only b->a, straight
-      const feat = makeFeature(pair.b, pair.a, bPos, aPos, pair.ba, false);
-      if (feat) {
-        features.push(feat);
+      if (pair.ba) {
+        pushIfFeature(pair.b, pair.a, bPos, aPos, pair.ba, false, features);
       }
     }
   }
diff --git a/packages/web/src/components/PageComponents/Map/Layers/WaypointLayer.tsx b/packages/web/src/components/PageComponents/Map/Layers/WaypointLayer.tsx
index 2bfbe9a1..9c747987 100644
--- a/packages/web/src/components/PageComponents/Map/Layers/WaypointLayer.tsx
+++ b/packages/web/src/components/PageComponents/Map/Layers/WaypointLayer.tsx
@@ -49,7 +49,7 @@ export const WaypointLayer = ({
     return rendered;
   }
 
-  waypoints.forEach((waypoint) => {
+  for (const waypoint of waypoints) {
     const [lng, lat] = toLngLat({
       latitudeI: waypoint.latitudeI,
       longitudeI: waypoint.longitudeI,
@@ -66,7 +66,7 @@ export const WaypointLayer = ({
         onClick={(_, e) => onMarkerClick(waypoint, e)}
       />,
     );
-  });
+  }
 
   if (popupState?.type === "waypoint") {
     const [lng, lat] = toLngLat({
diff --git a/packages/web/src/components/PageComponents/Map/Popups/WaypointDetail.tsx b/packages/web/src/components/PageComponents/Map/Popups/WaypointDetail.tsx
index 215b7340..68a1479e 100644
--- a/packages/web/src/components/PageComponents/Map/Popups/WaypointDetail.tsx
+++ b/packages/web/src/components/PageComponents/Map/Popups/WaypointDetail.tsx
@@ -13,13 +13,11 @@ import {
   ClockFadingIcon,
   ClockPlusIcon,
   CompassIcon,
-  //Edit3Icon,
   MapPinnedIcon,
   MoveHorizontalIcon,
   NavigationIcon,
   RotateCwIcon,
   UserLockIcon,
-  UserPenIcon,
 } from "lucide-react";
 import { useTranslation } from "react-i18next";
 
@@ -29,24 +27,7 @@ interface WaypointDetailProps {
   onEdit: () => void;
 }
 
-const RowElement: React.FC<{
-  label: string;
-  value: React.ReactNode | string | number | undefined;
-  icon?: React.ReactNode;
-}> = ({ label, value, icon }) => (
-  <div className="flex justify-between">
-    <span className="inline-flex items-center gap-2 text-slate-500 dark:text-slate-500">
-      {icon} {label}
-    </span>
-    <span className="inline-flex items-center gap-1">{value}</span>
-  </div>
-);
-
-export const WaypointDetail = ({
-  waypoint,
-  myNode,
-  //onEdit,
-}: WaypointDetailProps) => {
+export const WaypointDetail = ({ waypoint, myNode }: WaypointDetailProps) => {
   const { t } = useTranslation("map");
   const { getNode } = useNodeDB();
 
@@ -64,106 +45,147 @@ export const WaypointDetail = ({
     : undefined;
 
   return (
-    <div className="flex flex-col gap-2 px-1 text-sm dark:text-slate-900">
-      <div className="flex items-center my-1 justify-between">
-        <div className="flex items-center gap-2 font-semibold text-slate-900 ">
-          {String.fromCodePoint(waypoint.icon) ?? "📍"}
+    <article
+      aria-labelledby={`wp-${waypoint.id}-title`}
+      className="flex flex-col gap-2 px-1 text-sm dark:text-slate-900"
+    >
+      <header className="flex items-center my-1 justify-between">
+        <h3
+          id={`wp-${waypoint.id}-title`}
+          className="flex items-center gap-2 font-semibold text-slate-900"
+        >
+          <span aria-hidden>{String.fromCodePoint(waypoint.icon) ?? "📍"}</span>
           <span>{waypoint.name}</span>
-        </div>
-      </div>
+        </h3>
+      </header>
+
       {waypoint.description && (
-        <div>
-          <span className="inline-flex items-center gap-1">
-            {waypoint.description}
-          </span>
-        </div>
-      )}
-      <Separator className="dark:bg-slate-200" />
-      <div className="flex justify-between">
-        <span className="inline-flex items-start gap-2 text-slate-500 dark:text-slate-500">
-          <MapPinnedIcon size="20" />
-          <span>
-            {t("waypointDetail.longitude")} {t("waypointDetail.latitude")}
-          </span>
-        </span>
-        <span className="text-right">
-          {waypointLngLat[0]} {waypointLngLat[1]}
-        </span>
-      </div>
-      <RowElement
-        label={t("waypointDetail.createdDate")}
-        value={<TimeAgo timestamp={waypoint.metadata.created} />}
-        icon={<ClockPlusIcon size="14" />}
-      />
-      {waypoint.metadata.updated && (
-        <RowElement
-          label={t("waypointDetail.updated")}
-          value={<TimeAgo timestamp={waypoint.metadata.updated} />}
-          icon={<RotateCwIcon size="14" />}
-        />
-      )}
-      {waypoint.expire !== 0 && (
-        <RowElement
-          label={t("waypointDetail.expires")}
-          value={<TimeAgo timestamp={waypoint.expire * 1000} />}
-          icon={<ClockFadingIcon size="14" />}
-        />
+        <p className="inline-flex items-center gap-1">{waypoint.description}</p>
       )}
-      {distance && (
-        <RowElement
-          label={t("waypointDetail.distance")}
-          value={`${Math.round(distance)} ${distance === 1 ? t("unit.meter.one") : t("unit.meter.plural")}`}
-          icon={<MoveHorizontalIcon size="14" />}
-        />
-      )}
-      {bearing && (
-        <RowElement
-          label={t("waypointDetail.bearing")}
-          value={
-            <>
-              <NavigationIcon
-                size="16"
-                aria-hidden
-                className="shrink-0 origin-center transition-transform"
-                style={{ transform: `rotate(${bearing - 45}deg)` }}
-              />
-              {Math.round(bearing)}°
-            </>
-          }
-          icon={<CompassIcon size="14" />}
-        />
-      )}
-      <RowElement
-        label={t("waypointDetail.createdBy")}
-        value={
-          getNode(waypoint.metadata.from)?.user?.longName ??
-          t("unknown.longName")
-        }
-        icon={<UserPenIcon size="14" />}
-      />
-      {
-        waypoint.lockedTo ? (
-          <RowElement
-            label={t("waypointDetail.lockedTo")}
-            value={
-              getNode(waypoint.lockedTo)?.user?.longName ??
-              t("unknown.longName")
-            }
-            icon={<UserLockIcon size="14" />}
-          />
-        ) : null /*(
-        <div className="flex justify-end  ">
-          <button
-            type="button"
-            onClick={onEdit}
-            className="inline-flex items-center gap-1 rounded-md border border-slate-300 px-2 py-1 text-xs font-medium text-slate-700 hover:bg-slate-100 dark:border-slate-600 dark:text-slate-200 dark:hover:bg-slate-700"
-          >
-            <Edit3Icon className="h-4 w-4" />
-            {t("waypointDetail.edit")}
-          </button>
-        </div>
-      )*/
-      }
-    </div>
+
+      <Separator className="dark:bg-slate-200" role="separator" />
+
+      <section aria-label={t("waypointDetail.details")}>
+        <dl className="space-y-1.5">
+          {/* Coordinates */}
+          <div className="flex flex-wrap items-start gap-x-3">
+            <dt className="inline-flex items-top gap-2 text-slate-500 min-w-0">
+              <MapPinnedIcon size={14} aria-hidden className="mt-1" />
+              <span className="truncate">
+                {t("waypointDetail.longitude")}
+                <br />
+                {t("waypointDetail.latitude")}
+              </span>
+            </dt>
+            <dd className="ms-auto text-right">
+              <data value={waypointLngLat[0]}>{waypointLngLat[0]}</data>
+              <br />
+              <data value={waypointLngLat[1]}>{waypointLngLat[1]}</data>
+            </dd>
+          </div>
+
+          {/* Created */}
+          <div className="flex flex-wrap items-start gap-x-3">
+            <dt className="inline-flex items-center gap-2 text-slate-500 min-w-0">
+              <ClockPlusIcon size={14} aria-hidden />
+              <span className="truncate">
+                {t("waypointDetail.createdDate")}
+              </span>
+            </dt>
+            <dd className="ms-auto text-right">
+              <time
+                dateTime={new Date(waypoint.metadata.created).toISOString()}
+              >
+                <TimeAgo timestamp={waypoint.metadata.created} />
+              </time>
+            </dd>
+          </div>
+
+          {/* Updated */}
+          {waypoint.metadata.updated && (
+            <div className="flex flex-wrap items-start gap-x-3">
+              <dt className="inline-flex items-center gap-2 text-slate-500 min-w-0">
+                <RotateCwIcon size={14} aria-hidden />
+                <span className="truncate">{t("waypointDetail.updated")}</span>
+              </dt>
+              <dd className="ms-auto text-right">
+                <time
+                  dateTime={new Date(waypoint.metadata.updated).toISOString()}
+                >
+                  <TimeAgo timestamp={waypoint.metadata.updated} />
+                </time>
+              </dd>
+            </div>
+          )}
+
+          {/* Expires */}
+          {waypoint.expire !== 0 && (
+            <div className="flex flex-wrap items-start gap-x-3">
+              <dt className="inline-flex items-center gap-2 text-slate-500 min-w-0">
+                <ClockFadingIcon size={14} aria-hidden />
+                <span className="truncate">{t("waypointDetail.expires")}</span>
+              </dt>
+              <dd className="ms-auto text-right">
+                <time dateTime={new Date(waypoint.expire * 1000).toISOString()}>
+                  <TimeAgo timestamp={waypoint.expire * 1000} />
+                </time>
+              </dd>
+            </div>
+          )}
+
+          {/* Distance */}
+          {distance != null && (
+            <div className="flex flex-wrap items-start gap-x-3">
+              <dt className="inline-flex items-center gap-2 text-slate-500 min-w-0">
+                <MoveHorizontalIcon size={14} aria-hidden />
+                <span className="truncate">{t("waypointDetail.distance")}</span>
+              </dt>
+              <dd className="ms-auto text-right">
+                <data value={Math.round(distance)}>
+                  {Math.round(distance)}{" "}
+                  {distance === 1
+                    ? t("unit.meter.one")
+                    : t("unit.meter.plural")}
+                </data>
+              </dd>
+            </div>
+          )}
+
+          {/* Bearing */}
+          {bearing != null && (
+            <div className="flex flex-wrap items-start gap-x-3">
+              <dt className="inline-flex items-center gap-2 text-slate-500 min-w-0">
+                <CompassIcon size={14} aria-hidden />
+                <span className="truncate">{t("waypointDetail.bearing")}</span>
+              </dt>
+              <dd className="ms-auto text-right inline-flex items-center ">
+                <NavigationIcon
+                  size={16}
+                  aria-hidden
+                  className="shrink-0 origin-center transition-transform mr-2"
+                  style={{ transform: `rotate(${bearing - 45}deg)` }}
+                />
+                <data value={Math.round(bearing)}>{Math.round(bearing)}</data>
+                <span aria-hidden>{t("unit.degree.suffix")}</span>
+              </dd>
+            </div>
+          )}
+
+          {/* Locked To */}
+          {waypoint.lockedTo && (
+            <div className="flex flex-wrap items-start gap-x-3">
+              <dt className="inline-flex items-center gap-2 text-slate-500 min-w-0">
+                <UserLockIcon size={14} aria-hidden />
+                <span className="truncate">{t("waypointDetail.lockedTo")}</span>
+              </dt>
+              <dd className="ms-auto text-right">
+                {getNode(waypoint.lockedTo)?.user?.longName ??
+                  t("unknown.longName")}
+              </dd>
+            </div>
+          )}
+        </dl>
+      </section>
+    </article>
   );
 };