Merge pull request #297 from jkingsman/make-repeater-sortable

Make repeater neighbor pane sortable. Closes #290.

frontend/src/components/repeater/RepeaterNeighborsPane.tsx

@@ -1,4 +1,4 @@
-import { useMemo, lazy, Suspense } from 'react';
+import { useMemo, useState, useCallback, lazy, Suspense } from 'react';
 import { cn } from '@/lib/utils';
 import { RepeaterPane, NotFetched, formatDuration } from './repeaterPaneShared';
 import { isValidLocation, calculateDistance, formatDistance } from '../../utils/pathUtils';
@@ -15,6 +15,49 @@ const NeighborsMiniMap = lazy(() =>
   import('../NeighborsMiniMap').then((m) => ({ default: m.NeighborsMiniMap }))
 );
 
+type SortField = 'name' | 'snr' | 'distance' | 'last_heard';
+type SortDir = 'asc' | 'desc';
+
+// Direction applied when a column is first selected. Name reads naturally A→Z
+// and nearest-first/most-recent-first are the intuitive starting points; SNR
+// leads with the strongest signal to preserve the previous default ordering.
+const DEFAULT_DIR: Record<SortField, SortDir> = {
+  name: 'asc',
+  snr: 'desc',
+  distance: 'asc',
+  last_heard: 'asc',
+};
+
+function SortableHeader({
+  label,
+  field,
+  sortField,
+  sortDir,
+  onSort,
+  className,
+}: {
+  label: string;
+  field: SortField;
+  sortField: SortField;
+  sortDir: SortDir;
+  onSort: (field: SortField) => void;
+  className?: string;
+}) {
+  const active = sortField === field;
+  return (
+    <th
+      className={cn(
+        'pb-1 font-medium cursor-pointer select-none hover:text-foreground transition-colors',
+        className
+      )}
+      onClick={() => onSort(field)}
+      aria-sort={active ? (sortDir === 'asc' ? 'ascending' : 'descending') : 'none'}
+    >
+      {label} {active ? (sortDir === 'asc' ? '▲' : '▼') : ''}
+    </th>
+  );
+}
+
 export function NeighborsPane({
   data,
   state,
@@ -71,19 +114,37 @@ export function NeighborsPane({
           ? 'Waiting for repeater position'
           : 'No repeater position available';
 
-  // Resolve contact data for each neighbor in a single pass — used for
-  // coords (mini-map), distances (table column), and sorted display order.
-  const { neighborsWithCoords, sorted, hasDistances } = useMemo(() => {
+  const [sortField, setSortField] = useState<SortField>('snr');
+  const [sortDir, setSortDir] = useState<SortDir>('desc');
+
+  const handleSort = useCallback(
+    (field: SortField) => {
+      if (sortField === field) {
+        setSortDir((d) => (d === 'asc' ? 'desc' : 'asc'));
+      } else {
+        setSortField(field);
+        setSortDir(DEFAULT_DIR[field]);
+      }
+    },
+    [sortField]
+  );
+
+  // Resolve contact data for each neighbor in a single pass — used for coords
+  // (mini-map) and distances (table column + distance sort). The formatted
+  // string drives display; the raw km drives numeric distance sorting.
+  const { neighborsWithCoords, enriched, hasDistances } = useMemo(() => {
     if (!data) {
       return {
         neighborsWithCoords: [] as Array<NeighborInfo & { lat: number | null; lon: number | null }>,
-        sorted: [] as Array<NeighborInfo & { distance: string | null }>,
+        enriched: [] as Array<
+          NeighborInfo & { distance: string | null; distanceKm: number | null }
+        >,
         hasDistances: false,
       };
     }
 
     const withCoords: Array<NeighborInfo & { lat: number | null; lon: number | null }> = [];
-    const enriched: Array<NeighborInfo & { distance: string | null }> = [];
+    const list: Array<NeighborInfo & { distance: string | null; distanceKm: number | null }> = [];
     let anyDist = false;
 
     for (const n of data.neighbors) {
@@ -92,29 +153,54 @@ export function NeighborsPane({
       const nLon = contact?.lon ?? null;
 
       let dist: string | null = null;
+      let distKm: number | null = null;
       if (hasValidRepeaterGps && isValidLocation(nLat, nLon)) {
-        const distKm = calculateDistance(positionSource.lat, positionSource.lon, nLat, nLon);
-        if (distKm != null) {
-          dist = formatDistance(distKm, distanceUnit);
+        const km = calculateDistance(positionSource.lat, positionSource.lon, nLat, nLon);
+        if (km != null) {
+          distKm = km;
+          dist = formatDistance(km, distanceUnit);
           anyDist = true;
         }
       }
-      enriched.push({ ...n, distance: dist });
+      list.push({ ...n, distance: dist, distanceKm: distKm });
 
       if (isValidLocation(nLat, nLon)) {
         withCoords.push({ ...n, lat: nLat, lon: nLon });
       }
     }
 
-    enriched.sort((a, b) => b.snr - a.snr);
-
     return {
       neighborsWithCoords: withCoords,
-      sorted: enriched,
+      enriched: list,
       hasDistances: anyDist,
     };
   }, [contacts, data, distanceUnit, hasValidRepeaterGps, positionSource.lat, positionSource.lon]);
 
+  const sorted = useMemo(() => {
+    const dir = sortDir === 'asc' ? 1 : -1;
+    return [...enriched].sort((a, b) => {
+      switch (sortField) {
+        case 'name': {
+          const an = (a.name || a.pubkey_prefix).toLowerCase();
+          const bn = (b.name || b.pubkey_prefix).toLowerCase();
+          return an.localeCompare(bn) * dir;
+        }
+        case 'distance': {
+          // Neighbors without a known distance always sort last, regardless of direction.
+          if (a.distanceKm == null && b.distanceKm == null) return 0;
+          if (a.distanceKm == null) return 1;
+          if (b.distanceKm == null) return -1;
+          return (a.distanceKm - b.distanceKm) * dir;
+        }
+        case 'last_heard':
+          return (a.last_heard_seconds - b.last_heard_seconds) * dir;
+        case 'snr':
+        default:
+          return (a.snr - b.snr) * dir;
+      }
+    });
+  }, [enriched, sortField, sortDir]);
+
   return (
     <RepeaterPane
       title="Neighbors"
@@ -135,10 +221,39 @@ export function NeighborsPane({
             <table className="w-full text-sm">
               <thead>
                 <tr className="text-left text-muted-foreground text-xs">
-                  <th className="pb-1 font-medium">Name</th>
-                  <th className="pb-1 font-medium text-right">SNR</th>
-                  {hasDistances && <th className="pb-1 font-medium text-right">Dist</th>}
-                  <th className="pb-1 font-medium text-right">Last Heard</th>
+                  <SortableHeader
+                    label="Name"
+                    field="name"
+                    sortField={sortField}
+                    sortDir={sortDir}
+                    onSort={handleSort}
+                  />
+                  <SortableHeader
+                    label="SNR"
+                    field="snr"
+                    sortField={sortField}
+                    sortDir={sortDir}
+                    onSort={handleSort}
+                    className="text-right"
+                  />
+                  {hasDistances && (
+                    <SortableHeader
+                      label="Dist"
+                      field="distance"
+                      sortField={sortField}
+                      sortDir={sortDir}
+                      onSort={handleSort}
+                      className="text-right"
+                    />
+                  )}
+                  <SortableHeader
+                    label="Last Heard"
+                    field="last_heard"
+                    sortField={sortField}
+                    sortDir={sortDir}
+                    onSort={handleSort}
+                    className="text-right"
+                  />
                 </tr>
               </thead>
               <tbody>

frontend/src/test/repeaterDashboard.test.tsx

@@ -1,5 +1,5 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
-import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { render, screen, fireEvent, waitFor, within } from '@testing-library/react';
 import { RepeaterDashboard } from '../components/RepeaterDashboard';
 import type { UseRepeaterDashboardResult } from '../hooks/useRepeaterDashboard';
 import type { Contact, Conversation } from '../types';
@@ -409,6 +409,49 @@ describe('RepeaterDashboard', () => {
     expect(screen.getByText('Using advert position')).toBeInTheDocument();
   });
 
+  it('sorts the neighbors table when column headers are clicked', () => {
+    mockHook.loggedIn = true;
+    mockHook.paneData.neighbors = {
+      neighbors: [
+        { pubkey_prefix: 'cccccccccccc', name: 'Mike', snr: 5.0, last_heard_seconds: 20 },
+        { pubkey_prefix: 'dddddddddddd', name: 'Zeta', snr: 9.0, last_heard_seconds: 30 },
+        { pubkey_prefix: 'eeeeeeeeeeee', name: 'Alpha', snr: 1.0, last_heard_seconds: 10 },
+      ],
+    };
+    mockHook.paneStates.neighbors = {
+      loading: false,
+      attempt: 1,
+      error: null,
+      fetched_at: Date.now(),
+    };
+
+    render(<RepeaterDashboard {...defaultProps} />);
+
+    // Scope to the Neighbors table; the dashboard renders multiple panes at once.
+    const table = screen.getByRole('columnheader', { name: /last heard/i }).closest('table')!;
+    // The first child of each name cell is the bare name text node (the prefix
+    // lives in a nested span), so this reads exactly the neighbor name.
+    const names = () =>
+      within(table)
+        .getAllByRole('row')
+        .slice(1)
+        .map((r) => r.querySelector('td')?.firstChild?.textContent ?? '');
+    const header = (re: RegExp) => within(table).getByRole('columnheader', { name: re });
+
+    // Default order is SNR descending (preserves the pre-sorting behavior).
+    expect(names()).toEqual(['Zeta', 'Mike', 'Alpha']);
+
+    // Name ascending, then toggle to descending on a second click.
+    fireEvent.click(header(/name/i));
+    expect(names()).toEqual(['Alpha', 'Mike', 'Zeta']);
+    fireEvent.click(header(/name/i));
+    expect(names()).toEqual(['Zeta', 'Mike', 'Alpha']);
+
+    // Last Heard ascending surfaces the most recently heard neighbor first.
+    fireEvent.click(header(/last heard/i));
+    expect(names()).toEqual(['Alpha', 'Mike', 'Zeta']);
+  });
+
   it('shows fetching state with attempt counter', () => {
     mockHook.loggedIn = true;
     mockHook.paneStates.status = { loading: true, attempt: 2, error: null };