diff --git a/products/desktop/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx b/products/desktop/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx index 06b542f4ea18..97408b998029 100644 --- a/products/desktop/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx +++ b/products/desktop/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx @@ -51,7 +51,7 @@ import { useRouter, useRouterState, } from "@tanstack/react-router"; -import { useCallback, useEffect, useMemo } from "react"; +import { memo, useCallback, useEffect, useMemo } from "react"; import { useHotkeys } from "react-hotkeys-hook"; import { shouldHandleBrowserTabSwitch } from "./browserTabShortcuts"; import { @@ -126,7 +126,7 @@ type TabRef = { appView: string | null; }; -export function BrowserTabStrip() { +function BrowserTabStripImpl() { const spacesLayout = useChannelsLayout(); const snapshot = useTabsSnapshot(); const navigate = useNavigate(); @@ -381,16 +381,22 @@ export function BrowserTabStrip() { href: locationHref, ...(railPane === "spaces" ? { listOpen, spaceId: stampedSpaceId } : {}), }; + const previousLastByPane = mirrorActive?.viewState?.lastByPane ?? {}; const viewState: TabViewState = { // Keep the stored name when nothing has resolved yet, so a loading frame // does not blank a background tab's label. title: routeTitle ?? mirrorActive?.viewState?.title, listOpen, spaceId: stampedSpaceId, - lastByPane: { - ...(mirrorActive?.viewState?.lastByPane ?? {}), - [railPane]: visit, - }, + // Settings is a full-window overlay that classifies as the spaces pane, so + // recording its href here would overwrite the tab's real last spaces + // location and a later Spaces rail click would reopen Settings. Keep the + // existing map on the settings route, as the strip did before settings + // stayed mounted. + lastByPane: + routeAppView === "settings" + ? previousLastByPane + : { ...previousLastByPane, [railPane]: visit }, }; const decision = decideTabNavigation({ // The SETTLED tag, not the in-flight one. Pairing the in-flight tag with @@ -944,3 +950,6 @@ export function BrowserTabStrip() { /> ); } + +// The root layout re-renders on every navigation; this keeps that from cascading here. +export const BrowserTabStrip = memo(BrowserTabStripImpl); diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelItemRow.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelItemRow.tsx index 8ed1f5109253..e01ae0ffd572 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelItemRow.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelItemRow.tsx @@ -44,6 +44,7 @@ import { SidebarItem } from "@posthog/ui/features/sidebar/components/SidebarItem import { writeTaskDragData } from "@posthog/ui/features/sidebar/taskDrag"; import { SESSION_ROW_ATTRIBUTE } from "@posthog/ui/features/sidebar/useMarqueeSelection"; import { HandoffTaskDialog } from "@posthog/ui/features/task-detail/components/HandoffTaskDialog"; +import { useMountedOnceOpened } from "@posthog/ui/hooks/useMountedOnceOpened"; import { type DragEvent, type ReactNode, @@ -296,6 +297,7 @@ export function ChannelItemRow({ const subtitle = useChannelItemMetadata(item); const [confirmDeleteOpen, setConfirmDeleteOpen] = useState(false); const [handoffOpen, setHandoffOpen] = useState(false); + const handoffMounted = useMountedOnceOpened(handoffOpen); const currentUser = useCurrentUser(); const canHandoff = item.kind === "task" && @@ -454,7 +456,7 @@ export function ChannelItemRow({ - {canHandoff && item.task ? ( + {canHandoff && item.task && handoffMounted ? ( s.location.pathname }); + // Only paths inside this space matter here. Collapsing the rest to "" keeps + // every row still while the user is elsewhere (settings, another space). + const pathname = useRouterState({ + select: (s) => + s.location.pathname.startsWith(`/spaces/${channelId}`) + ? s.location.pathname + : "", + }); const loopsEnabled = useFeatureFlag(LOOPS_FLAG); const { items, actions, me, isLoading, channelMissing } = diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.test.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.test.tsx index c09ae25d8b7d..5a8b8bb60a28 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.test.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.test.tsx @@ -145,7 +145,11 @@ vi.mock("@posthog/ui/features/canvas/components/RenameChannelModal", () => ({ })); vi.mock("@tanstack/react-router", () => ({ useNavigate: () => mocks.navigate, - useRouterState: () => "/spaces", + useRouterState: ({ + select, + }: { + select: (s: { location: { pathname: string } }) => unknown; + }) => select({ location: { pathname: "/spaces" } }), })); import { diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.tsx index 9af86ae77da6..775d64514245 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.tsx @@ -110,6 +110,7 @@ import { } from "@posthog/ui/features/sidebar/components/items/taskStatusVocabulary"; import { useSidebarStore } from "@posthog/ui/features/sidebar/sidebarStore"; import { HandoffTaskDialog } from "@posthog/ui/features/task-detail/components/HandoffTaskDialog"; +import { useMountedOnceOpened } from "@posthog/ui/hooks/useMountedOnceOpened"; import { OverflowTickerText, useOverflowTickerReveal, @@ -436,12 +437,13 @@ const SpaceTaskRow = memo(function SpaceTaskRow({ spaceId: string; asOption: boolean; }) { - const pathname = useRouterState({ select: (s) => s.location.pathname }); + const isActive = useRouterState({ + select: (s) => s.location.pathname.endsWith(`/tasks/${item.id}`), + }); const openTask = useOpenSpaceTask(); // No PR lookup here: that is a host round trip per row, and the tree can show // a dozen spaces' worth of rows at once. const status = useChannelTaskStatus(item, { withPrStatus: false }); - const isActive = pathname.endsWith(`/tasks/${item.id}`); const actions = useSpaceTaskActionsContext(); // A boolean rather than the value itself, so a keypress re-renders only the // two rows whose answer changed. @@ -450,6 +452,8 @@ const SpaceTaskRow = memo(function SpaceTaskRow({ ); const [handoffOpen, setHandoffOpen] = useState(false); + + const handoffMounted = useMountedOnceOpened(handoffOpen); // Only the owner may hand a task off; the API 404s it for anyone else. const currentUser = useCurrentUser(); const canHandoff = @@ -529,7 +533,7 @@ const SpaceTaskRow = memo(function SpaceTaskRow({ > {row} - {canHandoff && item.task ? ( + {canHandoff && item.task && handoffMounted ? ( s.location.pathname }); + const insideChannel = useRouterState({ + select: (s) => s.location.pathname.startsWith(`/spaces/${channel.id}`), + }); const { deleteChannel, isDeleting } = useChannelMutations(); const { isStarred, toggleStar } = useChannelStarToggle(channel); @@ -751,7 +757,7 @@ function useChannelActions(channel: Channel): { success: true, }); // If we're inside the channel being deleted, fall back to the index. - if (pathname.startsWith(`/spaces/${channel.id}`)) { + if (insideChannel) { void navigate({ to: "/spaces" }); } return true; @@ -948,11 +954,14 @@ const ChannelSection = memo( }) { const spacesLayout = useChannelsLayout(); const noun = spacesLayout ? "space" : "channel"; - const pathname = useRouterState({ select: (s) => s.location.pathname }); - const openChannel = useOpenChannel(); const base = `/spaces/${channel.id}`; // Highlight the row whenever any of the channel's routes is open. - const isActive = pathname === base || pathname.startsWith(`${base}/`); + const isActive = useRouterState({ + select: (s) => + s.location.pathname === base || + s.location.pathname.startsWith(`${base}/`), + }); + const openChannel = useOpenChannel(); // Lifted so the hover button group stays visible while the menu is open. const [menuOpen, setMenuOpen] = useState(false); const { reveal, hoverProps, focusProps } = useOverflowTickerReveal(); @@ -973,6 +982,7 @@ const ChannelSection = memo( confirmDelete, isDeleting, } = useChannelActions(channel); + const renameMounted = useMountedOnceOpened(renameOpen); const newTask = () => { track(ANALYTICS_EVENTS.CHANNEL_ACTION, { @@ -1174,11 +1184,13 @@ const ChannelSection = memo( {/* One modal for both the dropdown and context-menu "Rename" actions. */} - + {renameMounted && ( + + )} {/* Destructive confirm for "Delete channel" — spells out what's removed. */} void; }) { const spacesLayout = useChannelsLayout(); - const pathname = useRouterState({ select: (s) => s.location.pathname }); const { channels } = useChannels(); const { ensureChannelId, openPersonalChannel } = useOpenPersonalChannel(); @@ -1358,10 +1369,12 @@ const PersonalChannelRow = memo(function PersonalChannelRow({ const isUnread = useIsChannelUnread()(meChannel?.id); const unreadSessions = useUnreadSessionCount()(meChannel?.id); const blockedSessions = useBlockedSessionCount()(meChannel?.id); - const isActive = - !!meChannel && - (pathname === `/spaces/${meChannel.id}` || - pathname.startsWith(`/spaces/${meChannel.id}/`)); + const isActive = useRouterState({ + select: (s) => + !!meChannel && + (s.location.pathname === `/spaces/${meChannel.id}` || + s.location.pathname.startsWith(`/spaces/${meChannel.id}/`)), + }); const newTask = () => { const channelId = ensureChannelId(); diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.test.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.test.tsx index 890a1394c832..ae02584130f8 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.test.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.test.tsx @@ -85,18 +85,28 @@ vi.mock("@posthog/ui/features/workspace/useWorkspace", () => ({ useWorkspaces: () => ({ data: {}, isFetched: true }), })); vi.mock("@tanstack/react-router", () => ({ - useParams: () => ({ channelId: mocks.routeChannelId }), + useParams: ({ + select, + }: { + select?: (params: { channelId: string | undefined }) => unknown; + } = {}) => { + const params = { channelId: mocks.routeChannelId }; + return select ? select(params) : params; + }, useRouterState: ({ select, }: { select: (s: { matches: { fullPath: string }[]; - location: { state: { tabId?: string } }; + location: { pathname: string; state: { tabId?: string } }; }) => unknown; }) => select({ matches: [{ fullPath: mocks.fullPath }], - location: { state: { tabId: mocks.historyTabId } }, + location: { + pathname: mocks.fullPath, + state: { tabId: mocks.historyTabId }, + }, }), })); diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx index 9101154b25f6..6a62ea82fa9f 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx @@ -51,7 +51,7 @@ import { useSidebarEdgeHoverPeek } from "@posthog/ui/primitives/hooks/useSidebar import { ResizableSidebar } from "@posthog/ui/primitives/ResizableSidebar"; import { navigateToArchived } from "@posthog/ui/router/navigationBridge"; import { useParams } from "@tanstack/react-router"; -import { useDeferredValue, useEffect, useRef } from "react"; +import { memo, useDeferredValue, useEffect, useRef } from "react"; /** * The sidebar slider: the channel list and the channel you're in, laid out side @@ -132,7 +132,7 @@ function ChannelPanes({ ); } -export function ChannelsSidebar() { +function ChannelsSidebarImpl() { const width = useChannelsSidebarStore((state) => state.width); const setWidth = useChannelsSidebarStore((state) => state.setWidth); const isResizing = useChannelsSidebarStore((state) => state.isResizing); @@ -204,7 +204,10 @@ export function ChannelsSidebar() { // slide to there's only the list. const { pane: railPane, showsActivityDetail } = useRailSurface(); const selectedActivityId = useActivitySelection()?.id; - const { feedId } = useParams({ strict: false }); + const feedId = useParams({ + strict: false, + select: (params) => params.feedId, + }); const pane = useChannelPaneStore((s) => s.pane); const { isPending: pendingTabSwitch, viewState: pendingTabViewState } = usePendingTabViewState(); @@ -315,3 +318,6 @@ export function ChannelsSidebar() { ); } + +// The root layout re-renders on every navigation; this keeps that from cascading here. +export const ChannelsSidebar = memo(ChannelsSidebarImpl); diff --git a/products/desktop/packages/ui/src/features/canvas/components/NavRail.tsx b/products/desktop/packages/ui/src/features/canvas/components/NavRail.tsx index 5cdb3b3b66a6..f026c7e0853d 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/NavRail.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/NavRail.tsx @@ -42,6 +42,7 @@ import { useCommandMenuStore } from "@posthog/ui/shell/commandMenuStore"; import { type ComponentPropsWithRef, type MouseEventHandler, + memo, type ReactElement, type ReactNode, useState, @@ -178,7 +179,7 @@ function ActivityNavItem({ * The app's leftmost column. Sits outside the resizable sidebar, so collapsing * that sidebar leaves the destinations reachable. */ -export function NavRail() { +function NavRailImpl() { const homeEnabled = useFeatureFlag(DESKTOP_HOME_FLAG); const loopsEnabled = useFeatureFlag(LOOPS_FLAG); const contextEnabled = useContextLayerFlag(); @@ -322,3 +323,6 @@ export function NavRail() { ); } + +// The root layout re-renders on every navigation; this keeps that from cascading here. +export const NavRail = memo(NavRailImpl); diff --git a/products/desktop/packages/ui/src/features/canvas/components/ShellLayout.test.tsx b/products/desktop/packages/ui/src/features/canvas/components/ShellLayout.test.tsx index aaa6de48ed1a..fac2b20a3575 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ShellLayout.test.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ShellLayout.test.tsx @@ -35,7 +35,12 @@ const { vi.mock("@tanstack/react-router", () => ({ Outlet: () => null, useNavigate: () => vi.fn(), - useParams, + useParams: (opts?: { + select?: (p: Record) => unknown; + }) => { + const params = useParams(); + return opts?.select ? opts.select(params) : params; + }, useRouterState: ({ select, }: { diff --git a/products/desktop/packages/ui/src/features/canvas/components/ShellLayout.tsx b/products/desktop/packages/ui/src/features/canvas/components/ShellLayout.tsx index 34b954fbf731..553c9d0c0f2e 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ShellLayout.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ShellLayout.tsx @@ -347,12 +347,20 @@ function CanvasBreadcrumb({ export function ShellLayout() { const spacesLayout = useChannelsLayout(); - const pathname = useRouterState({ select: (s) => s.location.pathname }); + const pathname = useRouterState({ + select: (s) => + s.location.pathname.startsWith("/spaces/") ? s.location.pathname : "", + }); const selectedCanvasId = useSelectedCanvasId(); - const params = useParams({ strict: false }); - - const channelId = params.channelId; - const dashboardId = params.dashboardId; + // Select each param on its own so an unrelated route param (a settings + // category) changing cannot re-render the shell. `useParams` without a + // selector subscribes to the whole param set, which the nearest match carries + // for the entire route chain. + const channelId = useParams({ strict: false, select: (p) => p.channelId }); + const dashboardId = useParams({ + strict: false, + select: (p) => p.dashboardId, + }); const { dashboard: selectedCanvas } = useDashboard(selectedCanvasId); const toolbarDashboardId = dashboardId ?? selectedCanvasId; const toolbarChannelId = channelId ?? selectedCanvas?.channelId; diff --git a/products/desktop/packages/ui/src/features/navigation/components/RightPanel.tsx b/products/desktop/packages/ui/src/features/navigation/components/RightPanel.tsx index 4327d6853120..a52bff636ad1 100644 --- a/products/desktop/packages/ui/src/features/navigation/components/RightPanel.tsx +++ b/products/desktop/packages/ui/src/features/navigation/components/RightPanel.tsx @@ -218,7 +218,7 @@ function useDrawnSide( } /** Every side belongs to a session, so nothing below this runs elsewhere. */ -export function RightPanel() { +function RightPanelImpl() { const { taskId } = useActiveSession(); // Keyed by session: carrying per-session state across a navigation draws the // previous session's panel over the new one for a frame. @@ -319,3 +319,6 @@ function SessionRightPanel({ taskId }: { taskId: string }) { ); } + +// The root layout re-renders on every navigation; this keeps that from cascading here. +export const RightPanel = memo(RightPanelImpl); diff --git a/products/desktop/packages/ui/src/features/navigation/useActiveSession.ts b/products/desktop/packages/ui/src/features/navigation/useActiveSession.ts index 0d2d5efe6fc4..55078881ad3a 100644 --- a/products/desktop/packages/ui/src/features/navigation/useActiveSession.ts +++ b/products/desktop/packages/ui/src/features/navigation/useActiveSession.ts @@ -16,7 +16,13 @@ export function useActiveSession(): ActiveSession { const { showsActivityDetail } = useRailSurface(); const selected = useActivitySelection(); const feedSelected = useTaskFeedSelectionStore((s) => s.selected); - const params = useParams({ strict: false }); + // Select each param on its own. `useParams` without a selector subscribes to + // the whole param set the nearest match carries for the route chain, so an + // unrelated param (a settings category) changing would re-render every + // consumer of this hook. + const taskId = useParams({ strict: false, select: (p) => p.taskId }); + const channelId = useParams({ strict: false, select: (p) => p.channelId }); + const feedId = useParams({ strict: false, select: (p) => p.feedId }); if (showsActivityDetail) { const taskSelection = selected?.kind === "task" ? selected : null; @@ -25,11 +31,11 @@ export function useActiveSession(): ActiveSession { channelId: taskSelection?.channelId ?? undefined, }; } - if (params.feedId && feedSelected?.feedId === params.feedId) { + if (feedId && feedSelected?.feedId === feedId) { return { taskId: feedSelected.taskId, channelId: feedSelected.channelId ?? undefined, }; } - return { taskId: params.taskId, channelId: params.channelId }; + return { taskId, channelId }; } diff --git a/products/desktop/packages/ui/src/features/navigation/useReviewInRightPanel.test.ts b/products/desktop/packages/ui/src/features/navigation/useReviewInRightPanel.test.ts index 9ced22f44821..d8e98b860309 100644 --- a/products/desktop/packages/ui/src/features/navigation/useReviewInRightPanel.test.ts +++ b/products/desktop/packages/ui/src/features/navigation/useReviewInRightPanel.test.ts @@ -13,7 +13,13 @@ vi.mock("@posthog/ui/features/canvas/hooks/useChannelsLayout", () => ({ useChannelsLayout: () => mocks.channelsLayout, })); vi.mock("@tanstack/react-router", () => ({ - useParams: () => mocks.routeParams, + useParams: (opts?: { + select?: (p: { + taskId?: string; + channelId?: string; + feedId?: string; + }) => unknown; + }) => (opts?.select ? opts.select(mocks.routeParams) : mocks.routeParams), useRouterState: ({ select, }: { diff --git a/products/desktop/packages/ui/src/features/panels/components/TabbedPanel.tsx b/products/desktop/packages/ui/src/features/panels/components/TabbedPanel.tsx index 4d714aae53b8..e2ba4d8208ce 100644 --- a/products/desktop/packages/ui/src/features/panels/components/TabbedPanel.tsx +++ b/products/desktop/packages/ui/src/features/panels/components/TabbedPanel.tsx @@ -180,33 +180,38 @@ export const TabbedPanel: React.FC = ({ ); useEffect(() => { - if (!scrollContainerRef.current || !content.activeTabId) return; + // Measured after paint: reading rects inside the commit forced a layout + // of the freshly mounted tab. + const frame = requestAnimationFrame(() => { + if (!scrollContainerRef.current || !content.activeTabId) return; - const activeTabIndex = content.tabs.findIndex( - (tab) => tab.id === content.activeTabId, - ); - if (activeTabIndex === -1) return; + const activeTabIndex = content.tabs.findIndex( + (tab) => tab.id === content.activeTabId, + ); + if (activeTabIndex === -1) return; - const container = scrollContainerRef.current; - const tabElement = container.children[activeTabIndex] as HTMLElement; - if (!tabElement) return; + const container = scrollContainerRef.current; + const tabElement = container.children[activeTabIndex] as HTMLElement; + if (!tabElement) return; - const containerRect = container.getBoundingClientRect(); - const tabRect = tabElement.getBoundingClientRect(); + const containerRect = container.getBoundingClientRect(); + const tabRect = tabElement.getBoundingClientRect(); - if (tabRect.right > containerRect.right - 64) { - tabElement.scrollIntoView({ - behavior: "smooth", - block: "nearest", - inline: "end", - }); - } else if (tabRect.left < containerRect.left) { - tabElement.scrollIntoView({ - behavior: "smooth", - block: "nearest", - inline: "start", - }); - } + if (tabRect.right > containerRect.right - 64) { + tabElement.scrollIntoView({ + behavior: "smooth", + block: "nearest", + inline: "end", + }); + } else if (tabRect.left < containerRect.left) { + tabElement.scrollIntoView({ + behavior: "smooth", + block: "nearest", + inline: "start", + }); + } + }); + return () => cancelAnimationFrame(frame); }, [content.activeTabId, content.tabs]); return ( diff --git a/products/desktop/packages/ui/src/features/sessions/components/chat-thread/ChatThread.tsx b/products/desktop/packages/ui/src/features/sessions/components/chat-thread/ChatThread.tsx index f9ffa07c6b5e..d85d579eab03 100644 --- a/products/desktop/packages/ui/src/features/sessions/components/chat-thread/ChatThread.tsx +++ b/products/desktop/packages/ui/src/features/sessions/components/chat-thread/ChatThread.tsx @@ -148,11 +148,14 @@ import { type DiffWorkerFactory, } from "@posthog/ui/shell/diffWorkerHost"; import { + createContext, + type FocusEvent, memo, type ReactElement, type ReactNode, type RefObject, useCallback, + useContext, useEffect, useLayoutEffect, useMemo, @@ -352,6 +355,14 @@ function formatTimestamp(ts: number): string { * A rated turn keeps its footer on screen, so the reader can see which thumb they picked without * hovering to find out. */ +/** + * True while the pointer is over the row, or focus sits inside it. The feedback thumbs (each a + * Tooltip around a Button) mount only then: a thread mounts dozens of rows at once and they are + * invisible until hover anyway. The copy button stays mounted regardless, so the footer keeps the + * tab stop a keyboard reader needs to reach the thumbs at all. + */ +const FooterRevealContext = createContext(false); + function TurnFooter({ turnId, timestamp, @@ -362,19 +373,24 @@ function TurnFooter({ copyText?: string; }) { const sentiment = useTurnFeedback(turnId); + const revealed = useContext(FooterRevealContext); if (timestamp == null) return null; return ( {formatTimestamp(timestamp)} {copyText && } - + {(revealed || sentiment) && ( + + )} ); } @@ -578,18 +594,20 @@ function UserBubble({ const [isExpanded, setIsExpanded] = useState(false); const [isOverflowing, setIsOverflowing] = useState(false); const textRef = useRef(null); + const footerRevealed = useContext(FooterRevealContext); // Only meaningful while collapsed: expanding removes the clamp so scrollHeight === clientHeight. // We keep the prior result when expanded so the "Show less" trigger stays put. // biome-ignore lint/correctness/useExhaustiveDependencies: re-measure when the message text changes. - useLayoutEffect(() => { + useEffect(() => { if (isExpanded) return; const el = textRef.current; if (!el) return; - const measure = () => - setIsOverflowing(el.scrollHeight - el.clientHeight > 1); - measure(); - const observer = new ResizeObserver(measure); + // The observer fires once on observe, after layout, so the first measure + // forces no layout inside the commit. + const observer = new ResizeObserver(() => + setIsOverflowing(el.scrollHeight - el.clientHeight > 1), + ); observer.observe(el); return () => observer.disconnect(); }, [displayContent, isExpanded]); @@ -698,9 +716,11 @@ function UserBubble({ )} {timestamp != null && ( - + {formatTimestamp(timestamp)} - + {(footerRevealed || keyboardFocused) && ( + + )} )} @@ -842,6 +862,38 @@ function ThreadItemBody({ return <>{renderItem(item)}; } +/** + * Pointer and focus state for one transcript row, which is what its footer mounts against. Focus + * counts alongside hover because the footer holds the only copy and rating controls a turn has: a + * reader who tabs into the row has to be able to bring them out. + */ +function useRowReveal(keyboardFocused?: boolean): { + revealed: boolean; + rowProps: { + onPointerEnter: () => void; + onPointerLeave: () => void; + onFocus: () => void; + onBlur: (event: FocusEvent) => void; + }; +} { + const [hovered, setHovered] = useState(false); + const [focusWithin, setFocusWithin] = useState(false); + return { + revealed: hovered || focusWithin || Boolean(keyboardFocused), + rowProps: { + onPointerEnter: () => setHovered(true), + onPointerLeave: () => setHovered(false), + onFocus: () => setFocusWithin(true), + // React's blur is focusout, so it also fires for moves within the row. + onBlur: (event) => { + if (!event.currentTarget.contains(event.relatedTarget)) { + setFocusWithin(false); + } + }, + }, + }; +} + /** * One transcript row. Memoized and scroll-state-free, so rows never re-render while scrolling — the * non-virtualized thread stays cheap. The pinned header is the separate overlay, not the rows. @@ -858,6 +910,7 @@ const ThreadRow = memo(function ThreadRow({ renderItem: (item: ConversationItem) => ReactNode; keyboardFocused?: boolean; }) { + const { revealed: footerRevealed, rowProps } = useRowReveal(keyboardFocused); if (item.type === "agent_turn") { return ( -
- {item.items.map((sub, i) => ( - // The scroller item's own content-visibility works at whole-turn granularity — a - // large turn (diffs, charts, dozens of tools) would render wholesale as soon as the - // card nears the viewport. Nesting content-visibility per sub-item keeps layout + - // paint bounded to the viewport-sized slice while scrolling; `auto` remembers each - // row's real size after first render so the scrollbar stays stable. -
- -
- ))} -
- + +
+ {item.items.map((sub, i) => ( + // The scroller item's own content-visibility works at whole-turn granularity — a + // large turn (diffs, charts, dozens of tools) would render wholesale as soon as the + // card nears the viewport. Nesting content-visibility per sub-item keeps layout + + // paint bounded to the viewport-sized slice while scrolling; `auto` remembers each + // row's real size after first render so the scrollbar stays stable. +
+ +
+ ))} +
+ + ); } @@ -899,12 +955,15 @@ const ThreadRow = memo(function ThreadRow({ scrollAnchor={item.type === "user_message"} className="mx-auto w-full py-1 empty:hidden" style={{ maxWidth: CHAT_CONTENT_MAX_WIDTH }} + {...rowProps} > - + + + ); }); @@ -1237,10 +1296,13 @@ const FlatRowView = memo( keyboardFocused: boolean; }) { const { item } = row; + const { revealed: footerRevealed, rowProps } = + useRowReveal(keyboardFocused); return ( - - {row.turnId != null && row.turnTimestamp != null && ( - + - )} + {row.turnId != null && row.turnTimestamp != null && ( + + )} + ); }, diff --git a/products/desktop/packages/ui/src/features/settings/hooks/useOpenSettings.test.ts b/products/desktop/packages/ui/src/features/settings/hooks/useOpenSettings.test.ts index e080f697cf3c..d9753deae109 100644 --- a/products/desktop/packages/ui/src/features/settings/hooks/useOpenSettings.test.ts +++ b/products/desktop/packages/ui/src/features/settings/hooks/useOpenSettings.test.ts @@ -6,6 +6,7 @@ const isOnSettingsRoute = vi.fn(() => false); vi.mock("@posthog/ui/router/navigationBridge", () => ({ navigateToSettings: (...args: unknown[]) => navigateToSettings(...args), isOnSettingsRoute: () => isOnSettingsRoute(), + isSettingsRouteId: (routeId: string) => routeId.includes("/settings/"), canGoBackInHistory: vi.fn(), goBackInHistory: vi.fn(), navigateToNewTask: vi.fn(), diff --git a/products/desktop/packages/ui/src/features/settings/hooks/useOpenSettings.ts b/products/desktop/packages/ui/src/features/settings/hooks/useOpenSettings.ts index 813ac3d64dc2..229f9a308ca9 100644 --- a/products/desktop/packages/ui/src/features/settings/hooks/useOpenSettings.ts +++ b/products/desktop/packages/ui/src/features/settings/hooks/useOpenSettings.ts @@ -76,6 +76,6 @@ export function useCloseSettings(): typeof closeSettings { */ export function useIsSettingsOpen(): boolean { return useRouterState({ - select: (s) => s.matches.some((m) => m.routeId.startsWith("/settings")), + select: (s) => s.matches.some((m) => nav.isSettingsRouteId(m.routeId)), }); } diff --git a/products/desktop/packages/ui/src/features/settings/sections/environments/EnvironmentsSettings.tsx b/products/desktop/packages/ui/src/features/settings/sections/environments/EnvironmentsSettings.tsx index 5d7b7ab44a3f..d768574f8c19 100644 --- a/products/desktop/packages/ui/src/features/settings/sections/environments/EnvironmentsSettings.tsx +++ b/products/desktop/packages/ui/src/features/settings/sections/environments/EnvironmentsSettings.tsx @@ -16,7 +16,9 @@ export function EnvironmentsSettings() { const { localWorkspaces } = useHostCapabilities(); const activeCategory = useRouterState({ select: (s) => { - const match = s.matches.find((m) => m.routeId === "/settings/$category"); + const match = s.matches.find( + (m) => m.routeId === "/_shell/settings/$category", + ); const params = match?.params as { category?: string } | undefined; return params?.category ?? "environments"; }, diff --git a/products/desktop/packages/ui/src/features/workspace/useWorkspace.ts b/products/desktop/packages/ui/src/features/workspace/useWorkspace.ts index d5b7ea98d582..927de84b3220 100644 --- a/products/desktop/packages/ui/src/features/workspace/useWorkspace.ts +++ b/products/desktop/packages/ui/src/features/workspace/useWorkspace.ts @@ -1,18 +1,54 @@ import { useHostTRPC } from "@posthog/host-router/react"; import type { Workspace } from "@posthog/shared"; import type { Task } from "@posthog/shared/domain-types"; -import { useQuery } from "@tanstack/react-query"; -import { useMemo } from "react"; +import { + type QueryClient, + QueryObserver, + useQueryClient, +} from "@tanstack/react-query"; +import { useCallback, useMemo, useSyncExternalStore } from "react"; -function useWorkspacesQuery() { - const trpc = useHostTRPC(); - return useQuery( +type HostTRPC = ReturnType; + +function createWorkspacesObserver(queryClient: QueryClient, trpc: HostTRPC) { + return new QueryObserver( + queryClient, trpc.workspace.getAll.queryOptions(undefined, { staleTime: 1000 * 60, + // The bare observer never tracks accessed fields the way `useQuery` does, + // so without this it notifies every subscriber on any result change + // (fetchStatus flips, dataUpdatedAt bumps on refetch). All hooks here read + // only these two fields. + notifyOnChangeProps: ["data", "isFetched"], }), ); } +type WorkspacesObserver = ReturnType; + +// Dozens of rows and panels read this map. One observer per query client, +// shared by every consumer, keeps their mount from creating a QueryObserver each. +const observers = new WeakMap(); + +function useWorkspacesQuery(): ReturnType< + WorkspacesObserver["getCurrentResult"] +> { + const trpc = useHostTRPC(); + const queryClient = useQueryClient(); + let observer = observers.get(queryClient); + if (!observer) { + observer = createWorkspacesObserver(queryClient, trpc); + observers.set(queryClient, observer); + } + const shared = observer; + const subscribe = useCallback( + (onChange: () => void) => shared.subscribe(onChange), + [shared], + ); + const getSnapshot = useCallback(() => shared.getCurrentResult(), [shared]); + return useSyncExternalStore(subscribe, getSnapshot, getSnapshot); +} + export function useWorkspaces(): { data: Record | undefined; isFetched: boolean; diff --git a/products/desktop/packages/ui/src/hooks/useMountedOnceOpened.ts b/products/desktop/packages/ui/src/hooks/useMountedOnceOpened.ts new file mode 100644 index 000000000000..ac14d8480293 --- /dev/null +++ b/products/desktop/packages/ui/src/hooks/useMountedOnceOpened.ts @@ -0,0 +1,10 @@ +import { useState } from "react"; + +// Dialogs rendered per list row are expensive to mount even while closed, so +// callers gate them on this: false until the first open, true from then on so +// close transitions keep their element. +export function useMountedOnceOpened(open: boolean): boolean { + const [mounted, setMounted] = useState(open); + if (open && !mounted) setMounted(true); + return mounted || open; +} diff --git a/products/desktop/packages/ui/src/primitives/MermaidDiagram.tsx b/products/desktop/packages/ui/src/primitives/MermaidDiagram.tsx index 303a833c1e01..45ab8372b0d1 100644 --- a/products/desktop/packages/ui/src/primitives/MermaidDiagram.tsx +++ b/products/desktop/packages/ui/src/primitives/MermaidDiagram.tsx @@ -7,6 +7,23 @@ import { useEffect, useId, useState } from "react"; let mermaidModule: Promise | null = null; let initializedDarkMode: boolean | null = null; +// Rendering runs dagre layout and builds thousands of SVG nodes on the main +// thread, so a session that is reopened reuses its diagrams instead. +const RENDER_CACHE_MAX = 50; +const renderCache = new Map(); + +function renderCacheKey(code: string, isDarkMode: boolean): string { + return `${isDarkMode ? "dark" : "light"}\n${code}`; +} + +function rememberRender(key: string, svg: string): void { + if (renderCache.size >= RENDER_CACHE_MAX) { + const oldest = renderCache.keys().next().value; + if (oldest !== undefined) renderCache.delete(oldest); + } + renderCache.set(key, svg); +} + function loadMermaid(): Promise { mermaidModule ??= import("mermaid").then((module) => module.default); return mermaidModule; @@ -48,6 +65,7 @@ async function renderDiagram( initializedDarkMode = isDarkMode; } const { svg } = await mermaid.render(id, code); + rememberRender(renderCacheKey(code, isDarkMode), svg); return svg; } @@ -64,9 +82,17 @@ interface MermaidDiagramProps { export function MermaidDiagram({ code, className }: MermaidDiagramProps) { const isDarkMode = useThemeStore((s) => s.isDarkMode); const diagramId = `mermaid-${useId().replace(/[^a-zA-Z0-9]/g, "")}`; - const [state, setState] = useState({ status: "loading" }); + const [state, setState] = useState(() => { + const svg = renderCache.get(renderCacheKey(code, isDarkMode)); + return svg ? { status: "ready", svg } : { status: "loading" }; + }); useEffect(() => { + const cached = renderCache.get(renderCacheKey(code, isDarkMode)); + if (cached) { + setState({ status: "ready", svg: cached }); + return; + } let cancelled = false; renderDiagram(diagramId, code, isDarkMode) .then((svg) => { diff --git a/products/desktop/packages/ui/src/router/navigationBridge.ts b/products/desktop/packages/ui/src/router/navigationBridge.ts index 651520f00562..5d7ea2a907f6 100644 --- a/products/desktop/packages/ui/src/router/navigationBridge.ts +++ b/products/desktop/packages/ui/src/router/navigationBridge.ts @@ -257,10 +257,17 @@ export function navigateToSettings( }); } +// Settings sits under the pathless `_shell` layout, so its route IDs read +// `/_shell/settings/…` rather than `/settings/…`. Match on the substring so a +// later move between layouts does not silently switch this off. +export function isSettingsRouteId(routeId: string): boolean { + return routeId.includes("/settings/"); +} + export function isOnSettingsRoute(): boolean { return ( getRouterOrNull()?.state.matches.some((m) => - m.routeId.startsWith("/settings"), + isSettingsRouteId(m.routeId), ) ?? false ); } diff --git a/products/desktop/packages/ui/src/router/routeTree.gen.ts b/products/desktop/packages/ui/src/router/routeTree.gen.ts index bffc8e03cfe5..b07949c1df7f 100644 --- a/products/desktop/packages/ui/src/router/routeTree.gen.ts +++ b/products/desktop/packages/ui/src/router/routeTree.gen.ts @@ -17,7 +17,6 @@ import { Route as ArchivedRouteImport } from './routes/archived' import { Route as AgentsRouteImport } from './routes/agents' import { Route as ShellRouteImport } from './routes/_shell' import { Route as WebsiteIndexRouteImport } from './routes/website.index' -import { Route as SettingsIndexRouteImport } from './routes/settings/index' import { Route as LoopsIndexRouteImport } from './routes/loops/index' import { Route as InboxIndexRouteImport } from './routes/inbox/index' import { Route as CodeIndexRouteImport } from './routes/code.index' @@ -25,7 +24,6 @@ import { Route as AgentsIndexRouteImport } from './routes/agents/index' import { Route as ShellIndexRouteImport } from './routes/_shell/index' import { Route as WebsiteSplatRouteImport } from './routes/website.$' import { Route as TasksTaskIdRouteImport } from './routes/tasks/$taskId' -import { Route as SettingsCategoryRouteImport } from './routes/settings/$category' import { Route as LoopsNewRouteImport } from './routes/loops/new' import { Route as LoopsLoopIdRouteImport } from './routes/loops/$loopId' import { Route as InboxRunsRouteImport } from './routes/inbox/runs' @@ -49,6 +47,7 @@ import { Route as InboxPullsIndexRouteImport } from './routes/inbox/pulls.index' import { Route as InboxDismissedIndexRouteImport } from './routes/inbox/dismissed.index' import { Route as AgentsScoutsIndexRouteImport } from './routes/agents/scouts.index' import { Route as ShellSpacesIndexRouteImport } from './routes/_shell/spaces/index' +import { Route as ShellSettingsIndexRouteImport } from './routes/_shell/settings/index' import { Route as TasksPendingKeyRouteImport } from './routes/tasks/pending.$key' import { Route as LoopsLoopIdEditRouteImport } from './routes/loops/$loopId/edit' import { Route as InboxRunsReportIdRouteImport } from './routes/inbox/runs.$reportId' @@ -59,6 +58,7 @@ import { Route as AgentsScoutsScratchpadRouteImport } from './routes/agents/scou import { Route as AgentsScoutsFindingsRouteImport } from './routes/agents/scouts.findings' import { Route as AgentsScoutsSkillNameRouteImport } from './routes/agents/scouts.$skillName' import { Route as ShellSpacesContextRouteImport } from './routes/_shell/spaces/context' +import { Route as ShellSettingsCategoryRouteImport } from './routes/_shell/settings/$category' import { Route as ShellFeedsFeedIdRouteImport } from './routes/_shell/feeds/$feedId' import { Route as AgentsScoutsSkillNameIndexRouteImport } from './routes/agents/scouts.$skillName.index' import { Route as ShellSpacesChannelIdIndexRouteImport } from './routes/_shell/spaces/$channelId/index' @@ -111,11 +111,6 @@ const WebsiteIndexRoute = WebsiteIndexRouteImport.update({ path: '/website/', getParentRoute: () => rootRouteImport, } as any) -const SettingsIndexRoute = SettingsIndexRouteImport.update({ - id: '/settings/', - path: '/settings/', - getParentRoute: () => rootRouteImport, -} as any) const LoopsIndexRoute = LoopsIndexRouteImport.update({ id: '/loops/', path: '/loops/', @@ -151,11 +146,6 @@ const TasksTaskIdRoute = TasksTaskIdRouteImport.update({ path: '/tasks/$taskId', getParentRoute: () => rootRouteImport, } as any) -const SettingsCategoryRoute = SettingsCategoryRouteImport.update({ - id: '/settings/$category', - path: '/settings/$category', - getParentRoute: () => rootRouteImport, -} as any) const LoopsNewRoute = LoopsNewRouteImport.update({ id: '/loops/new', path: '/loops/new', @@ -271,6 +261,11 @@ const ShellSpacesIndexRoute = ShellSpacesIndexRouteImport.update({ path: '/spaces/', getParentRoute: () => ShellRoute, } as any) +const ShellSettingsIndexRoute = ShellSettingsIndexRouteImport.update({ + id: '/settings/', + path: '/settings/', + getParentRoute: () => ShellRoute, +} as any) const TasksPendingKeyRoute = TasksPendingKeyRouteImport.update({ id: '/tasks/pending/$key', path: '/tasks/pending/$key', @@ -321,6 +316,11 @@ const ShellSpacesContextRoute = ShellSpacesContextRouteImport.update({ path: '/spaces/context', getParentRoute: () => ShellRoute, } as any) +const ShellSettingsCategoryRoute = ShellSettingsCategoryRouteImport.update({ + id: '/settings/$category', + path: '/settings/$category', + getParentRoute: () => ShellRoute, +} as any) const ShellFeedsFeedIdRoute = ShellFeedsFeedIdRouteImport.update({ id: '/feeds/$feedId', path: '/feeds/$feedId', @@ -416,16 +416,15 @@ export interface FileRoutesByFullPath { '/inbox/runs': typeof InboxRunsRouteWithChildren '/loops/$loopId': typeof LoopsLoopIdRouteWithChildren '/loops/new': typeof LoopsNewRoute - '/settings/$category': typeof SettingsCategoryRoute '/tasks/$taskId': typeof TasksTaskIdRoute '/website/$': typeof WebsiteSplatRoute '/agents/': typeof AgentsIndexRoute '/code/': typeof CodeIndexRoute '/inbox/': typeof InboxIndexRoute '/loops/': typeof LoopsIndexRoute - '/settings/': typeof SettingsIndexRoute '/website/': typeof WebsiteIndexRoute '/feeds/$feedId': typeof ShellFeedsFeedIdRoute + '/settings/$category': typeof ShellSettingsCategoryRoute '/spaces/context': typeof ShellSpacesContextRoute '/agents/scouts/$skillName': typeof AgentsScoutsSkillNameRouteWithChildren '/agents/scouts/findings': typeof AgentsScoutsFindingsRoute @@ -436,6 +435,7 @@ export interface FileRoutesByFullPath { '/inbox/runs/$reportId': typeof InboxRunsReportIdRoute '/loops/$loopId/edit': typeof LoopsLoopIdEditRoute '/tasks/pending/$key': typeof TasksPendingKeyRoute + '/settings/': typeof ShellSettingsIndexRoute '/spaces/': typeof ShellSpacesIndexRoute '/agents/scouts/': typeof AgentsScoutsIndexRoute '/inbox/dismissed/': typeof InboxDismissedIndexRoute @@ -470,7 +470,6 @@ export interface FileRoutesByTo { '/folders/$folderId': typeof FoldersFolderIdRoute '/inbox/agents': typeof InboxAgentsRoute '/loops/new': typeof LoopsNewRoute - '/settings/$category': typeof SettingsCategoryRoute '/tasks/$taskId': typeof TasksTaskIdRoute '/website/$': typeof WebsiteSplatRoute '/': typeof ShellIndexRoute @@ -478,9 +477,9 @@ export interface FileRoutesByTo { '/code': typeof CodeIndexRoute '/inbox': typeof InboxIndexRoute '/loops': typeof LoopsIndexRoute - '/settings': typeof SettingsIndexRoute '/website': typeof WebsiteIndexRoute '/feeds/$feedId': typeof ShellFeedsFeedIdRoute + '/settings/$category': typeof ShellSettingsCategoryRoute '/spaces/context': typeof ShellSpacesContextRoute '/agents/scouts/findings': typeof AgentsScoutsFindingsRoute '/agents/scouts/scratchpad': typeof AgentsScoutsScratchpadRoute @@ -490,6 +489,7 @@ export interface FileRoutesByTo { '/inbox/runs/$reportId': typeof InboxRunsReportIdRoute '/loops/$loopId/edit': typeof LoopsLoopIdEditRoute '/tasks/pending/$key': typeof TasksPendingKeyRoute + '/settings': typeof ShellSettingsIndexRoute '/spaces': typeof ShellSpacesIndexRoute '/agents/scouts': typeof AgentsScoutsIndexRoute '/inbox/dismissed': typeof InboxDismissedIndexRoute @@ -534,7 +534,6 @@ export interface FileRoutesById { '/inbox/runs': typeof InboxRunsRouteWithChildren '/loops/$loopId': typeof LoopsLoopIdRouteWithChildren '/loops/new': typeof LoopsNewRoute - '/settings/$category': typeof SettingsCategoryRoute '/tasks/$taskId': typeof TasksTaskIdRoute '/website/$': typeof WebsiteSplatRoute '/_shell/': typeof ShellIndexRoute @@ -542,9 +541,9 @@ export interface FileRoutesById { '/code/': typeof CodeIndexRoute '/inbox/': typeof InboxIndexRoute '/loops/': typeof LoopsIndexRoute - '/settings/': typeof SettingsIndexRoute '/website/': typeof WebsiteIndexRoute '/_shell/feeds/$feedId': typeof ShellFeedsFeedIdRoute + '/_shell/settings/$category': typeof ShellSettingsCategoryRoute '/_shell/spaces/context': typeof ShellSpacesContextRoute '/agents/scouts/$skillName': typeof AgentsScoutsSkillNameRouteWithChildren '/agents/scouts/findings': typeof AgentsScoutsFindingsRoute @@ -555,6 +554,7 @@ export interface FileRoutesById { '/inbox/runs/$reportId': typeof InboxRunsReportIdRoute '/loops/$loopId/edit': typeof LoopsLoopIdEditRoute '/tasks/pending/$key': typeof TasksPendingKeyRoute + '/_shell/settings/': typeof ShellSettingsIndexRoute '/_shell/spaces/': typeof ShellSpacesIndexRoute '/agents/scouts/': typeof AgentsScoutsIndexRoute '/inbox/dismissed/': typeof InboxDismissedIndexRoute @@ -600,16 +600,15 @@ export interface FileRouteTypes { | '/inbox/runs' | '/loops/$loopId' | '/loops/new' - | '/settings/$category' | '/tasks/$taskId' | '/website/$' | '/agents/' | '/code/' | '/inbox/' | '/loops/' - | '/settings/' | '/website/' | '/feeds/$feedId' + | '/settings/$category' | '/spaces/context' | '/agents/scouts/$skillName' | '/agents/scouts/findings' @@ -620,6 +619,7 @@ export interface FileRouteTypes { | '/inbox/runs/$reportId' | '/loops/$loopId/edit' | '/tasks/pending/$key' + | '/settings/' | '/spaces/' | '/agents/scouts/' | '/inbox/dismissed/' @@ -654,7 +654,6 @@ export interface FileRouteTypes { | '/folders/$folderId' | '/inbox/agents' | '/loops/new' - | '/settings/$category' | '/tasks/$taskId' | '/website/$' | '/' @@ -662,9 +661,9 @@ export interface FileRouteTypes { | '/code' | '/inbox' | '/loops' - | '/settings' | '/website' | '/feeds/$feedId' + | '/settings/$category' | '/spaces/context' | '/agents/scouts/findings' | '/agents/scouts/scratchpad' @@ -674,6 +673,7 @@ export interface FileRouteTypes { | '/inbox/runs/$reportId' | '/loops/$loopId/edit' | '/tasks/pending/$key' + | '/settings' | '/spaces' | '/agents/scouts' | '/inbox/dismissed' @@ -717,7 +717,6 @@ export interface FileRouteTypes { | '/inbox/runs' | '/loops/$loopId' | '/loops/new' - | '/settings/$category' | '/tasks/$taskId' | '/website/$' | '/_shell/' @@ -725,9 +724,9 @@ export interface FileRouteTypes { | '/code/' | '/inbox/' | '/loops/' - | '/settings/' | '/website/' | '/_shell/feeds/$feedId' + | '/_shell/settings/$category' | '/_shell/spaces/context' | '/agents/scouts/$skillName' | '/agents/scouts/findings' @@ -738,6 +737,7 @@ export interface FileRouteTypes { | '/inbox/runs/$reportId' | '/loops/$loopId/edit' | '/tasks/pending/$key' + | '/_shell/settings/' | '/_shell/spaces/' | '/agents/scouts/' | '/inbox/dismissed/' @@ -770,12 +770,10 @@ export interface RootRouteChildren { FoldersFolderIdRoute: typeof FoldersFolderIdRoute LoopsLoopIdRoute: typeof LoopsLoopIdRouteWithChildren LoopsNewRoute: typeof LoopsNewRoute - SettingsCategoryRoute: typeof SettingsCategoryRoute TasksTaskIdRoute: typeof TasksTaskIdRoute WebsiteSplatRoute: typeof WebsiteSplatRoute CodeIndexRoute: typeof CodeIndexRoute LoopsIndexRoute: typeof LoopsIndexRoute - SettingsIndexRoute: typeof SettingsIndexRoute WebsiteIndexRoute: typeof WebsiteIndexRoute TasksPendingKeyRoute: typeof TasksPendingKeyRoute } @@ -838,13 +836,6 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof WebsiteIndexRouteImport parentRoute: typeof rootRouteImport } - '/settings/': { - id: '/settings/' - path: '/settings' - fullPath: '/settings/' - preLoaderRoute: typeof SettingsIndexRouteImport - parentRoute: typeof rootRouteImport - } '/loops/': { id: '/loops/' path: '/loops' @@ -894,13 +885,6 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof TasksTaskIdRouteImport parentRoute: typeof rootRouteImport } - '/settings/$category': { - id: '/settings/$category' - path: '/settings/$category' - fullPath: '/settings/$category' - preLoaderRoute: typeof SettingsCategoryRouteImport - parentRoute: typeof rootRouteImport - } '/loops/new': { id: '/loops/new' path: '/loops/new' @@ -1062,6 +1046,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof ShellSpacesIndexRouteImport parentRoute: typeof ShellRoute } + '/_shell/settings/': { + id: '/_shell/settings/' + path: '/settings' + fullPath: '/settings/' + preLoaderRoute: typeof ShellSettingsIndexRouteImport + parentRoute: typeof ShellRoute + } '/tasks/pending/$key': { id: '/tasks/pending/$key' path: '/tasks/pending/$key' @@ -1132,6 +1123,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof ShellSpacesContextRouteImport parentRoute: typeof ShellRoute } + '/_shell/settings/$category': { + id: '/_shell/settings/$category' + path: '/settings/$category' + fullPath: '/settings/$category' + preLoaderRoute: typeof ShellSettingsCategoryRouteImport + parentRoute: typeof ShellRoute + } '/_shell/feeds/$feedId': { id: '/_shell/feeds/$feedId' path: '/feeds/$feedId' @@ -1228,7 +1226,9 @@ interface ShellRouteChildren { ShellSkillsRoute: typeof ShellSkillsRoute ShellIndexRoute: typeof ShellIndexRoute ShellFeedsFeedIdRoute: typeof ShellFeedsFeedIdRoute + ShellSettingsCategoryRoute: typeof ShellSettingsCategoryRoute ShellSpacesContextRoute: typeof ShellSpacesContextRoute + ShellSettingsIndexRoute: typeof ShellSettingsIndexRoute ShellSpacesIndexRoute: typeof ShellSpacesIndexRoute ShellSpacesChannelIdArtifactsRoute: typeof ShellSpacesChannelIdArtifactsRoute ShellSpacesChannelIdCanvasesRoute: typeof ShellSpacesChannelIdCanvasesRoute @@ -1251,7 +1251,9 @@ const ShellRouteChildren: ShellRouteChildren = { ShellSkillsRoute: ShellSkillsRoute, ShellIndexRoute: ShellIndexRoute, ShellFeedsFeedIdRoute: ShellFeedsFeedIdRoute, + ShellSettingsCategoryRoute: ShellSettingsCategoryRoute, ShellSpacesContextRoute: ShellSpacesContextRoute, + ShellSettingsIndexRoute: ShellSettingsIndexRoute, ShellSpacesIndexRoute: ShellSpacesIndexRoute, ShellSpacesChannelIdArtifactsRoute: ShellSpacesChannelIdArtifactsRoute, ShellSpacesChannelIdCanvasesRoute: ShellSpacesChannelIdCanvasesRoute, @@ -1415,12 +1417,10 @@ const rootRouteChildren: RootRouteChildren = { FoldersFolderIdRoute: FoldersFolderIdRoute, LoopsLoopIdRoute: LoopsLoopIdRouteWithChildren, LoopsNewRoute: LoopsNewRoute, - SettingsCategoryRoute: SettingsCategoryRoute, TasksTaskIdRoute: TasksTaskIdRoute, WebsiteSplatRoute: WebsiteSplatRoute, CodeIndexRoute: CodeIndexRoute, LoopsIndexRoute: LoopsIndexRoute, - SettingsIndexRoute: SettingsIndexRoute, WebsiteIndexRoute: WebsiteIndexRoute, TasksPendingKeyRoute: TasksPendingKeyRoute, } diff --git a/products/desktop/packages/ui/src/router/routes/__root.tsx b/products/desktop/packages/ui/src/router/routes/__root.tsx index 408b642e9e28..9027ea01a256 100644 --- a/products/desktop/packages/ui/src/router/routes/__root.tsx +++ b/products/desktop/packages/ui/src/router/routes/__root.tsx @@ -17,7 +17,6 @@ import { UsageLimitModal } from "@posthog/ui/features/billing/UsageLimitModal"; import { useSpendGuardrails } from "@posthog/ui/features/billing/useSpendGuardrails"; import { BrowserTabStrip } from "@posthog/ui/features/browser-tabs/BrowserTabStrip"; import { BrowserTabsDndProvider } from "@posthog/ui/features/browser-tabs/BrowserTabsDnd"; -import { TabShortcutFallback } from "@posthog/ui/features/browser-tabs/TabShortcutFallback"; import { isBluebirdOnlyPath } from "@posthog/ui/features/canvas/bluebirdRoutes"; import { ChannelHotkeys } from "@posthog/ui/features/canvas/components/ChannelHotkeys"; import { ChannelRouteSync } from "@posthog/ui/features/canvas/components/ChannelRouteSync"; @@ -65,6 +64,7 @@ import { UpdateAvailableModal } from "@posthog/ui/features/updates/UpdateAvailab import { WhatsNewModal } from "@posthog/ui/features/updates/WhatsNewModal"; import { useWorkspaces } from "@posthog/ui/features/workspace/useWorkspace"; import { AnimatedLogo } from "@posthog/ui/primitives/AnimatedLogo"; +import { isSettingsRouteId } from "@posthog/ui/router/navigationBridge"; import { useAppView } from "@posthog/ui/router/useAppView"; import { openTask, openTaskInput } from "@posthog/ui/router/useOpenTask"; import { track } from "@posthog/ui/shell/analytics"; @@ -284,7 +284,7 @@ function RootLayout() { // Settings is a full-page route — drop the app chrome (header/sidebar/ // space-switcher) so the panel occupies the full window. const isSettingsRoute = useRouterState({ - select: (s) => s.matches.some((m) => m.routeId.startsWith("/settings")), + select: (s) => s.matches.some((m) => isSettingsRouteId(m.routeId)), }); // ShellLayout draws the in-pane header under `_shell`, so the shared @@ -304,42 +304,18 @@ function RootLayout() { } }, [flagsLoaded, bluebirdEnabled, onBluebirdOnlyPath]); - if (isSettingsRoute) { - return ( - - - - - - - (open ? null : closeShortcutsSheet())} - /> - - {/* The settings shell has never mounted the tab strip, so nothing here - was stopping Cmd+W from closing the window. */} - - {billingEnabled && } - - - - - - - ); - } - return ( // DnD scope for the tab strip's drag-to-reorder (pill sortables live in // the title bar; the provider must sit above them). - + {/* Settings renders over this tree through a portal. Going inert keeps + focus and clicks out of the covered chrome without unmounting it. */} + {/* Full-width title bar: a window-drag region carrying the PostHog mark. The left section sizes to its controls so the tab strip sits beside the history buttons; its padding clears the macOS stoplights @@ -430,7 +406,10 @@ function RootLayout() { )} - + {/* Settings draws its own copies over this tree — see the settings + route. One instance of each at a time, so an announcement is not + reported as seen twice. */} + {!isSettingsRoute && } {/* Scrim under the peeked nav: dims the content while the overlay is out. Purely visual (pointer-transparent) and paired with the @@ -470,7 +449,7 @@ function RootLayout() { {/* Inside the framed pane, not the app column: announcements overlay the content, never the sidebar. */} - + {!isSettingsRoute && } {/* The shell renders its own header (ShellLayout); everywhere else the shared header carries the view title and, on a task, its action row. */} @@ -497,7 +476,7 @@ function RootLayout() { {/* Renders nothing — owns ⌘1-9 under the channels layout. Mounted here rather than in the switcher, which only exists once a channel is already scoped. */} - + {!isSettingsRoute && } {/* Renders nothing — owns which space is scoped. The sidebar used to, but the rail can take that column away and the scoping still has to happen. */} diff --git a/products/desktop/packages/ui/src/router/routes/_shell/settings/$category.tsx b/products/desktop/packages/ui/src/router/routes/_shell/settings/$category.tsx new file mode 100644 index 000000000000..ce92ab3f1bca --- /dev/null +++ b/products/desktop/packages/ui/src/router/routes/_shell/settings/$category.tsx @@ -0,0 +1,48 @@ +import { AnnouncementBanner } from "@posthog/ui/features/announcements/AnnouncementBanner"; +import { ConnectivityBanner } from "@posthog/ui/features/connectivity/ConnectivityBanner"; +import { SettingsPanel } from "@posthog/ui/features/settings/components/SettingsPanel"; +import { useSettingsPageStore } from "@posthog/ui/features/settings/stores/settingsPageStore"; +import { resolveSettingsCategory } from "@posthog/ui/features/settings/types"; +import { createFileRoute } from "@tanstack/react-router"; +import { useEffect } from "react"; +import { createPortal } from "react-dom"; + +// Nested under `_shell` so the sidebar, tab strip and panels stay mounted while +// settings covers them; leaving settings then costs one tab switch instead of +// rebuilding the whole shell. +export const Route = createFileRoute("/_shell/settings/$category")({ + component: SettingsRoute, +}); + +function SettingsRoute() { + const { category } = Route.useParams(); + const cat = resolveSettingsCategory(category) ?? "general"; + + // Reset transient state when leaving the route entirely. Switching between + // categories (e.g. general → environments) does not unmount this component, + // only the cleanup on full unmount needs to fire. + useEffect(() => { + return () => useSettingsPageStore.getState().reset(); + }, []); + + // Portalling to document.body would land outside the Radix subtree. + const container = + document.getElementById("portal-container") ?? document.body; + + return createPortal( +
+ {/* The shell's copies are covered by this overlay and inert, so the + banners move here for the duration: losing connectivity while + settings is open must still show the offline state and its Retry. */} + + +
+ +
+
, + container, + ); +} diff --git a/products/desktop/packages/ui/src/router/routes/settings/index.tsx b/products/desktop/packages/ui/src/router/routes/_shell/settings/index.tsx similarity index 77% rename from products/desktop/packages/ui/src/router/routes/settings/index.tsx rename to products/desktop/packages/ui/src/router/routes/_shell/settings/index.tsx index e842724ca5f1..e109a6c5ef4c 100644 --- a/products/desktop/packages/ui/src/router/routes/settings/index.tsx +++ b/products/desktop/packages/ui/src/router/routes/_shell/settings/index.tsx @@ -1,6 +1,6 @@ import { createFileRoute, redirect } from "@tanstack/react-router"; -export const Route = createFileRoute("/settings/")({ +export const Route = createFileRoute("/_shell/settings/")({ beforeLoad: () => { throw redirect({ to: "/settings/$category", diff --git a/products/desktop/packages/ui/src/router/routes/settings/$category.tsx b/products/desktop/packages/ui/src/router/routes/settings/$category.tsx deleted file mode 100644 index 14cfe2b508f5..000000000000 --- a/products/desktop/packages/ui/src/router/routes/settings/$category.tsx +++ /dev/null @@ -1,23 +0,0 @@ -import { SettingsPanel } from "@posthog/ui/features/settings/components/SettingsPanel"; -import { useSettingsPageStore } from "@posthog/ui/features/settings/stores/settingsPageStore"; -import { resolveSettingsCategory } from "@posthog/ui/features/settings/types"; -import { createFileRoute } from "@tanstack/react-router"; -import { useEffect } from "react"; - -export const Route = createFileRoute("/settings/$category")({ - component: SettingsRoute, -}); - -function SettingsRoute() { - const { category } = Route.useParams(); - const cat = resolveSettingsCategory(category) ?? "general"; - - // Reset transient state when leaving the route entirely. Switching between - // categories (e.g. general → environments) does not unmount this component, - // only the cleanup on full unmount needs to fire. - useEffect(() => { - return () => useSettingsPageStore.getState().reset(); - }, []); - - return ; -} diff --git a/products/desktop/packages/ui/src/router/useAppView.test.ts b/products/desktop/packages/ui/src/router/useAppView.test.ts new file mode 100644 index 000000000000..48fb35c77ed1 --- /dev/null +++ b/products/desktop/packages/ui/src/router/useAppView.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it, vi } from "vitest"; + +type Match = { fullPath: string; params: Record }; + +const mocks = vi.hoisted(() => ({ matches: [] as Match[] })); + +vi.mock("./navigationBridge", () => ({ + getCurrentMatches: () => mocks.matches, +})); + +import { getAppViewSnapshot } from "./useAppView"; + +// The view is derived by switching on a match's `fullPath`. The pathless +// `_shell` layout lives only in a route's id, never its fullPath, so a case +// written in id form silently never matches and settings falls through to the +// task-input view. +describe("getAppViewSnapshot", () => { + it.each([ + { fullPath: "/settings/$category", params: { category: "general" } }, + { fullPath: "/settings/", params: {} }, + ])("maps the settings route $fullPath to the settings view", (match) => { + mocks.matches = [match]; + expect(getAppViewSnapshot()).toEqual({ type: "settings" }); + }); +});