Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 154 additions & 9 deletions packages/components/src/components/sessions/session-detail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,16 @@ import {
} from '@/components/terminal/terminal-controller';
import { isElectronRenderer } from '@/lib/electron';
import { sidebarCollapsedAtom } from '@/atoms/sidebar-state';
import { memo, useCallback, useEffect, useMemo, useRef, useState, type CSSProperties } from 'react';
import {
memo,
useCallback,
useEffect,
useMemo,
useRef,
useState,
type CSSProperties,
type ReactNode,
} from 'react';
import { useDocumentTitle } from '@/hooks/use-document-title';
import { useTabStatus, type TabStatus } from '@/hooks/use-tab-status';
import {
Expand Down Expand Up @@ -654,6 +663,25 @@ const TerminalDockToggleButton = memo(function TerminalDockToggleButton() {
);
});

/** Stable empty default, so the memos that read `surfaceTabs` do not see a new
* array identity on every render of a page that contributes none. */
const EMPTY_SURFACE_TABS: readonly SessionSurfaceTab[] = [];

/** One tab the HOST contributes to this session's tab strip.
*
* A host that embeds this page may own surfaces of its own that belong beside
* the conversation rather than in a second strip. The page draws the tab and
* lays the content out; the host owns the list, the selection and both verbs. */
export interface SessionSurfaceTab {
/** Unique across this strip. Must not collide with a session id. */
id: string;
label: string;
icon?: ReactNode;
/** Rendered as a peer of the chat surfaces: mounted always, hidden when
* another tab is active. */
content: ReactNode;
}

