fix(ui): column widths ignore colspan rows and empty first renders (#258)

.github/workflows/deploy.yml

@@ -442,6 +442,7 @@ jobs:
           BASE_URL=http://localhost:13581 node test-logo-pulse-1173-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1122-packets-filter-ux-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1122-details-row-clamp-e2e.js 2>&1 | tee -a e2e-output.txt
+          BASE_URL=http://localhost:13581 node test-issue-258-column-widths-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1128-packets-layout-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-189-group-caret-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-254-affinity-toggle-mobile-aria-e2e.js 2>&1 | tee -a e2e-output.txt

public/app.js

@@ -2245,72 +2245,80 @@ function initTabBar(container, onChange) {
   });
 }
 
+// #258: columns are measured from at most COL_MEASURE_MAX_ROWS body rows. A body
+// with fewer than COL_MEASURE_MIN_ROWS usable rows only gives provisional widths.
+const COL_MEASURE_MAX_ROWS = 30;
+const COL_MEASURE_MIN_ROWS = 5;
+
 /**
- * Make table columns resizable with drag handles. Widths saved to localStorage.
- * Call after table is in DOM. Re-call safe (idempotent per table).
- * @param {string} tableSelector - CSS selector for the table
- * @param {string} storageKey - localStorage key for persisted widths
+ * #258: the body rows a column measurement can use, i.e. rows with exactly one
+ * cell per header cell. A row that spans columns (a virtual-scroll spacer, "No
+ * packets found", a group-detail row) belongs to no single column; measured by
+ * index, it used to inflate column 0.
  */
