From dfe3a23a606a9530aeb05727d2d4e4e2f097e10b Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 20:57:32 -0500 Subject: [PATCH] fix(desktop): scroll the tab strip so a new tab and the "+" stay in view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening a tab in a zone with more tabs than fit appended it past the right edge of the scrolling strip — the new tab and the "+" that created it were both off-screen, so a second new tab meant scrolling back by hand. Activating a tab from a keybind had the same problem in the other direction. The zone header now scrolls its active tab into view on activation, and scrolls all the way to the end when that tab is the last one so the trailing "+" comes with it. --- .../tree/renderer/tab-strip-scroll.test.ts | 38 ++++++++ .../tree/renderer/tab-strip-scroll.ts | 97 +++++++++++++++++++ .../pane-shell/tree/renderer/tree-group.tsx | 14 +++ 3 files changed, 149 insertions(+) create mode 100644 apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-scroll.test.ts create mode 100644 apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-scroll.ts diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-scroll.test.ts b/apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-scroll.test.ts new file mode 100644 index 000000000000..a05c801fb56e --- /dev/null +++ b/apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-scroll.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' + +import { tabStripScrollLeft } from './tab-strip-scroll' + +describe('tabStripScrollLeft', () => { + // 300px window over 900px of tabs + "+" → 600px of scroll range. + const strip = (over: Partial[0]> = {}) => + tabStripScrollLeft({ + clientWidth: 300, + last: false, + scrollLeft: 0, + scrollWidth: 900, + tabEnd: 0, + tabStart: 0, + ...over + }) + + it('scrolls to the end for the last tab so the "+" comes with it', () => { + expect(strip({ last: true, scrollLeft: 0, tabEnd: 880, tabStart: 780 })).toBe(600) + }) + + it('leaves the offset alone when the tab is already fully visible', () => { + expect(strip({ scrollLeft: 200, tabEnd: 400, tabStart: 300 })).toBe(200) + }) + + it('scrolls left to the tab start when it sits before the window', () => { + expect(strip({ scrollLeft: 400, tabEnd: 200, tabStart: 100 })).toBe(100) + }) + + it('scrolls right just far enough to reveal a tab past the window', () => { + expect(strip({ scrollLeft: 0, tabEnd: 450, tabStart: 350 })).toBe(150) + }) + + it('never exceeds the scrollable range', () => { + expect(strip({ last: true, scrollWidth: 250 })).toBe(0) + expect(strip({ scrollLeft: 0, tabEnd: 1200, tabStart: 1100 })).toBe(600) + }) +}) diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-scroll.ts b/apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-scroll.ts new file mode 100644 index 000000000000..236896fb72d3 --- /dev/null +++ b/apps/desktop/src/components/pane-shell/tree/renderer/tab-strip-scroll.ts @@ -0,0 +1,97 @@ +/** + * Keeping the active tab (and the trailing "+") inside a scrolling tab strip. + * + * Once a zone has more tabs than fit, opening a new one appended it past the + * right edge: the tab you just created — and the "+" that created it — were + * both off-screen, so a second new tab meant scrolling back by hand first. + * Activating a tab from a keybind or the session list had the same problem in + * the other direction. + */ + +import { type RefObject, useLayoutEffect } from 'react' + +export interface TabStripGeometry { + /** Visible width of the strip. */ + clientWidth: number + /** Active tab is the LAST one, so reveal the trailing "+" along with it. */ + last: boolean + scrollLeft: number + /** Full scroll content: every tab plus the trailing "+". */ + scrollWidth: number + /** Active tab's edges, measured from the start of the scroll content. */ + tabEnd: number + tabStart: number +} + +/** Where the strip should be scrolled to for the active tab to be in view — + * the current offset when it already is (the caller skips the write). */ +export function tabStripScrollLeft({ + clientWidth, + last, + scrollLeft, + scrollWidth, + tabEnd, + tabStart +}: TabStripGeometry): number { + const max = Math.max(0, scrollWidth - clientWidth) + + // The last tab scrolls to the very end rather than to its own edge: the "+" + // lives after it in the same scroll content and has to come along. + if (last) { + return max + } + + if (tabStart < scrollLeft) { + return Math.min(tabStart, max) + } + + if (tabEnd > scrollLeft + clientWidth) { + return Math.min(tabEnd - clientWidth, max) + } + + return Math.min(scrollLeft, max) +} + +/** Scroll `activeId`'s tab into view whenever the activation or the tab set + * changes. `last` drives the "+"-follows-the-final-tab case. */ +export function useActiveTabVisible( + scrollerRef: RefObject, + activeId: string, + { enabled, last, tabCount }: { enabled: boolean; last: boolean; tabCount: number } +): void { + // Layout effect: the scroll write lands in the same frame as the tab's + // insertion, so a new tab never paints off-screen first. + useLayoutEffect(() => { + const scroller = scrollerRef.current + + if (!enabled || !scroller) { + return + } + + const tab = scroller.querySelector(`[data-tree-tab="${CSS.escape(activeId)}"]`) + + if (!tab) { + return + } + + // Rects, not offsetLeft: the tab's offsetParent is the header strip (the + // scroller itself isn't positioned), so offsets would be measured against + // the wrong origin. + const view = scroller.getBoundingClientRect() + const rect = tab.getBoundingClientRect() + const tabStart = rect.left - view.left + scroller.scrollLeft + + const next = tabStripScrollLeft({ + clientWidth: scroller.clientWidth, + last, + scrollLeft: scroller.scrollLeft, + scrollWidth: scroller.scrollWidth, + tabEnd: tabStart + rect.width, + tabStart + }) + + if (next !== scroller.scrollLeft) { + scroller.scrollLeft = next + } + }, [activeId, enabled, last, scrollerRef, tabCount]) +} diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx index b46d4068479e..8ed3ca944049 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx +++ b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx @@ -49,6 +49,7 @@ import { import { type DoubleTapContext, startPaneDrag } from './drag-session' import { forceLoneHeaderForPanes } from './lone-header' +import { useActiveTabVisible } from './tab-strip-scroll' import { paneChrome } from './track-model' /** A directional action in the zone menu (computed per group state). */ @@ -144,6 +145,9 @@ export function TreeGroup({ const { t } = useI18n() const ref = useRef(null) const stripRef = useRef(null) + // The scrolling tab list inside the header (the strip also holds the + // minimize chevron, which must not scroll away). + const tabsRef = useRef(null) // The chip under the last right-click — the pane the zone menu's Split // actions carry into the new zone (header background = the active pane). // STATE, not a ref: the menu items (incl. Close's visibility) are JSX @@ -229,6 +233,15 @@ export function TreeGroup({ const verticalCollapse = Boolean(node.minimized) && parentAxis === 'row' && !isEmpty const headerVisible = !isEmpty && !verticalCollapse && (Boolean(node.minimized) || !headerHidden) + // Keep the activated tab — and, on the last one, the trailing "+" — inside + // the strip's scroll window. Opening a tab past the right edge otherwise + // left both the new tab and the button that made it out of view. + useActiveTabVisible(tabsRef, activeId, { + enabled: headerVisible, + last: shown[shown.length - 1] === activeId, + tabCount: shown.length + }) + // Drag handles preventDefault pointerdown (no native dblclick), so the // header + chips share a synthesized double-tap: restore if collapsed // (undoing the first tap's minimize toggle) and hide the chrome. @@ -426,6 +439,7 @@ export function TreeGroup({ >
{shown.map(paneId => {