diff --git a/src/apps/officer-web/state/usePageTitle.ts b/src/apps/officer-web/state/usePageTitle.ts index c8a520f0..3693145e 100644 --- a/src/apps/officer-web/state/usePageTitle.ts +++ b/src/apps/officer-web/state/usePageTitle.ts @@ -1,5 +1,6 @@ -import { useCallback, useEffect } from 'react'; +import { useCallback, useEffect, useRef } from 'react'; import { useLocation } from 'react-router'; +import type { PageTitleOverride } from 'officerdev'; import { usePageTitleOverride } from 'officerdev'; import { useSessionState, writeSessionValue } from 'hooks/useSessionState'; @@ -163,9 +164,28 @@ export function usePageTitle() { useEffect(() => onTabLabelDropped(() => setLabel(null)), [setLabel]); + /** + * A rename outranks a tab name you typed earlier; opening a different chat does not. + * + * The precedence below makes a typed name permanent, which is right for navigation — you named this + * window to find it again — and wrong the moment you rename the conversation itself. That was a + * deliberate act on the same thing the tab is showing, and it appeared to do nothing: the tab kept the + * old name, and kept it across reloads, because the stale one is in sessionStorage. + * + * The two are told apart by the id, which is why the override carries one. Same id and a new title is + * a rename, and the newer act wins. A new id is navigation, and the tab name survives it. + */ + const seenRef = useRef(null); + useEffect(() => { + const seen = seenRef.current; + seenRef.current = override; + if (!override || !seen) return; + if (seen.id === override.id && seen.title !== override.title) setLabel(null); + }, [override, setLabel]); + const rename = useCallback((next: string) => setLabel(next.trim() || null), [setLabel]); - return [label ?? override ?? titleForPath(pathname), rename] as const; + return [label ?? override?.title ?? titleForPath(pathname), rename] as const; } /** diff --git a/src/workspaces/officerdev/src/apps/ChatHistory/ChatDetailPanel.tsx b/src/workspaces/officerdev/src/apps/ChatHistory/ChatDetailPanel.tsx index 5227efa9..791257fc 100644 --- a/src/workspaces/officerdev/src/apps/ChatHistory/ChatDetailPanel.tsx +++ b/src/workspaces/officerdev/src/apps/ChatHistory/ChatDetailPanel.tsx @@ -221,7 +221,9 @@ export const ChatDetailPanel = () => { const { sessions } = useClaudeSessions(selected?.cwd); const title = sessions.find((session) => session.id === sessionId)?.title ?? selected?.title ?? null; - usePublishPageTitle(sessionId ? title : null); + // The id rides along so the shell can tell a rename of this conversation from opening a different one + // — only the first should override a tab name typed earlier. See usePageTitle. + usePublishPageTitle(sessionId && title ? { id: sessionId, title } : null); if (!selected) { return ( diff --git a/src/workspaces/officerdev/src/index.ts b/src/workspaces/officerdev/src/index.ts index 502f88cd..c3b207a2 100644 --- a/src/workspaces/officerdev/src/index.ts +++ b/src/workspaces/officerdev/src/index.ts @@ -2,6 +2,7 @@ export * from './hooks'; export * from './channels'; // For the shell: a screen naming itself better than its route can — see usePageTitle's precedence. export { usePageTitleOverride, usePublishPageTitle } from './page-title'; +export type { PageTitleOverride } from './page-title'; export * from './AppRegistry'; export * from './WidgetRegistry'; export * from './MusicPlayer'; diff --git a/src/workspaces/officerdev/src/page-title.ts b/src/workspaces/officerdev/src/page-title.ts index 827e4a51..aeafb2eb 100644 --- a/src/workspaces/officerdev/src/page-title.ts +++ b/src/workspaces/officerdev/src/page-title.ts @@ -16,8 +16,19 @@ import { useGlobal } from 'hooks/useGlobal'; // stale copy would be a header naming a conversation that is no longer open. const PAGE_TITLE_OVERRIDE = 'PAGE_TITLE_OVERRIDE'; +/** + * What the screen wants the page called, and which thing it is naming. + * + * The `id` is not decoration. The shell has to tell two changes apart that look identical from the + * outside — you opened a DIFFERENT conversation (new id, new title) versus you RENAMED the one you are + * in (same id, new title) — because only the second should override a tab name you typed earlier. With + * the title alone the shell would have to treat navigation as a rename and drop the tab name every time + * you clicked a chat. + */ +export type PageTitleOverride = { id: string; title: string }; + /** The override, or null. For the shell. */ -export const usePageTitleOverride = () => useGlobal(PAGE_TITLE_OVERRIDE, null)[0]; +export const usePageTitleOverride = () => useGlobal(PAGE_TITLE_OVERRIDE, null)[0]; /** * Name the page from inside a screen. Pass `null` when there is nothing to say. @@ -25,16 +36,20 @@ export const usePageTitleOverride = () => useGlobal(PAGE_TITLE_OV * The cleanup is the load-bearing half: navigating away unmounts the publisher without anything setting * the title back, and a leftover value is a header still showing the last chat you had open. */ -export function usePublishPageTitle(title: string | null) { - const [, setOverride] = useGlobal(PAGE_TITLE_OVERRIDE, null); +export function usePublishPageTitle(override: PageTitleOverride | null) { + const [, setOverride] = useGlobal(PAGE_TITLE_OVERRIDE, null); // Through a ref: `useGlobal` rebuilds its setter every render, so as a dependency it would re-run the // effect — and therefore the cleanup's `null` — on every render of the publisher. const setOverrideRef = useRef(setOverride); setOverrideRef.current = setOverride; + // Depend on the fields, not the object: callers build a fresh literal every render, so an object + // dependency would re-run this — and its cleanup null — on every render of the publisher. + const id = override?.id ?? null; + const title = override?.title ?? null; useEffect(() => { - setOverrideRef.current(title); + setOverrideRef.current(id && title ? { id, title } : null); return () => setOverrideRef.current(null); - }, [title]); + }, [id, title]); }