let renaming a chat take the tab name back
`label ?? override ?? route` made a typed tab name permanent. That is right for navigation — you named the window to find it again — and wrong the moment you rename the conversation itself: the tab kept the old name, and kept it across reloads, because the stale one is in sessionStorage. The rename looked like it had failed. Both are deliberate acts, so the newer wins. The hard part is telling a rename from ordinary navigation: from the outside, "same conversation, new title" and "different conversation, different title" are the same event — a changed override. Clearing the tab name on any change would have wiped it every time you clicked a chat. So the override now carries the id of the thing it names. Same id with a new title is a rename and drops the tab name; a new id is navigation and leaves it alone. The alternative was to have the panel clear the label directly, which needs a QueryClient dragged across the workspace boundary the bridge exists to avoid — the shell owns the tab name, so the shell decides when to drop it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
|||||||
import { useCallback, useEffect } from 'react';
|
import { useCallback, useEffect, useRef } from 'react';
|
||||||
import { useLocation } from 'react-router';
|
import { useLocation } from 'react-router';
|
||||||
|
import type { PageTitleOverride } from 'officerdev';
|
||||||
import { usePageTitleOverride } from 'officerdev';
|
import { usePageTitleOverride } from 'officerdev';
|
||||||
import { useSessionState, writeSessionValue } from 'hooks/useSessionState';
|
import { useSessionState, writeSessionValue } from 'hooks/useSessionState';
|
||||||
|
|
||||||
@@ -163,9 +164,28 @@ export function usePageTitle() {
|
|||||||
|
|
||||||
useEffect(() => onTabLabelDropped(() => setLabel(null)), [setLabel]);
|
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<PageTitleOverride | null>(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]);
|
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;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -221,7 +221,9 @@ export const ChatDetailPanel = () => {
|
|||||||
const { sessions } = useClaudeSessions(selected?.cwd);
|
const { sessions } = useClaudeSessions(selected?.cwd);
|
||||||
const title = sessions.find((session) => session.id === sessionId)?.title ?? selected?.title ?? null;
|
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) {
|
if (!selected) {
|
||||||
return (
|
return (
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ export * from './hooks';
|
|||||||
export * from './channels';
|
export * from './channels';
|
||||||
// For the shell: a screen naming itself better than its route can — see usePageTitle's precedence.
|
// For the shell: a screen naming itself better than its route can — see usePageTitle's precedence.
|
||||||
export { usePageTitleOverride, usePublishPageTitle } from './page-title';
|
export { usePageTitleOverride, usePublishPageTitle } from './page-title';
|
||||||
|
export type { PageTitleOverride } from './page-title';
|
||||||
export * from './AppRegistry';
|
export * from './AppRegistry';
|
||||||
export * from './WidgetRegistry';
|
export * from './WidgetRegistry';
|
||||||
export * from './MusicPlayer';
|
export * from './MusicPlayer';
|
||||||
|
|||||||
@@ -16,8 +16,19 @@ import { useGlobal } from 'hooks/useGlobal';
|
|||||||
// stale copy would be a header naming a conversation that is no longer open.
|
// stale copy would be a header naming a conversation that is no longer open.
|
||||||
const PAGE_TITLE_OVERRIDE = 'PAGE_TITLE_OVERRIDE';
|
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. */
|
/** The override, or null. For the shell. */
|
||||||
export const usePageTitleOverride = () => useGlobal<string | null>(PAGE_TITLE_OVERRIDE, null)[0];
|
export const usePageTitleOverride = () => useGlobal<PageTitleOverride | null>(PAGE_TITLE_OVERRIDE, null)[0];
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Name the page from inside a screen. Pass `null` when there is nothing to say.
|
* Name the page from inside a screen. Pass `null` when there is nothing to say.
|
||||||
@@ -25,16 +36,20 @@ export const usePageTitleOverride = () => useGlobal<string | null>(PAGE_TITLE_OV
|
|||||||
* The cleanup is the load-bearing half: navigating away unmounts the publisher without anything setting
|
* 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.
|
* the title back, and a leftover value is a header still showing the last chat you had open.
|
||||||
*/
|
*/
|
||||||
export function usePublishPageTitle(title: string | null) {
|
export function usePublishPageTitle(override: PageTitleOverride | null) {
|
||||||
const [, setOverride] = useGlobal<string | null>(PAGE_TITLE_OVERRIDE, null);
|
const [, setOverride] = useGlobal<PageTitleOverride | null>(PAGE_TITLE_OVERRIDE, null);
|
||||||
|
|
||||||
// Through a ref: `useGlobal` rebuilds its setter every render, so as a dependency it would re-run the
|
// 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.
|
// effect — and therefore the cleanup's `null` — on every render of the publisher.
|
||||||
const setOverrideRef = useRef(setOverride);
|
const setOverrideRef = useRef(setOverride);
|
||||||
setOverrideRef.current = 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(() => {
|
useEffect(() => {
|
||||||
setOverrideRef.current(title);
|
setOverrideRef.current(id && title ? { id, title } : null);
|
||||||
return () => setOverrideRef.current(null);
|
return () => setOverrideRef.current(null);
|
||||||
}, [title]);
|
}, [id, title]);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user