/**
* Session detail page component.
* Displays the chat interface for a single session.
Expand All @@ -664,12 +692,47 @@ const SessionDetail = ({
urlPrNumber,
urlBrowser,
onMobileBack,
surfaceTabs = EMPTY_SURFACE_TABS,
activeSurfaceTabId = null,
onSurfaceTabSelect,
onSurfaceTabClose,
onSessionTabSelect,
onSessionMissing,
}: {
sessionId: SessionId;
urlTab?: string;
urlPrNumber?: number;
urlBrowser?: boolean;
onMobileBack?: () => void;
/** Host-contributed tabs, drawn after the session tabs. Empty by default, so
* the strip and the surfaces are exactly what they were without them. */
surfaceTabs?: readonly SessionSurfaceTab[];
/** Which host tab is selected, or `null` when a session tab is. */
activeSurfaceTabId?: string | null;
onSurfaceTabSelect?: (tabId: string) => void;
onSurfaceTabClose?: (tabId: string) => void;
/**
* This page moved its own tab selection to a CONVERSATION tab.
*
* The host owns `activeSurfaceTabId` and this page owns the conversation
* selection, so a host tab stays selected — and keeps covering the
* conversation — until the host is told to drop it. Nothing else can tell it:
* the conversation selection is local state and it is not in the URL.
*
* Fired with the tab id the page selected, so a host that draws its own
* chrome can follow the selection rather than only clear its own.
*/
onSessionTabSelect?: (tabId: string) => void;
/**
* This page has no session to draw: the id it was given is not in the
* workspace, so it renders the not-found card and returns BEFORE the tab
* strip.
*
* A host that contributes tabs loses all of them at that return, including
* the one the member is looking at, so it has to be told: the strip is gone
* and whatever the host had selected in it needs another home.
*/
onSessionMissing?: (sessionId: string) => void;
}) => {
const { t } = useTranslation();
const router = useRouter();
Expand Down Expand Up @@ -753,9 +816,39 @@ const SessionDetail = ({
const [mobileFileViewerTabId, setMobileFileViewerTabId] = useState<string | null>(null);
const [mobileFileViewerOpen, setMobileFileViewerOpen] = useState(false);
const mobileFilesBrowserRef = useRef<MobileProjectFileBrowserHandle>(null);
const [activeTabSessionIdRaw, setActiveTabSessionId] = useState<string>(
const [activeTabSessionIdRaw, setActiveTabSessionIdState] = useState<string>(
() => initialTabState.activeTabSessionId
);
const onSessionTabSelectRef = useRef(onSessionTabSelect);
onSessionTabSelectRef.current = onSessionTabSelect;
// Seam patch 5 hunk 20 reads this from an effect whose dependency list is
// upstream's; a ref keeps a fresh host closure from re-running that effect.
const onSessionMissingRef = useRef(onSessionMissing);
onSessionMissingRef.current = onSessionMissing;
/**
* THE ONE PLACE THIS PAGE SELECTS A CONVERSATION TAB (seam patch 5 hunk 17).
*
* Ten call sites move `activeTabSessionIdRaw` — the strip click, the `+`, a
* close, a restore, a fork, a mention navigation, the next/previous cycle, a
* promoted draft, the browser panel, and the URL sync — and a host that
* contributed tabs has to hear about every one of them: an active host tab
* hides all the conversation surfaces, so a selection the host never learns
* about leaves its tab covering the conversation the user just asked for.
*
* A wrapper rather than a notification at each site, and rather than an
* effect on the value: an effect would only fire on a CHANGE, and the click
* that most needs the call does not change anything — the parent tab is
* already `activeTabSessionId` while a host tab covers it.
*
* The three writers that do NOT come through here take the raw setter, and
* each is a CORRECTION rather than a selection: the session-switch reset
* (which runs during render, where a host callback may not), and the two URL
* syncs (which re-assert what `?tab=` already says).
*/
const setActiveTabSessionId = useCallback((tabId: string) => {
setActiveTabSessionIdState(tabId);
onSessionTabSelectRef.current?.(tabId);
}, []);
const [localStateSessionId, setLocalStateSessionId] = useState(sessionId);
const [commentReferenceKeysBySession, setCommentReferenceKeysBySession] = useState<
Record<string, string[]>
Expand Down Expand Up @@ -944,7 +1037,9 @@ const SessionDetail = ({
setMobileDiffState(null);
setMobileFilesBrowserOpen(false);
setFileProviderRequestedByInteraction(false);
setActiveTabSessionId(nextInitialTabState.activeTabSessionId);
// The raw setter: this runs during RENDER, where a host callback may not,
// and a session switch is the host's own navigation anyway.
setActiveTabSessionIdState(nextInitialTabState.activeTabSessionId);
}

const setDraftTabs = useCallback(
Expand Down Expand Up @@ -2582,13 +2677,16 @@ const SessionDetail = ({
setActiveViewerTabId(null);
}
if (urlSyncAction.kind === 'activate-session') {
setActiveTabSessionId((prev) =>
// The raw setter: `?tab=` is a correction of a selection that already
// happened, not a new one, and it fires whenever the parsed value's
// identity changes.
setActiveTabSessionIdState((prev) =>
prev === urlSyncAction.sessionId ? prev : urlSyncAction.sessionId
);
return;
}

setActiveTabSessionId((prev) => (prev === sessionId ? prev : sessionId));
setActiveTabSessionIdState((prev) => (prev === sessionId ? prev : sessionId));
}, [isMobile, parsedUrlTab, sessionId]);

const resolveDiffFilePaths = useCallback(
Expand Down Expand Up @@ -3386,8 +3484,25 @@ const SessionDetail = ({
return openedSidebarTabs.filter((tabId) => availableTabIds.has(tabId));
}, [openedSidebarTabs, sidePanelFixedOptions]);

// Host tabs, in the shape the strip draws a non-session tab in. Memoized on
// the host's own list, so a page that contributes none hands `SessionTabBar`
// the same empty array on every render and its `memo` still holds.
const surfaceTabItems = useMemo<ViewerTabItem[]>(
() =>
surfaceTabs.map((tab) => ({
id: tab.id,
type: 'custom' as const,
label: tab.label,
icon: tab.icon,
})),
[surfaceTabs]
);

// Shared viewer metadata powers the mobile switcher and desktop side-panel tabs.
const viewerTabItems: ViewerTabItem[] = useMemo(
// Narrowed to this page's own viewer kinds: the side panel and the mobile
// switcher have no row for a host tab, and host tabs travel in
// `surfaceTabItems` instead, which goes to the tab strip alone.
const viewerTabItems: (ViewerTabItem & { type: 'file' | 'diff' })[] = useMemo(
() =>
viewerTabs.map((tab) => ({
id: tab.id,
Expand Down Expand Up @@ -4304,6 +4419,11 @@ const SessionDetail = ({
}

if (sessionPresenceState === 'not-found') {
// Seam patch 5 hunk 20. ABOVE the once-per-session analytics gate on
// purpose: the host has to hear this every time the page settles on
// not-found, because what it does with it is move an address, and an
// address can come back.
onSessionMissingRef.current?.(sessionId);
if (!fireDetailNotFoundOnce(sessionId)) {
return;
}
Expand Down Expand Up @@ -5502,14 +5622,18 @@ const SessionDetail = ({
lives in the context strip above the composer and in the "…" menu. */
const tabBar = (
<SessionTabBar
variant="session"
variant={surfaceTabs.length > 0 ? 'mixed' : 'session'}
parentSession={activeSession}
childSessions={visibleChildSessions}
draftTabs={draftTabs}
tabOrder={sessionTabOrder}
activeTabSessionId={activeTabSessionId}
onTabSelect={handleSessionTabSelect}
onNewTab={handleNewTab}
viewerTabs={surfaceTabItems}
activeViewerTabId={activeSurfaceTabId}
onViewerTabSelect={onSurfaceTabSelect}
onViewerTabClose={onSurfaceTabClose}
onTabRename={handleTabRename}
onTabClose={handleTabClose}
archivedChildSessions={archivedChildSessions}
Expand Down Expand Up @@ -5577,14 +5701,22 @@ const SessionDetail = ({
};
};

// An active HOST tab deselects every conversation surface, the same rule
// `hasActiveViewerTab` applies to the strip. Null when the host contributes
// none, so the surfaces below read exactly what they read before.
const activeChatSurfaceId =
activeSurfaceTabId !== null && surfaceTabs.some((tab) => tab.id === activeSurfaceTabId)
? null
: activeTabSessionId;

const desktopChatSurfaces = (
<SessionMentionDropLayer
enabled
excludeSessionId={sessionMentionExcludeId}
onDropSessionId={handleInsertDroppedSessionMention}
>
{[activeSession, ...visibleChildSessions].map((tabSession) => {
const isActive = tabSession.id === activeTabSessionId;
const isActive = tabSession.id === activeChatSurfaceId;
const externalHistoryRefresh = externalHistoryRefreshBySessionId[tabSession.id];
const externalHistoryProviderLabel = externalHistoryRefresh
? getExternalHistoryProviderLabel(externalHistoryRefresh.provider)
Expand Down Expand Up @@ -5618,7 +5750,7 @@ const SessionDetail = ({
);
})}
{draftTabs.map((draft) => {
const isActive = draft.id === activeTabSessionId;
const isActive = draft.id === activeChatSurfaceId;
return (
<div
key={draft.id}
Expand All @@ -5637,6 +5769,19 @@ const SessionDetail = ({
</div>
);
})}
{surfaceTabs.map((tab) => {
const isActive = tab.id === activeSurfaceTabId;
return (
<div
key={tab.id}
className={cn('absolute inset-0', !isActive && 'hidden')}
aria-hidden={!isActive}
data-surface-tab-id={tab.id}
>
{tab.content}
</div>
);
})}
</SessionMentionDropLayer>
);

Expand Down
29 changes: 20 additions & 9 deletions packages/components/src/components/sessions/session-tab-bar.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { memo, useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from 'react';
import { Plus, Loader2, X, History, Undo2, Pin, FileDiff } from 'lucide-react';
import { cn } from '@/lib/utils';
import { getSessionLaunchConfigLegacyFields, type SessionId, type SessionMeta } from '@lody/shared';
Expand Down Expand Up @@ -39,11 +39,18 @@ import {
startSessionMentionDrag,
} from '@/lib/session-mention-drag';

/** A viewer tab item (file or diff) displayed in the tab bar. */
/** A viewer tab item (file or diff) displayed in the tab bar.
*
* `custom` is for a tab a HOST contributes: the bar draws and sorts it exactly
* like a file or a diff tab, and the host supplies both the glyph ({@link icon})
* and the content, so the bar needs to know nothing about what is behind it. */
export interface ViewerTabItem {
id: string;
type: 'file' | 'diff';
type: 'file' | 'diff' | 'custom';
label: string;
/** Drawn in place of the file/diff glyph. Required in practice for a
* `custom` tab, which has no file path to derive an icon from. */
icon?: ReactNode;
filePath?: string;
dirty?: boolean;
saving?: boolean;
Expand All @@ -55,7 +62,10 @@ type MaybePromiseVoid = void | Promise<void>;

interface SessionTabBarProps {
variant?: SessionTabBarVariant;
parentSession: SessionMeta;
/** The session the strip is rooted in. Optional because `variant="viewer"`
* draws no session tabs at all, so a host using that variant has no session
* to name — requiring it is what stops the declared variant being usable. */
parentSession?: SessionMeta;
childSessions: SessionMeta[];
draftTabs: DraftSessionTab[];
archivedChildSessions: SessionMeta[];
Expand Down Expand Up @@ -463,11 +473,11 @@ function ViewerTabContent({
}}
>
<span className="shrink-0">
{tab.type === 'file' && tab.filePath ? (
{tab.icon ?? (tab.type === 'file' && tab.filePath ? (
<FileIcon filePath={tab.filePath} className="h-3 w-3" />
) : (
<FileDiff className="h-3 w-3 opacity-60" />
)}
))}
</span>
{saveStateLabel ? (
<span
Expand Down Expand Up @@ -723,8 +733,9 @@ export const SessionTabBar = memo(function SessionTabBar({
// In mixed mode, an active viewer tab deselects the session tabs.
const hasActiveViewerTab = variant === 'mixed' && !!activeViewerTabId;
const visibleTabIds = useMemo(
() => (showSessionTabs ? [parentSession.id, ...sortableIds] : sortableIds),
[parentSession.id, showSessionTabs, sortableIds]
() =>
showSessionTabs && parentSession ? [parentSession.id, ...sortableIds] : sortableIds,
[parentSession, showSessionTabs, sortableIds]
);
const activeTabId =
showViewerTabs && activeViewerTabId
Expand Down Expand Up @@ -762,7 +773,7 @@ export const SessionTabBar = memo(function SessionTabBar({
paddingLeft={variant === 'session' ? 4 : 8}
paddingRight={8}
>
{showSessionTabs && (
{showSessionTabs && parentSession && (
<AdaptiveTabStripItem itemId={parentSession.id}>
<TabContent
session={parentSession}
Expand Down