mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-09 16:59:43 +02:00
fix(tabs): never size the state label column from hidden headings
#538's by-state header strip lines its labels up in a column measured into --tab-triage-gutter by _sizeTabTriageGutter(). Under 600px mobile.css hides the headings (the phone row keeps its chips, no labels), but the sizer measured them anyway: every part was 0 wide, yet `width + 5 * (parts - 1)` made each labelled heading 5, so the "nothing to size" guard never tripped, the column became 15px and the group key was cached as if measured. A phone turned to landscape (>= 768px) or a foldable opened (Find N5: folded under 600, unfolded 1124 wide) then crossed into the wrapping strip with no tab render behind it (the resize handler only calls updateTabOverflowMode()), the key had not changed, and "WAITING 2" sat in a 15px column on top of the first tab of its row. Only the wrapping strip reads the column, so the sizer now measures nothing and forgets its key while the strip does not wrap, counts only parts that are laid out (a hidden heading sizes and keys nothing), and updateTabOverflowMode() calls it right after deciding the wrap. Phones and tablets therefore never measure (no layout read per render pass), and every flip into the wrapping strip, the 768px breakpoint included, measures afresh. The 600px breakpoint only matters below 768px, where nothing is measured. Tests in test/tab-triage.test.ts pin that hidden headings size and key nothing, that the real updateTabOverflowMode() measures on the unfold with no render, and that wrapping again re-measures with the counts unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -903,7 +903,7 @@ Tests: `test/terminal-touch-tap.test.ts`.
|
|||||||
|
|
||||||
**Tab layouts** (`tabArrangement`, per-device, default `state`; Discussion #426): App Settings → Appearance → Tabs → **Tab Layout**. Four values: `state` (option C), `case` (option A), `ledger` (option B) and `classic`, the strip as before. `<html data-tab-arrangement>` is stamped pre-paint and by `applyTabOrientation()`; the render paths read it through `isTabTriage()`, `isTabClusters()` and `isTabLedger()`. Named groups in the vertical rail (owner tab layouts, `_projectTabGroups()` non-null) win over both groupings, and the grouped tree renders as before.
|
**Tab layouts** (`tabArrangement`, per-device, default `state`; Discussion #426): App Settings → Appearance → Tabs → **Tab Layout**. Four values: `state` (option C), `case` (option A), `ledger` (option B) and `classic`, the strip as before. `<html data-tab-arrangement>` is stamped pre-paint and by `applyTabOrientation()`; the render paths read it through `isTabTriage()`, `isTabClusters()` and `isTabLedger()`. Named groups in the vertical rail (owner tab layouts, `_projectTabGroups()` non-null) win over both groupings, and the grouped tree renders as before.
|
||||||
|
|
||||||
**By state.** The list is split into four groups: **needs you** (a permission or question dialog; a failed session joins it), **waiting** (idle prompt pending), **working**, **idle** (also ended sessions, an agent that exited inside a live pane, and web tabs). Idle is `quiet` (`TAB_TRIAGE_GROUPS`): its heading element still exists, because it anchors the row's `order` band and, as the lead, holds the row's place beside the brand, but it draws no label or count (`.tab-triage-head--quiet`). `tabStateOrder` (`urgent-first` default, `urgent-last`) decides which end they start from; only the group order flips, never the rows inside a group. The desktop header strip draws a row per group with its label left-aligned in a column that `_sizeTabTriageGutter()` measures to the widest label on screen (re-measured only when the label text changes, and once on `document.fonts.ready`), so no row carries a fixed gutter's worth of empty space. The brand leaves the flow and sits over the strip's top-left corner (`.header:has(...)`), so the first row's heading (`.tab-triage-head--lead`, natural width) is pushed past it by `--tab-triage-brand` (a ResizeObserver keeps that width, so no render pass reads layout for it) and every later row starts at the edge, under "Codeman"; the flat rail and the sidebar draw a section per group; the tablet strip (600-767px) keeps its scrolling row with the headings as inline dividers; phones keep their chip row in group order with the headings hidden (mobile.css). Classification is `_mobileOverviewState()` + `_mobileOverviewExit()`, the home screens' own; the fold into four groups and the order bands are pure in `CodemanTabTriage` (constants.js, `computeTabTriageLayout()`).
|
**By state.** The list is split into four groups: **needs you** (a permission or question dialog; a failed session joins it), **waiting** (idle prompt pending), **working**, **idle** (also ended sessions, an agent that exited inside a live pane, and web tabs). Idle is `quiet` (`TAB_TRIAGE_GROUPS`): its heading element still exists, because it anchors the row's `order` band and, as the lead, holds the row's place beside the brand, but it draws no label or count (`.tab-triage-head--quiet`). `tabStateOrder` (`urgent-first` default, `urgent-last`) decides which end they start from; only the group order flips, never the rows inside a group. The desktop header strip draws a row per group with its label left-aligned in a column that `_sizeTabTriageGutter()` measures to the widest label on screen (re-measured only when the label text changes, when the strip starts wrapping, and once on `document.fonts.ready`), so no row carries a fixed gutter's worth of empty space. The brand leaves the flow and sits over the strip's top-left corner (`.header:has(...)`), so the first row's heading (`.tab-triage-head--lead`, natural width) is pushed past it by `--tab-triage-brand` (a ResizeObserver keeps that width, so no render pass reads layout for it) and every later row starts at the edge, under "Codeman"; the flat rail and the sidebar draw a section per group; the tablet strip (600-767px) keeps its scrolling row with the headings as inline dividers; phones keep their chip row in group order with the headings hidden (mobile.css). ⚠️ Only the wrapping strip reads the label column, so `_sizeTabTriageGutter()` measures nothing (and forgets its key) while the strip does not wrap, skips parts that are not laid out (a hidden heading measures 0 per part, and the 5px gap between label and count used to turn that into a 15px column cached as if measured), and `updateTabOverflowMode()` calls it right after deciding to wrap, because a phone turned to landscape or a foldable opened crosses 768px with no tab render behind it (the resize handler only calls `updateTabOverflowMode()`). Classification is `_mobileOverviewState()` + `_mobileOverviewExit()`, the home screens' own; the fold into four groups and the order bands are pure in `CodemanTabTriage` (constants.js, `computeTabTriageLayout()`).
|
||||||
|
|
||||||
⚠️ **By state is the flex `order` property, never a DOM reorder**, exactly like the sorted rail: `#sessionTabs` stays in `sessionOrder`, so the Alt+N badges, drag, the keyboard walk (which sorts by COMPUTED order) and the sidebar filter keep reading the list they always read. Each group owns a band of `TAB_TRIAGE_STRIDE` order values: its heading, its rows, its web tabs, then the row break that ends the header line. The headings and breaks are `aria-hidden` direct children of `#sessionTabs`, reconciled in place by `_syncTabTriageChrome()` after BOTH render paths, because a state change is an incremental pass and can still empty a group or fill a new one. Inside a header row tabs keep TAB order; a sorted rail ranks each section the way it ranks the flat rail. The header strip only wraps into rows on desktop, where `updateTabOverflowMode()` forces `tabs-auto-wrap`; the breaks only display in a wrapping strip, since a `flex-basis: 100%` break in a nowrap scroller would steal width.
|
⚠️ **By state is the flex `order` property, never a DOM reorder**, exactly like the sorted rail: `#sessionTabs` stays in `sessionOrder`, so the Alt+N badges, drag, the keyboard walk (which sorts by COMPUTED order) and the sidebar filter keep reading the list they always read. Each group owns a band of `TAB_TRIAGE_STRIDE` order values: its heading, its rows, its web tabs, then the row break that ends the header line. The headings and breaks are `aria-hidden` direct children of `#sessionTabs`, reconciled in place by `_syncTabTriageChrome()` after BOTH render paths, because a state change is an incremental pass and can still empty a group or fill a new one. Inside a header row tabs keep TAB order; a sorted rail ranks each section the way it ranks the flat rail. The header strip only wraps into rows on desktop, where `updateTabOverflowMode()` forces `tabs-auto-wrap`; the breaks only display in a wrapping strip, since a `flex-basis: 100%` break in a nowrap scroller would steal width.
|
||||||
|
|
||||||
|
|||||||
+32
-7
@@ -5174,10 +5174,16 @@ class CodemanApp {
|
|||||||
* every later row starting under it.
|
* every later row starting under it.
|
||||||
*
|
*
|
||||||
* The labels are measured only when their text changes (a group appears,
|
* The labels are measured only when their text changes (a group appears,
|
||||||
* goes, or its count gains a digit) and once more when the web fonts finish
|
* goes, or its count gains a digit), when the strip starts wrapping, and
|
||||||
* loading. The brand is watched by a ResizeObserver (a display-name change,
|
* once more when the web fonts finish loading. Only the WRAPPING strip reads
|
||||||
* the sidebar toggle appearing), so a render pass never forces a layout read
|
* the label column, so the phone and tablet row (headings hidden under
|
||||||
* for it. The vertical lists use neither length and are never measured.
|
* 600px, inline dividers above) is never measured and keeps no measurement:
|
||||||
|
* a phone turned to landscape or a foldable opened crosses into the wrapping
|
||||||
|
* strip with no tab render behind it, and updateTabOverflowMode(), which the
|
||||||
|
* resize handler calls, sizes it right after deciding to wrap. The brand is
|
||||||
|
* watched by a ResizeObserver (a display-name change, the sidebar toggle
|
||||||
|
* appearing), so a render pass never forces a layout read for it. The
|
||||||
|
* vertical lists use neither length and are never measured.
|
||||||
*/
|
*/
|
||||||
_sizeTabTriageGutter(container, triage) {
|
_sizeTabTriageGutter(container, triage) {
|
||||||
const inHeader = !!container.parentElement?.classList.contains('session-tabs-host');
|
const inHeader = !!container.parentElement?.classList.contains('session-tabs-host');
|
||||||
@@ -5194,15 +5200,31 @@ class CodemanApp {
|
|||||||
container.style.setProperty('--tab-triage-brand', brand);
|
container.style.setProperty('--tab-triage-brand', brand);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Not wrapping: forget the measurement, so wrapping again measures afresh.
|
||||||
|
if (!container.classList.contains('tabs-auto-wrap') && !container.classList.contains('tabs-two-rows')) {
|
||||||
|
this._tabTriageGutterKey = null;
|
||||||
|
return;
|
||||||
|
}
|
||||||
const key = triage.groups.map((g) => `${g.key}:${g.count}`).join('|');
|
const key = triage.groups.map((g) => `${g.key}:${g.count}`).join('|');
|
||||||
if (key === this._tabTriageGutterKey) return;
|
if (key === this._tabTriageGutterKey) return;
|
||||||
let widest = 0;
|
let widest = 0;
|
||||||
for (const head of container.querySelectorAll(':scope > .tab-triage-head')) {
|
for (const head of container.querySelectorAll(':scope > .tab-triage-head')) {
|
||||||
|
// Laid-out parts only. A hidden heading measures 0 per part, and counting
|
||||||
|
// the 5px gap between its parts anyway turned that into a 15px column
|
||||||
|
// that was then cached as if measured.
|
||||||
let width = 0;
|
let width = 0;
|
||||||
for (const part of head.children) width += part.getBoundingClientRect().width;
|
let parts = 0;
|
||||||
widest = Math.max(widest, width + 5 * Math.max(0, head.children.length - 1));
|
for (const part of head.children) {
|
||||||
|
const partWidth = part.getBoundingClientRect().width;
|
||||||
|
if (partWidth > 0) {
|
||||||
|
width += partWidth;
|
||||||
|
parts++;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (parts) widest = Math.max(widest, width + 5 * (parts - 1));
|
||||||
}
|
}
|
||||||
// Hidden (display: none on phones, or a detached strip): nothing to size.
|
// Hidden (display: none, or a detached strip): nothing to size, and no key
|
||||||
|
// either, so the next pass measures again.
|
||||||
if (!widest) return;
|
if (!widest) return;
|
||||||
this._tabTriageGutterKey = key;
|
this._tabTriageGutterKey = key;
|
||||||
container.style.setProperty('--tab-triage-gutter', `${Math.ceil(widest + 10)}px`);
|
container.style.setProperty('--tab-triage-gutter', `${Math.ceil(widest + 10)}px`);
|
||||||
@@ -6394,6 +6416,8 @@ class CodemanApp {
|
|||||||
|
|
||||||
if (manualTwoRows || deviceType !== 'desktop') {
|
if (manualTwoRows || deviceType !== 'desktop') {
|
||||||
container.classList.remove('tabs-auto-wrap');
|
container.classList.remove('tabs-auto-wrap');
|
||||||
|
// Wrap decided: the state labels' column follows it (_sizeTabTriageGutter).
|
||||||
|
this._sizeTabTriageGutter(container, this._lastTabTriage);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -6404,6 +6428,7 @@ class CodemanApp {
|
|||||||
// ledger stays the plain strip.
|
// ledger stays the plain strip.
|
||||||
if (container.classList.contains('tabs-triage') || container.classList.contains('tabs-ledger')) {
|
if (container.classList.contains('tabs-triage') || container.classList.contains('tabs-ledger')) {
|
||||||
container.classList.add('tabs-auto-wrap');
|
container.classList.add('tabs-auto-wrap');
|
||||||
|
this._sizeTabTriageGutter(container, this._lastTabTriage);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+91
-1
@@ -17,6 +17,10 @@
|
|||||||
* - In the phone/tablet strip (one scrolling row) the ACTIVE tab changing band
|
* - In the phone/tablet strip (one scrolling row) the ACTIVE tab changing band
|
||||||
* is revealed, since its chip moves while scrollLeft stays; another tab
|
* is revealed, since its chip moves while scrollLeft stays; another tab
|
||||||
* changing band never moves the strip (#257's browse-the-far-end rule).
|
* changing band never moves the strip (#257's browse-the-far-end rule).
|
||||||
|
* - The desktop label column (`--tab-triage-gutter`) is never sized or cached
|
||||||
|
* from headings that are not laid out (mobile.css hides them under 600px),
|
||||||
|
* and is measured again whenever the strip starts wrapping, which a phone
|
||||||
|
* turned to landscape or a foldable opened does with no tab render behind it.
|
||||||
*
|
*
|
||||||
* The real modules run INSIDE a JSDOM window (runScripts: 'outside-only'), so
|
* The real modules run INSIDE a JSDOM window (runScripts: 'outside-only'), so
|
||||||
* `document` below is that window's.
|
* `document` below is that window's.
|
||||||
@@ -28,7 +32,7 @@ import { readFileSync } from 'node:fs';
|
|||||||
import { join } from 'node:path';
|
import { join } from 'node:path';
|
||||||
import vm from 'node:vm';
|
import vm from 'node:vm';
|
||||||
import { JSDOM } from 'jsdom';
|
import { JSDOM } from 'jsdom';
|
||||||
import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
|
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
|
||||||
const PUBLIC = join(process.cwd(), 'src/web/public');
|
const PUBLIC = join(process.cwd(), 'src/web/public');
|
||||||
const read = (name: string) => readFileSync(join(PUBLIC, name), 'utf8');
|
const read = (name: string) => readFileSync(join(PUBLIC, name), 'utf8');
|
||||||
@@ -510,6 +514,92 @@ describe('tab grouping in the render paths (app.js)', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('sizing the label column (--tab-triage-gutter)', () => {
|
||||||
|
// JSDOM lays nothing out, so every heading measures as hidden (0 wide) until
|
||||||
|
// `layHeadings()` gives its label and count a width, the way a desktop
|
||||||
|
// browser would once mobile.css stops hiding them.
|
||||||
|
const gutter = () => container().style.getPropertyValue('--tab-triage-gutter');
|
||||||
|
function layHeadings(widths: { label: number; count: number } | null) {
|
||||||
|
for (const head of container().querySelectorAll<HTMLElement>(':scope > .tab-triage-head')) {
|
||||||
|
for (const part of head.children) {
|
||||||
|
const width = !widths ? 0 : part.classList.contains('tab-triage-label') ? widths.label : widths.count;
|
||||||
|
(part as HTMLElement).getBoundingClientRect = () => ({ width }) as DOMRect;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
let realDeviceType: () => string;
|
||||||
|
beforeEach(() => {
|
||||||
|
realDeviceType = window.MobileDetection.getDeviceType;
|
||||||
|
});
|
||||||
|
afterEach(() => {
|
||||||
|
window.MobileDetection.getDeviceType = realDeviceType;
|
||||||
|
});
|
||||||
|
/** An app whose updateTabOverflowMode() is the real one, as the resize handler calls it. */
|
||||||
|
function liveOverflowApp(deviceType: string) {
|
||||||
|
const app = makeApp();
|
||||||
|
app.updateTabOverflowMode = CodemanApp.prototype.updateTabOverflowMode;
|
||||||
|
app.loadAppSettingsFromStorage = () => ({});
|
||||||
|
app.getDefaultSettings = () => ({});
|
||||||
|
window.MobileDetection.getDeviceType = () => deviceType;
|
||||||
|
return app;
|
||||||
|
}
|
||||||
|
|
||||||
|
it('never sizes or keys the column from hidden headings', () => {
|
||||||
|
const app = makeApp();
|
||||||
|
app._fullRenderSessionTabs();
|
||||||
|
// The phone row: not wrapping, headings hidden.
|
||||||
|
expect(gutter()).toBe('');
|
||||||
|
expect(app._tabTriageGutterKey).toBeNull();
|
||||||
|
// Even in a wrapping strip, headings that are not laid out size nothing.
|
||||||
|
// They used to: 5px of gap per label + count made a 15px column, cached.
|
||||||
|
container().classList.add('tabs-auto-wrap');
|
||||||
|
app._sizeTabTriageGutter(container(), app._lastTabTriage);
|
||||||
|
expect(gutter()).toBe('');
|
||||||
|
expect(app._tabTriageGutterKey).toBeNull();
|
||||||
|
// Laid out, the widest label (+ the gap + 10px) sizes it and is cached.
|
||||||
|
layHeadings({ label: 50, count: 8 });
|
||||||
|
app._sizeTabTriageGutter(container(), app._lastTabTriage);
|
||||||
|
expect(gutter()).toBe('73px');
|
||||||
|
expect(app._tabTriageGutterKey).not.toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('measures when a phone turns to landscape or a foldable opens, with no tab render', () => {
|
||||||
|
const app = liveOverflowApp('mobile');
|
||||||
|
app._fullRenderSessionTabs();
|
||||||
|
app.updateTabOverflowMode();
|
||||||
|
expect(container().classList.contains('tabs-auto-wrap')).toBe(false);
|
||||||
|
expect(gutter()).toBe('');
|
||||||
|
// Unfolded: mobile.css no longer hides the headings, and the resize
|
||||||
|
// handler's updateTabOverflowMode() is the only call that arrives.
|
||||||
|
layHeadings({ label: 50, count: 8 });
|
||||||
|
window.MobileDetection.getDeviceType = () => 'desktop';
|
||||||
|
app.updateTabOverflowMode();
|
||||||
|
expect(container().classList.contains('tabs-auto-wrap')).toBe(true);
|
||||||
|
expect(gutter()).toBe('73px');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('measures afresh after the strip stopped wrapping and starts again, counts unchanged', () => {
|
||||||
|
const app = liveOverflowApp('desktop');
|
||||||
|
layHeadings({ label: 50, count: 8 });
|
||||||
|
app._fullRenderSessionTabs();
|
||||||
|
layHeadings({ label: 50, count: 8 });
|
||||||
|
app.updateTabOverflowMode();
|
||||||
|
expect(gutter()).toBe('73px');
|
||||||
|
// Folded: the strip stops wrapping and the measurement is forgotten.
|
||||||
|
layHeadings(null);
|
||||||
|
window.MobileDetection.getDeviceType = () => 'mobile';
|
||||||
|
app.updateTabOverflowMode();
|
||||||
|
expect(container().classList.contains('tabs-auto-wrap')).toBe(false);
|
||||||
|
expect(app._tabTriageGutterKey).toBeNull();
|
||||||
|
// Unfolded again with wider labels (a font, a skin): re-measured, even
|
||||||
|
// though no group or count changed.
|
||||||
|
layHeadings({ label: 60, count: 8 });
|
||||||
|
window.MobileDetection.getDeviceType = () => 'desktop';
|
||||||
|
app.updateTabOverflowMode();
|
||||||
|
expect(gutter()).toBe('83px');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it('degrades to the flat strip when mobile-overview.js is stale or missing', () => {
|
it('degrades to the flat strip when mobile-overview.js is stale or missing', () => {
|
||||||
const app = makeApp();
|
const app = makeApp();
|
||||||
app._mobileOverviewState = undefined;
|
app._mobileOverviewState = undefined;
|
||||||
|
|||||||
Reference in New Issue
Block a user