-function makeColumnsResizable(tableSelector, storageKey) {
-  const table = document.querySelector(tableSelector);
-  if (!table) return;
-  const thead = table.querySelector('thead');
-  if (!thead) return;
-  const ths = Array.from(thead.querySelectorAll('tr:first-child th'));
-  if (ths.length < 2) return;
-
-  if (table.dataset.resizable) return;
-  table.dataset.resizable = '1';
-  table.style.tableLayout = 'fixed';
-
-  const containerW = table.parentElement.clientWidth;
-  const saved = localStorage.getItem(storageKey);
-  let widths;
-
-  if (saved) {
-    try { widths = JSON.parse(saved); } catch { widths = null; }
-    // Validate: must be array of correct length with values summing to ~100 (percentages)
-    if (widths && Array.isArray(widths) && widths.length === ths.length) {
-      const sum = widths.reduce((s, w) => s + w, 0);
-      if (sum > 90 && sum < 110) {
-        // Saved percentages — apply directly
-        table.style.tableLayout = 'fixed';
-        table.style.width = '100%';
-        ths.forEach((th, i) => { th.style.width = widths[i] + '%'; });
-        // Skip measurement, jump to adding handles
-        addResizeHandles();
-        return;
-      }
+function columnMeasureRows(tbody, colCount, limit) {
+  const out = [];
+  if (!tbody) return out;
+  for (const row of tbody.rows) {
+    const cells = row.children;
+    if (cells.length !== colCount) continue;
+    let spans = false;
+    for (let i = 0; i < cells.length; i++) {
+      if (cells[i].colSpan > 1) { spans = true; break; }
     }
-    widths = null; // Force remeasure
+    if (spans) continue;
+    out.push(row);
+    if (out.length >= limit) break;
   }
+  return out;
+}
 
-  if (!widths) {
-    // Measure actual max content width per column by scanning visible rows
-    const tbody = table.querySelector('tbody');
-    const rows = tbody ? Array.from(tbody.querySelectorAll('tr')).slice(0, 30) : [];
+/** Saved column widths (percentages) for storageKey, or null if none are valid. */
+function readSavedColumnWidths(storageKey, colCount) {
+  const saved = localStorage.getItem(storageKey);
+  if (!saved) return null;
+  let widths;
+  try { widths = JSON.parse(saved); } catch { return null; }
+  if (!Array.isArray(widths) || widths.length !== colCount) return null;
+  const sum = widths.reduce((s, w) => s + w, 0);
+  return sum > 90 && sum < 110 ? widths : null;
+}
 
-    // Temporarily set auto layout to measure
+// Max content width per column (header + rows), measured in auto layout without
+// wrapping. TableResponsive's column hiding is lifted while measuring: the first
+// measure runs before TableResponsive.register(), and a re-measure (#258) runs
+// before it has marked the newly rendered cells, so without that the header and
+// the rows would disagree on which columns exist.
+function measureColumnWidths(table, ths, rows) {
+  const tr = window.TableResponsive;
+  const measure = () => {
     table.style.tableLayout = 'auto';
     table.style.width = 'auto';
-    // Remove nowrap temporarily so we get true content width
+    // The resize handles stick out of their th (right: -4px), which would add
+    // to its scrollWidth; the first measure runs before they exist.
+    const handles = table.querySelectorAll('.col-resize-handle');
+    handles.forEach(h => { h.style.display = 'none'; });
+    // Remove wrapping temporarily so we get true content width
     const cells = table.querySelectorAll('td, th');
     cells.forEach(c => { c.dataset.origWs = c.style.whiteSpace || ''; c.style.whiteSpace = 'nowrap'; });
-
-    // Measure each column's max content width across header + rows
-    widths = ths.map((th, i) => {
+    const widths = ths.map((th, i) => {
       let maxW = th.scrollWidth;
       rows.forEach(row => {
         const td = row.children[i];
         if (td) maxW = Math.max(maxW, td.scrollWidth);
       });
       return maxW + 4; // small padding buffer
     });
-
     cells.forEach(c => { c.style.whiteSpace = c.dataset.origWs || ''; delete c.dataset.origWs; });
-  }
+    handles.forEach(h => { h.style.display = ''; });
+    return widths;
+  };
+  return tr && typeof tr.unhidden === 'function' ? tr.unhidden(table, measure) : measure();
+}
 
-  // Now fit to container: if total > container, squish widest first
+// Fit measured widths to the container: if the total is too wide, squish the
+// widest columns first; if there is room left, give it to the 2 widest.
+function fitColumnWidths(widths, containerW) {
   const totalNeeded = widths.reduce((s, w) => s + w, 0);
   const finalWidths = [...widths];
 
@@ -2348,13 +2356,72 @@ function makeColumnsResizable(tableSelector, storageKey) {
     const topTotal = topN.reduce((s, x) => s + x.w, 0);
     topN.forEach(x => { finalWidths[x.i] += Math.round(surplus * (x.w / topTotal)); });
   }
+  return finalWidths;
+}
 
+// Measure the columns from `rows` and set them as percentages of the table.
+function applyMeasuredColumnWidths(table, ths, rows) {
+  const containerW = table.parentElement.clientWidth;
+  const finalWidths = fitColumnWidths(measureColumnWidths(table, ths, rows), containerW);
   table.style.width = '100%';
   const totalFinal = finalWidths.reduce((s, w) => s + w, 0);
   ths.forEach((th, i) => { th.style.width = (finalWidths[i] / totalFinal * 100) + '%'; });
+}
 
+/**
+ * Make table columns resizable with drag handles. Widths saved to localStorage.
+ * Call after table is in DOM. Re-call safe (idempotent per table).
+ * Without saved widths the columns are measured from the header and the first
+ * body rows; if the body has too few rows for that, once more when it fills (#258).
+ * @param {string} tableSelector - CSS selector for the table
+ * @param {string} storageKey - localStorage key for persisted widths
+ */
+function makeColumnsResizable(tableSelector, storageKey) {
+  const table = document.querySelector(tableSelector);
+  if (!table) return;
+  const thead = table.querySelector('thead');
+  if (!thead) return;
+  const ths = Array.from(thead.querySelectorAll('tr:first-child th'));
+  if (ths.length < 2) return;
+
+  if (table.dataset.resizable) return;
+  table.dataset.resizable = '1';
+  table.style.tableLayout = 'fixed';
+
+  const saved = readSavedColumnWidths(storageKey, ths.length);
+  if (saved) {
+    // Saved percentages — apply directly, no measurement
+    table.style.width = '100%';
+    ths.forEach((th, i) => { th.style.width = saved[i] + '%'; });
+    addResizeHandles();
+    return;
+  }
+
+  const tbody = table.querySelector('tbody');
+  const authoredWidths = ths.map(th => th.style.width);
+  const rows = columnMeasureRows(tbody, ths.length, COL_MEASURE_MAX_ROWS);
+  applyMeasuredColumnWidths(table, ths, rows);
   addResizeHandles();
 
+  // #258: an (almost) empty first render, e.g. a quiet packets time window,
+  // only gives provisional widths. Measure once more when real rows arrive,
+  // unless the user has saved widths by then. Until then each body render costs
+  // a cheap guard; after the re-measure the observer is gone.
+  if (tbody && rows.length < COL_MEASURE_MIN_ROWS && typeof MutationObserver === 'function') {
+    const filled = new MutationObserver(() => {
+      if (!table.isConnected || readSavedColumnWidths(storageKey, ths.length)) { filled.disconnect(); return; }
+      if (tbody.rows.length < COL_MEASURE_MIN_ROWS) return;
+      const rowsNow = columnMeasureRows(tbody, ths.length, COL_MEASURE_MAX_ROWS);
+      if (rowsNow.length < COL_MEASURE_MIN_ROWS) return;
+      filled.disconnect();
+      // Measure as the first time: the widths the page itself gave the header
+      // cells, not the provisional ones.
+      ths.forEach((th, i) => { th.style.width = authoredWidths[i]; });
+      applyMeasuredColumnWidths(table, ths, rowsNow);
+    });
+    filled.observe(tbody, { childList: true });
+  }
+
   function addResizeHandles() {
   // Add resize handles
   ths.forEach((th, i) => {

public/packets.js

@@ -32,6 +32,24 @@
     if (pill) pill.remove();
   }
 
+  // #258: run fn with this module's column hiding lifted -- no col-hidden on
+  // any cell and no pill -- and put it back afterwards. makeColumnsResizable()
+  // (app.js) measures inside it, so a re-measure sees the columns as the first
+  // measure did, before register(); other CSS that hides a column still applies.
+  function unhidden(table, fn) {
+    const hidden = Array.from(table.querySelectorAll('.' + HIDDEN_CLASS));
+    const pills = Array.from(table.querySelectorAll('.' + PILL_CLASS));
+    const pillDisplay = pills.map(p => p.style.display);
+    hidden.forEach(el => el.classList.remove(HIDDEN_CLASS));
+    pills.forEach(p => { p.style.display = 'none'; });
+    try {
+      return fn();
+    } finally {
+      hidden.forEach(el => el.classList.add(HIDDEN_CLASS));
+      pills.forEach((p, i) => { p.style.display = pillDisplay[i]; });
+    }
+  }
+
   function colIndexCells(table, idx) {
     // Return the <td> at column index `idx` for every body row.
     const out = [];
@@ -221,7 +239,7 @@
     }, 120);
   });
 
-  window.TableResponsive = { apply, register, sweep: sweepDetached };
+  window.TableResponsive = { apply, register, sweep: sweepDetached, unhidden };
 })();
 
 /* === #1056 AC#4: SlideOver — narrow-viewport row-detail overlay ============

test-all.sh

@@ -58,6 +58,7 @@ run test-important-links-byte-filter.js
 run test-url-state.js
 run test-node-adverts.js
 run test-issue-254-affinity-debug-toggle.js
+run test-issue-258-column-widths.js
 run test-perf-go-runtime.js
 run test-channel-psk-ux.js
 run test-channel-sidebar-layout.js

test-issue-1122-details-row-clamp-e2e.js

@@ -43,19 +43,18 @@ const VIEWPORTS = [
 ];
 
 // #244: open with the widest selectable time window, not the default 15 min.
-// makeColumnsResizable() (app.js) sizes the columns ONCE, from the rows of the
-// first render, and later time-window changes keep those widths. With the
-// default window those rows depend on the fixture age: in CI the window holds
-// only a few packets ~13 min after freshen-fixture.sh, and none after 15 min;
-// the expand column then gets ~43% of the table and Details ~5% (64px at
-// 1200px), and advert names wrap onto the clamped-away 2nd line. The newest
-// rows of a 24 h window (3 h at <=1024px, where longer windows are disabled)
-// are the same ones for the whole e2e job (150 min timeout), so the measured
-// layout no longer depends on the fixture age.
+// makeColumnsResizable() (app.js) sizes the columns from the rows of the first
+// render; since #258 a (nearly) empty first render is measured again once real
+// rows arrive (test-issue-258-column-widths-e2e.js covers that). With the
+// default window the first rows depend on the fixture age (a few packets ~13
+// min after freshen-fixture.sh, none after 15 min). The newest rows of a 24 h
+// window (3 h at <=1024px, where longer windows are disabled) are the same ones
+// for the whole e2e job (150 min timeout), so this test measures one layout
+// regardless of the fixture age.
 const pinnedWindowMin = (vp) => (vp.w > 1024 ? 1440 : 180);
-// A fixture advert with a long name ("KN6PLV-BrkOxfLA-Yebes", 21 chars) that
-// is wider than its Details clip at every viewport here; it renders near the
-// top of the list.
+// A fixture advert with a long name ("KN6PLV-BrkOxfLA-Yebes", 21 chars); it
+// renders near the top of the list. It is wider than its Details clip on
+// mobile; at 900/1200px Details is wide enough to show it (#258).
 const PINNED_ADVERT_ROW = 'e8b09a35ac87fa5c';
 // Enough of a name to read and click: about two characters.
 const MIN_VISIBLE_LINK_PX = 12;
@@ -194,13 +193,22 @@ function measureRows() {
         return checked;
       });
       assert(res.length > 0, 'no on-screen advert link in Details to check');
-      // The pinned long-name row must be among them and be cut by the clip,
-      // so the test cannot pass without exercising a long advert name.
+      // The pinned long-name row must be among them, so the test cannot pass
+      // without exercising a long advert name. On mobile its clip is narrow and
+      // the name must be cut by it (the clamped-link case). Wider, Details is
+      // sized from the real rows (#258), so the name may fit; it must then show
+      // at least half of itself, not just the icon.
       const pinned = res.find(r => r.row === PINNED_ADVERT_ROW);
       assert(pinned, `pinned advert row ${PINNED_ADVERT_ROW} is not on screen; checked: ` +
         JSON.stringify(res.map(r => [r.row, r.text])));
-      assert(pinned.truncated,
-        'pinned advert name fits its Details clip -- long names are not exercised: ' + JSON.stringify(pinned));
+      if (vp.w <= 640) {
+        assert(pinned.truncated,
+          'pinned advert name fits its mobile Details clip -- the clamped long name is not exercised: ' + JSON.stringify(pinned));
+      } else {
+        const nameW = pinned.frags.reduce((s, r) => s + (r.right - r.left), 0);
+        assert(pinned.visibleW >= nameW / 2,
+          `pinned long advert name shows ${pinned.visibleW}px of ${Math.round(nameW)}px in Details: ` + JSON.stringify(pinned));
+      }
       const hidden = res.filter(r => !(r.visibleW >= MIN_VISIBLE_LINK_PX));
       assert(hidden.length === 0, `${hidden.length}/${res.length} advert links show < ${MIN_VISIBLE_LINK_PX}px ` +
         'of their name in the one-line Details clip: ' + JSON.stringify(hidden.slice(0, 3)));