diff --git a/src/web/public/app.js b/src/web/public/app.js index f96281dd..897e4e35 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -6359,12 +6359,29 @@ class CodemanApp { tabCount: this.sessions.size, scrollWidth: container.scrollWidth, clientWidth: container.clientWidth, + innerWrap: container.classList.contains('tabs-clusters') && this._tabClustersWrapInside(container), }) : container.scrollWidth > container.clientWidth + 1; container.classList.toggle('tabs-auto-wrap', shouldWrap); } + /** + * True when a case cluster in the header strip wraps inside its own box: a case + * wider than the whole strip (styles.css caps a box at the strip's width). The + * strip then has rows although it never overflows, so it must still wrap, or + * the lineage routing room (`.lineage-tree.tabs-auto-wrap`) is never reserved + * and the routes have no gap between the box's rows to run in. Read right + * after the overflow measure, so layout is already clean. + */ + _tabClustersWrapInside(container) { + for (const box of container.querySelectorAll(':scope > .tab-cluster')) { + const tabs = box.querySelectorAll(':scope > .session-tab'); + if (tabs.length > 1 && tabs[tabs.length - 1].offsetTop > tabs[0].offsetTop + 4) return true; + } + return false; + } + // Middle-click closes a tab, mirroring browser tab strips. Session tabs go // through requestCloseSession (the same confirm modal as the x button), web // tabs through closeWebviewTab (same as theirs). Delegated on the container: diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 087944fc..9e8661f1 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -207,6 +207,9 @@ function shouldAutoWrapTabs(input) { if (!input || input.deviceType !== 'desktop') return false; if (input.manualTwoRows) return false; if ((input.tabCount || 0) < 2) return false; + // A box in the strip already wraps inside itself (a case cluster wider than the + // whole strip): the strip has rows although nothing overflows. + if (input.innerWrap) return true; const scrollWidth = Number(input.scrollWidth) || 0; const clientWidth = Number(input.clientWidth) || 0; @@ -330,6 +333,12 @@ function computeTabScrollLeft(input) { // left out of `routes`. `.session-tabs` scrolls, so a scrolled-out tab still HAS a // rect, lying over the logo or the header buttons. Skipping is honest; clamping // would point at a tab that is not there. +// +// A child is also left out when its route has nowhere to run: rows whose spans +// overlap are one row (no gap between them), a row with no gap under it routes +// nothing, and a route that would still cross a tab is dropped whole. So no tab +// arrangement can put a line through a tab; a layout without the reserved room +// loses lines instead. const LINEAGE_CORNER_RADIUS_PX = 10; // Families drawn together take separate lanes: gap lines this far apart, spines // LINEAGE_SPINE_STEP_PX apart. @@ -342,6 +351,8 @@ const LINEAGE_SPINE_STEP_PX = 4; // The last row has no row below it; with no strip rect to measure, its gap is // taken to be this deep. const LINEAGE_LAST_GAP_PX = 12; +// Narrower than this, the space between two rows is not a gap a route can run in. +const LINEAGE_MIN_GAP_PX = 2; const LINEAGE_ROW_TOLERANCE_PX = 6; const LINEAGE_STRIP_TOLERANCE_PX = 4; // How far the vertical rail's track sits in from the rail's left edge. It has to @@ -385,21 +396,37 @@ function lineageRect(rect) { * Group tab rects into the strip's visual rows, top to bottom. A row spans from its * highest top to its LOWEST bottom, so a taller tab (the active one) sets the row's * gap for everyone in it. + * + * ⚠ Rows never overlap. Tabs whose spans overlap are in one row however far apart + * their tops are: case clusters stack and centre tabs inside their boxes, and two + * overlapping "rows" put the gap of one in the middle of the other's tabs. */ function computeLineageRows(rects) { - const rows = []; + const sorted = []; for (const raw of rects || []) { const r = lineageRect(raw); - if (!r) continue; - const row = rows.find((candidate) => Math.abs(candidate.top - r.top) <= LINEAGE_ROW_TOLERANCE_PX); - if (row) { - row.top = Math.min(row.top, r.top); + if (r) sorted.push(r); + } + sorted.sort((a, b) => a.top - b.top); + const rows = []; + for (const r of sorted) { + // Sorted by top, so only the last row can take this rect, and merging into it + // keeps every earlier row clear of it. + const row = rows[rows.length - 1]; + if (row && (r.top < row.bottom || r.top - row.top <= LINEAGE_ROW_TOLERANCE_PX)) { row.bottom = Math.max(row.bottom, r.bottom); } else { rows.push({ top: r.top, bottom: r.bottom }); } } - return rows.sort((a, b) => a.top - b.top); + return rows; +} + +/** Does an axis-aligned segment pass through the inside of a rect? Touching an edge is fine. */ +function lineageSegmentCrosses([x1, y1], [x2, y2], r) { + const eps = 0.5; + if (Math.max(x1, x2) <= r.left + eps || Math.min(x1, x2) >= r.right - eps) return false; + return Math.max(y1, y2) > r.top + eps && Math.min(y1, y2) < r.bottom - eps; } /** @@ -503,19 +530,23 @@ function computeLineageTree(input) { const rowOf = (r) => rows.findIndex((row) => r.cy >= row.top - tol && r.cy <= row.bottom + tol); const laneOffset = (lane - (laneCount - 1) / 2) * LINEAGE_LANE_STEP_PX; // Y of the gap under row i, this family's lane. The offset is clamped so a busy - // gap never pushes a lane into the tabs on either side of it. + // gap never pushes a lane into the tabs on either side of it. Null when there is + // no gap: the row is not found, or the next row starts (nearly) where it ends. const gapUnder = (i) => { const row = rows[i]; + if (!row) return null; const next = rows[i + 1]; let bottom; if (next) bottom = next.top; else if (strip && strip.bottom > row.bottom + 1) bottom = strip.bottom; else bottom = row.bottom + LINEAGE_LAST_GAP_PX; + if (!(bottom - row.bottom >= LINEAGE_MIN_GAP_PX)) return null; const half = Math.max(0, (bottom - row.bottom) / 2 - 1); return (row.bottom + bottom) / 2 + Math.max(-half, Math.min(half, laneOffset)); }; const pRow = rowOf(parent); const gp = gapUnder(pRow); + if (gp === null) return { routes }; // The spine runs from one row's gap to another's, so it only ever passes BESIDE // rows after the first, and only their tabs bound it. The first row may start // left of the channel (grouped by state it starts after the brand and a label @@ -524,16 +555,23 @@ function computeLineageTree(input) { const lowerLefts = [parent, ...visible.map((c) => c.rect), ...(input?.tabs || []).map(lineageRect)] .filter((r) => r && rowOf(r) > 0) .map((r) => r.left); - const minLeft = lowerLefts.length ? Math.min(...lowerLefts) : Math.min(parent.left, ...visible.map((c) => c.rect.left)); + const minLeft = lowerLefts.length + ? Math.min(...lowerLefts) + : Math.min(parent.left, ...visible.map((c) => c.rect.left)); const channelLeft = Number(input?.spineLeft); const spineBase = strip ? Math.max(strip.left, Number.isFinite(channelLeft) ? channelLeft : strip.left) : minLeft - LINEAGE_SPINE_INSET_PX * 2; const spineX = Math.min(spineBase + LINEAGE_SPINE_INSET_PX + lane * LINEAGE_SPINE_STEP_PX, minLeft - 2); + // Every tab a route must stay out of. A segment can only cross one if the rows + // above failed to describe the layout, so this is a backstop that drops the + // route whole rather than drawing it through a tab. + const obstacles = [parent, ...visible.map((c) => c.rect), ...(input?.tabs || []).map(lineageRect)].filter(Boolean); for (const { id, rect } of visible) { const cRow = rowOf(rect); const gc = gapUnder(cRow); + if (gc === null) continue; const points = cRow === pRow ? [ @@ -550,6 +588,10 @@ function computeLineageTree(input) { [rect.cx, gc], [rect.cx, rect.bottom], ]; + // The rounded corners cut inside each turn by a few pixels at most + // (lineagePolylinePath), so the segments are what has to clear the tabs. + const blocked = points.some((p, i) => i > 0 && obstacles.some((r) => lineageSegmentCrosses(points[i - 1], p, r))); + if (blocked) continue; const d = lineagePolylinePath(points, radius); if (d) routes.push({ id, points: roundPoints(points), d, endX: r1(rect.cx), endY: r1(rect.bottom) }); } diff --git a/src/web/public/styles.css b/src/web/public/styles.css index 753f7ce6..48ff229c 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -20891,6 +20891,21 @@ html[data-session-list='sidebar'][data-sidebar='collapsed'] .session-sidebar .ta padding-left: 3px; } +/* Desktop: a cluster keeps its width, so clusters that do not fit on one line + overflow the strip and updateTabOverflowMode() wraps it box by box. Allowed + to shrink, every box squeezed and wrapped inside itself instead (a one-tab + case's swatch alone on a line above its tab), the strip never overflowed, + and the lineage routing room, which needs a wrapped strip, never applied: + the routes ran through tabs, labels and box borders. Only a case wider than + the whole strip still wraps inside its box (max-width: 100%), and the strip + wraps around it too (_tabClustersWrapInside). The narrower screens keep + their scrolling strip. */ +@media (min-width: 768px) { + .session-tabs-host > .session-tabs.tabs-clusters > .tab-cluster { + flex-shrink: 0; + } +} + /* Side rail and sidebar: every case is a section with its label on top (vertical space is cheap there, so a one-tab case is labelled too). The label goes first by `order`, because a sorted rail orders the rows inside diff --git a/test/session-lineage-lines.test.ts b/test/session-lineage-lines.test.ts index ce69542a..1bad4570 100644 --- a/test/session-lineage-lines.test.ts +++ b/test/session-lineage-lines.test.ts @@ -380,6 +380,87 @@ describe('lineage tree geometry: header strip', () => { }); }); +describe('lineage tree geometry: never through a tab, in any arrangement', () => { + // Measured live (1440x900, tabArrangement 'case', nine tabs in four cases) before + // the case boxes kept their width: every box squeezed and wrapped inside itself, + // so tabs sat at tops 10, 20, 42 and 43 with spans that overlap. The old rows + // (grouped by top alone) put the "gap" under the first row at y 30, inside the + // tabs, and the routes ran through them. + const strip: Rect = { left: 94, top: 6, width: 980, height: 85 }; + const squeezed: Record = { + 'w1-webshop': { left: 201, top: 10, width: 99, height: 30 }, + 'w2-webshop': { left: 109, top: 42, width: 101, height: 30 }, + 'w3-webshop': { left: 212, top: 42, width: 101, height: 30 }, + 'w1-api-gateway': { left: 518, top: 10, width: 99, height: 30 }, + 'w2-api-gateway': { left: 407, top: 42, width: 143, height: 32 }, + 'w3-api-gateway': { left: 552, top: 43, width: 101, height: 30 }, + 'w1-notes': { left: 744, top: 20, width: 136, height: 30 }, + 'w1-docs-site': { left: 968, top: 10, width: 99, height: 30 }, + 'w2-docs-site': { left: 870, top: 42, width: 101, height: 30 }, + }; + const all = Object.values(squeezed); + + it('makes overlapping spans one row, so no gap is ever inside a tab', () => { + const helper = loadLineageHelper(); + // 10-40 and 20-50 overlap, and 42-74 overlaps 20-50: one row, no gap inside. + expect(helper.computeRows(all)).toEqual([{ top: 10, bottom: 74 }]); + // Rows that only share a top (the active tab is taller) stay one row, and rows + // with a real gap between them stay apart. + expect(helper.computeRows([tab(0, 4), tab(130, 4, 120, 32), tab(0, 46)])).toEqual([ + { top: 4, bottom: 36 }, + { top: 46, bottom: 76 }, + ]); + }); + + it('draws no segment inside a tab, whichever tab is the parent or the child', () => { + const helper = loadLineageHelper(); + const names = Object.keys(squeezed); + let drawn = 0; + for (const parentName of names) { + for (const lane of [0, 1, 2]) { + const children = names.filter((n) => n !== parentName).map((n) => ({ id: n, rect: squeezed[n] })); + const geom = helper.computeTree({ + parent: squeezed[parentName], + children, + strip, + tabs: all, + lane, + laneCount: 3, + }); + for (const route of geom?.routes ?? []) { + drawn++; + for (let i = 1; i < route.points.length; i++) { + for (const r of all) { + expect(crossesRect(route.points[i - 1], route.points[i], r), `${parentName} -> ${route.id}`).toBe(false); + } + } + } + } + } + // Some routes still have a clean way (a tab with nothing under it), so the + // check above is not vacuous. + expect(drawn).toBeGreaterThan(0); + }); + + it('draws nothing into rows with no gap between them', () => { + const helper = loadLineageHelper(); + // Two rows touching (no row gap at all): the only place a route could run is + // through the tabs of the other row. + const upper = [tab(0, 4), tab(130, 4)]; + const lower = [tab(0, 34), tab(130, 34)]; + const geom = helper.computeTree({ + parent: upper[0], + children: [ + { id: 'below', rect: lower[1] }, + { id: 'beside', rect: upper[1] }, + ], + strip: { left: 0, top: 0, width: 600, height: 80 }, + tabs: [...upper, ...lower], + })!; + expect(geom.routes).toEqual([]); + }); +}); + describe('lineage tree geometry: vertical rail', () => { const strip: Rect = { left: 100, top: 20, width: 320, height: 320 }; const parent: Rect = { left: 132, top: 40, width: 260, height: 40 }; diff --git a/test/tab-clusters.test.ts b/test/tab-clusters.test.ts index bf0521aa..21028e1f 100644 --- a/test/tab-clusters.test.ts +++ b/test/tab-clusters.test.ts @@ -171,7 +171,9 @@ describe('tab layouts by case and ledger (app.js)', () => { name: box.querySelector('.tab-cluster-name')?.textContent ?? null, count: box.querySelector('.tab-cluster-count')?.textContent ?? null, single: box.classList.contains('tab-cluster--single'), - rows: [...box.querySelectorAll('.session-tab')].map((t) => t.dataset.id || `web:${t.dataset.webviewId}`), + rows: [...box.querySelectorAll('.session-tab')].map( + (t) => t.dataset.id || `web:${t.dataset.webviewId}` + ), })); beforeEach(() => { @@ -190,7 +192,9 @@ describe('tab layouts by case and ledger (app.js)', () => { { name: null, count: null, single: true, rows: ['web:w1'] }, ]); for (const box of container().querySelectorAll(':scope > .tab-cluster:not(.tab-cluster--web)')) { - expect(box.getAttribute('style')).toMatch(/^--cluster-color: var\(--session-(blue|green|purple|orange|pink|yellow|red)\)$/); + expect(box.getAttribute('style')).toMatch( + /^--cluster-color: var\(--session-(blue|green|purple|orange|pink|yellow|red)\)$/ + ); expect(box.getAttribute('role')).toBe('presentation'); } }); @@ -261,6 +265,35 @@ describe('tab layouts by case and ledger (app.js)', () => { expect(container().classList.contains('tabs-clusters')).toBe(false); }); + it('wraps the header strip when a case box wraps inside itself', () => { + // A case wider than the strip wraps in its own box and never overflows, so + // the measured auto-wrap alone would leave the strip unwrapped, and with it the + // lineage routing room unreserved. + const app = makeApp(); + app._fullRenderSessionTabs(); + Object.assign(app, { + _syncLineageGutter: () => {}, + isSessionSidebarActive: () => false, + loadAppSettingsFromStorage: () => ({ tabArrangement: 'case' }), + getDefaultSettings: () => ({ tabTwoRows: false, tabOrientation: 'horizontal' }), + }); + const webshop = [...container().querySelectorAll('[data-cluster-key="/c/webshop"] > .session-tab')]; + expect(webshop).toHaveLength(3); + const place = (tops: number[]) => + webshop.forEach((el, i) => Object.defineProperty(el, 'offsetTop', { configurable: true, value: tops[i] })); + const overflow = CodemanApp.prototype.updateTabOverflowMode; + + place([0, 0, 0]); + overflow.call(app); + expect(container().classList.contains('tabs-auto-wrap')).toBe(false); + expect(app._tabClustersWrapInside(container())).toBe(false); + + place([0, 0, 38]); + overflow.call(app); + expect(app._tabClustersWrapInside(container())).toBe(true); + expect(container().classList.contains('tabs-auto-wrap')).toBe(true); + }); + it('draws the ledger with the classic markup and one class', () => { makeApp('classic')._fullRenderSessionTabs(); const classic = container().innerHTML; @@ -279,6 +312,19 @@ describe('tab layouts by case and ledger (static)', () => { expect(css).toMatch(/\.session-tabs\.tabs-clusters \.tab-name-case \{\s*display: none;/); }); + it('keeps each case box its width on the desktop header strip, so the strip wraps box by box', () => { + // Allowed to shrink, every box squeezed and wrapped inside itself at 1440px + // (overlapping tab rows, the lineage routes through tabs and labels) and the + // strip never overflowed, so it never wrapped. + const media = css.indexOf( + '@media (min-width: 768px) {\n .session-tabs-host > .session-tabs.tabs-clusters > .tab-cluster {' + ); + expect(media).toBeGreaterThan(-1); + expect(css.slice(media, css.indexOf('}', media))).toContain('flex-shrink: 0;'); + // Only a case wider than the strip wraps inside its box. + expect(css).toMatch(/\.session-tabs-host > \.session-tabs\.tabs-clusters > \.tab-cluster \{[^}]*max-width: 100%;/); + }); + it('keeps the ledger to the desktop header strip', () => { const media = css.indexOf('@media (min-width: 768px) {\n .session-tabs-host > .session-tabs.tabs-ledger {'); expect(media).toBeGreaterThan(-1); @@ -288,7 +334,9 @@ describe('tab layouts by case and ledger (static)', () => { }); it('keeps every ledger row one height and makes the active cell stand out', () => { - const ledger = css.slice(css.indexOf('@media (min-width: 768px) {\n .session-tabs-host > .session-tabs.tabs-ledger {')); + const ledger = css.slice( + css.indexOf('@media (min-width: 768px) {\n .session-tabs-host > .session-tabs.tabs-ledger {') + ); expect(ledger).toContain('align-items: stretch;'); expect(ledger).toContain('min-height: 30px;'); expect(ledger).toMatch( diff --git a/test/tab-overflow.test.ts b/test/tab-overflow.test.ts index 65473cfe..fccd591c 100644 --- a/test/tab-overflow.test.ts +++ b/test/tab-overflow.test.ts @@ -78,6 +78,22 @@ describe('tab overflow layout policy', () => { // A single overflowing tab must not wrap (need at least 2 to form a second row). expect(helper.shouldAutoWrapTabs({ ...base, tabCount: 1, scrollWidth: 1400, clientWidth: 760 })).toBe(false); }); + + it('wraps a strip whose case cluster already wraps inside itself, though nothing overflows', () => { + // A case wider than the whole strip is capped at its width and wraps in its + // box, so the strip has rows that scrollWidth never shows. Without the wrap, + // the lineage routing room (row gap, spine channel) is never reserved. + const helper = loadTabOverflowHelper(); + const fits = { manualTwoRows: false, tabCount: 12, scrollWidth: 800, clientWidth: 800 }; + + expect(helper.shouldAutoWrapTabs({ ...fits, deviceType: 'desktop', innerWrap: true })).toBe(true); + expect(helper.shouldAutoWrapTabs({ ...fits, deviceType: 'desktop', innerWrap: false })).toBe(false); + // The other guards still win. + expect(helper.shouldAutoWrapTabs({ ...fits, deviceType: 'tablet', innerWrap: true })).toBe(false); + expect(helper.shouldAutoWrapTabs({ ...fits, deviceType: 'desktop', manualTwoRows: true, innerWrap: true })).toBe( + false + ); + }); }); // Issue #257: the phone tab strip scrolls horizontally, so the active tab can