From 14de2b70124334259caa5a1eb30ba9d13a5f69db Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Mon, 24 Aug 2026 22:58:28 +0200 Subject: [PATCH] fix(tabs): keep the created stamp reachable on a tight rail, and do not skip the first render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review nits on the vertical rail's detailed rows. 1. The tab-rail-tight rule (below 288px) hides `.tab-meta-created`, and its comment claimed the value "survives in the row's title attribute either way". It did not: the only title carrying it lived ON that element, and a `display: none` element has no hover target, so the created stamp was not shrunk but gone with no way to ask for it. Rather than just correcting the comment, `_sidebarRichMetaHTML()` now puts BOTH absolute stamps on the `.tab-meta` line itself, so the pill and the gaps around the stamps remain as hover targets. An item's own title still wins where the item is visible. 2. applyTabOrientation() decided whether applyTabWrapSettings() had already re-rendered by comparing `_tallTabsEnabled` before and after. That reads an UNDEFINED previous value as "it rendered", but applyTabWrapSettings() deliberately renders nothing on its first call ever (it only establishes the baseline: `prevTallTabs !== undefined && prevTallTabs !== showFolder`). So on a first call that also flips the folder row, neither function rendered and the rows stayed stale. Reachable when the pre-paint script throws and leaves the layout attributes on their catch-branch fallbacks for applyTabOrientation() to correct. The guard now mirrors applyTabWrapSettings()'s own condition. Both new tests were run against the unfixed code first and fail there, which is the only thing that makes them regression tests. (The third, "does not render twice", passes either way by design: it pins that fix 2 did not introduce a double rebuild.) Verified in a real browser against a live server with two sessions, driving the narrowing through _setTabRailWidth() the way the resize drag does: at the 320 default the row reads "CREATED 2m ago · IDLE <1m" with the created element displayed; at 256 the tight class is on, the created element computes to display:none, the visible text drops to "IDLE <1m", and the meta line's title still reads "First created: ...". At 220 the compact threshold drops rich rows entirely. Screenshots confirm no truncation artifacts in either state. Full gate green (6104 passed), typecheck, lint, format, frontend-syntax and public-assets all clean. --- src/web/public/app.js | 16 +++++++++- src/web/public/settings-ui.js | 10 +++++- src/web/public/styles.css | 5 ++- test/session-list-layout.test.ts | 55 ++++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 3 deletions(-) diff --git a/src/web/public/app.js b/src/web/public/app.js index 3ae3c330..73eee32c 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -4202,7 +4202,21 @@ class CodemanApp { parts.push(stamp(row.since.key, row.since.at, 'for', 'tab-meta-since')); } parts.push(`${escapeHtml(row.pill)}`); - return `${parts.join('')}`; + // Both absolute stamps ALSO on the line itself, not only on the two items. + // Below 288px the rail hides `.tab-meta-created` (the `tab-rail-tight` + // rule), and a tooltip on a `display: none` element has no hover target — + // so without this the created stamp is not merely shrunk, it is gone with + // no way to ask for it. The pill and the gaps around the stamps are the + // hover targets that remain; an item's own title still wins over this one + // where the item is visible. + const lineTitle = [ + row.createdAt ? `First created: ${new Date(row.createdAt).toLocaleString()}` : '', + row.since && row.since.at ? `${row.since.key}: ${new Date(row.since.at).toLocaleString()}` : '', + ] + .filter(Boolean) + .join(' \u00B7 '); + const lineTitleAttr = lineTitle ? ` title="${escapeHtml(lineTitle)}"` : ''; + return `${parts.join('')}`; } /** Same formatter as both home screens, so a duration is written the same way everywhere. */ diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 9978ade3..5f07f3a8 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -2753,7 +2753,15 @@ Object.assign(CodemanApp.prototype, { const prevTall = this._tallTabsEnabled; if (changed) this.applyTabWrapSettings?.(); if (changed) { - if (prevTall === this._tallTabsEnabled) this._fullRenderSessionTabs?.(); + // Mirror of applyTabWrapSettings()'s OWN render condition, which is + // `prevTallTabs !== undefined && prevTallTabs !== showFolder`: its first + // call ever only establishes the baseline and deliberately renders + // nothing. Reading an undefined previous value as "it rendered" skips + // BOTH renders and leaves the rows stale — reachable whenever this is the + // first call, i.e. when the pre-paint script threw and left the + // attributes on their fallbacks for applyTabOrientation() to correct. + const wrapRendered = prevTall !== undefined && prevTall !== this._tallTabsEnabled; + if (!wrapRendered) this._fullRenderSessionTabs?.(); this._updateConnectionLinesImmediate?.(); this._refreshHomeSessionsIfVisible?.(); } diff --git a/src/web/public/styles.css b/src/web/public/styles.css index 4abc13e0..e4ebf4e7 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -17285,7 +17285,10 @@ html[data-tab-orientation='vertical'][data-tab-rail-detail='rich']:not(.tab-rail /* Narrower than the detailed default (the .tab-rail-tight class, _setTabRailWidth): the created stamp is dropped rather than shown as - "CREA…". Its value survives in the row's title attribute either way. */ + "CREA…". Its value stays reachable as the tooltip on the meta LINE + (`.tab-meta` carries both absolute stamps, _sidebarRichMetaHTML) — the title + on the hidden `.tab-meta-created` itself goes away with it, since a + `display: none` element has no hover target. */ html.tab-rail-tight[data-tab-orientation='vertical'][data-tab-rail-detail='rich']:not(.tab-rail-compact) .tab-rail .tab-meta-created, html.tab-rail-tight[data-tab-orientation='vertical'][data-tab-rail-detail='rich']:not(.tab-rail-compact) .tab-rail .tab-meta-sep { display: none; diff --git a/test/session-list-layout.test.ts b/test/session-list-layout.test.ts index bb0acab1..44898db5 100644 --- a/test/session-list-layout.test.ts +++ b/test/session-list-layout.test.ts @@ -711,6 +711,22 @@ describe('rich session sidebar', () => { expect(html).toContain('data-i18n-skip'); }); + it('carries both absolute stamps on the LINE, not only on the two items', () => { + // Below 288px the rail hides `.tab-meta-created` (tab-rail-tight), and a + // title on a `display: none` element has no hover target — so a tooltip + // living only there means the created stamp is gone, not shrunk. The line + // itself has to carry it for the CSS rule's "still reachable" to be true. + const { app } = boot({ stored: { sessionListLayout: 'sidebar-rich' } }); + stubOverview(app); + const html = app._sidebarRichMetaHTML(app._sidebarRichRow('s1', SESSION)); + + const line = html.slice(0, html.indexOf('>')); + expect(line).toContain('class="tab-meta"'); + expect(line).toContain('title="'); + expect(line).toContain('First created'); + expect(line).toContain('working'); + }); + it('drops the second stamp when the session has never been active', () => { const { app } = boot({ stored: { sessionListLayout: 'sidebar-rich' } }); stubOverview(app); @@ -906,6 +922,45 @@ describe('detailed rows in the vertical tab rail', () => { expect(app._sidebarRichClock).toBeTruthy(); }); + it('still renders on the first call, when applyTabWrapSettings only sets its baseline', () => { + // The pre-paint script stamps the layout attributes; if it THREW it leaves + // them on the catch-branch fallbacks and applyTabOrientation() is the first + // thing to correct them, with `_tallTabsEnabled` still undefined. + // applyTabWrapSettings() renders only when it has a previous value to + // compare, so reading "the value changed" as "it rendered" skipped BOTH + // renders and left the rows stale. + const { win, app } = boot({ stored: { sessionListLayout: 'header', tabOrientation: 'vertical' } }); + expect(app._tallTabsEnabled).toBeUndefined(); + expect(win.document.documentElement.getAttribute('data-tab-orientation')).toBeNull(); + + app.applyTabOrientation(); + + expect(win.document.documentElement.dataset.tabOrientation).toBe('vertical'); + // The folder row turned on in the same pass, so this is exactly the case + // where the two guards could point at each other and neither fires. + expect(app._tallTabsEnabled).toBe(true); + expect(app._fullRenderSessionTabs).toHaveBeenCalled(); + }); + + it('does not render twice when applyTabWrapSettings already did', () => { + // The mirror case: a detail flip that turns the folder row off makes + // applyTabWrapSettings() re-render, and applyTabOrientation() must not + // stack a second full rebuild of the strip on top of it. + const { win, app } = railBoot({ tabOrientation: 'vertical', tabRailDetail: 'rich' }); + expect(app._tallTabsEnabled).toBe(true); + (app._fullRenderSessionTabs as unknown as { mockClear(): void }).mockClear(); + + win.localStorage.setItem( + 'codeman-app-settings', + JSON.stringify({ sessionListLayout: 'header', tabOrientation: 'vertical', tabRailDetail: 'simple' }) + ); + delete (app as unknown as { _cachedAppSettings?: unknown })._cachedAppSettings; + app.applyTabOrientation(); + + expect(app._tallTabsEnabled).toBe(false); + expect(app._fullRenderSessionTabs).toHaveBeenCalledTimes(1); + }); + it('plumbs the rail detail through the settings UI, the schema and the pre-paint script', () => { expect(INDEX_HTML).toContain('id="appSettingsTabRailDetail"'); expect(INDEX_HTML).toContain('');