From bc9e33d66b126b8166ade7bf74c43baee2e1a4be Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 4 Jun 2026 00:28:57 -0500 Subject: [PATCH] refactor(desktop): DRY/elegance pass over PR-touched files - Shared useDeepLinkHighlight hook collapses 3 near-identical settings deep-link effects (keys/mcp); config kept inline (distinct bail-clear). - command-center: table-driven SECTION_ICONS + single errorText helper. - clarify-tool: OPTION_ROW_CLASS + RadioDot extracted from option rows. - desktop-controller: merge Cmd+K / Cmd+. into one keydown handler. - statusbar-controls: hoist shared action class. - Misc: drop redundant cn()/cursor-pointer/dead fields; tidy switch. --- apps/desktop/src/app/artifacts/index.tsx | 10 +- apps/desktop/src/app/command-center/index.tsx | 18 ++- apps/desktop/src/app/desktop-controller.tsx | 26 ++--- .../src/app/settings/about-settings.tsx | 2 +- apps/desktop/src/app/settings/constants.ts | 7 +- .../src/app/settings/keys-settings.tsx | 45 ++------ .../desktop/src/app/settings/mcp-settings.tsx | 40 ++----- .../src/app/settings/sessions-settings.tsx | 104 +++++++----------- .../app/settings/use-deep-link-highlight.ts | 60 ++++++++++ .../src/app/shell/statusbar-controls.tsx | 20 ++-- .../components/assistant-ui/clarify-tool.tsx | 38 ++++--- .../src/components/ui/dropdown-menu.tsx | 6 +- apps/desktop/src/components/ui/switch.tsx | 2 +- 13 files changed, 181 insertions(+), 197 deletions(-) create mode 100644 apps/desktop/src/app/settings/use-deep-link-highlight.ts diff --git a/apps/desktop/src/app/artifacts/index.tsx b/apps/desktop/src/app/artifacts/index.tsx index 98e472171..c5e8183c2 100644 --- a/apps/desktop/src/app/artifacts/index.tsx +++ b/apps/desktop/src/app/artifacts/index.tsx @@ -657,11 +657,7 @@ interface ArtifactImageCardProps { function ArtifactImageCard({ artifact, failedImage, onImageError, onOpenChat }: ArtifactImageCardProps) { return ( -
+
= { usage: 'Token, cost, and skill activity over time' } +const SECTION_ICONS: Record = { + sessions: Pin, + system: Activity, + usage: BarChart3 +} + +const errorText = (error: unknown): string => (error instanceof Error ? error.message : String(error)) + function formatTimestamp(value?: number | null): string { if (!value) { return '' @@ -182,7 +190,7 @@ export function CommandCenterView({ setStatus(nextStatus) setLogs(nextLogs.lines) } catch (error) { - setSystemError(error instanceof Error ? error.message : String(error)) + setSystemError(errorText(error)) } finally { setSystemLoading(false) } @@ -202,7 +210,7 @@ export function CommandCenterView({ } } catch (error) { if (usageRequestRef.current === requestId) { - setUsageError(error instanceof Error ? error.message : String(error)) + setUsageError(errorText(error)) } } finally { if (usageRequestRef.current === requestId) { @@ -266,7 +274,7 @@ export function CommandCenterView({ upsertDesktopActionTask(pendingStatus) } } catch (error) { - setSystemError(error instanceof Error ? error.message : String(error)) + setSystemError(errorText(error)) } finally { void refreshSystem() } @@ -281,7 +289,7 @@ export function CommandCenterView({ {SECTIONS.map(value => ( setSection(value)} diff --git a/apps/desktop/src/app/desktop-controller.tsx b/apps/desktop/src/app/desktop-controller.tsx index 801b963b3..86a6cca19 100644 --- a/apps/desktop/src/app/desktop-controller.tsx +++ b/apps/desktop/src/app/desktop-controller.tsx @@ -201,25 +201,21 @@ export function DesktopController() { } }, []) - // Global command palette: Cmd/Ctrl+K from anywhere. Plain Cmd+K is reserved - // for the palette; the composer's "drain next queued" moved to Cmd+Shift+K. + // Global chrome shortcuts (plain Cmd/Ctrl, no alt/shift): Cmd+K → command + // palette (the composer's "drain next queued" moved to Cmd+Shift+K), Cmd+. → + // command center (sessions / system / usage). useEffect(() => { const onKeyDown = (event: KeyboardEvent) => { - if ((event.metaKey || event.ctrlKey) && !event.altKey && !event.shiftKey && event.key.toLowerCase() === 'k') { + if (!(event.metaKey || event.ctrlKey) || event.altKey || event.shiftKey) { + return + } + + const key = event.key.toLowerCase() + + if (key === 'k') { event.preventDefault() toggleCommandPalette() - } - } - - window.addEventListener('keydown', onKeyDown) - - return () => window.removeEventListener('keydown', onKeyDown) - }, []) - - // Cmd/Ctrl+. toggles the command center (sessions / system / usage). - useEffect(() => { - const onKeyDown = (event: KeyboardEvent) => { - if ((event.metaKey || event.ctrlKey) && !event.altKey && !event.shiftKey && event.key === '.') { + } else if (key === '.') { event.preventDefault() toggleCommandCenter() } diff --git a/apps/desktop/src/app/settings/about-settings.tsx b/apps/desktop/src/app/settings/about-settings.tsx index 4d276788e..48e451c22 100644 --- a/apps/desktop/src/app/settings/about-settings.tsx +++ b/apps/desktop/src/app/settings/about-settings.tsx @@ -126,7 +126,7 @@ export function AboutSettings() { size="sm" variant="textStrong" > - {checking ? : null} + {checking && } {checking ? 'Checking…' : 'Check now'} diff --git a/apps/desktop/src/app/settings/constants.ts b/apps/desktop/src/app/settings/constants.ts index 3f499b2ea..09c75d1b1 100644 --- a/apps/desktop/src/app/settings/constants.ts +++ b/apps/desktop/src/app/settings/constants.ts @@ -289,12 +289,11 @@ export const SECTIONS: DesktopConfigSection[] = [ export interface ModeOption { id: ThemeMode label: string - description: string icon: IconComponent } export const MODE_OPTIONS: ModeOption[] = [ - { id: 'light', label: 'Light', description: 'Bright desktop surfaces', icon: Sun }, - { id: 'dark', label: 'Dark', description: 'Low-glare workspace', icon: Moon }, - { id: 'system', label: 'System', description: 'Follow OS appearance', icon: Monitor } + { id: 'light', label: 'Light', icon: Sun }, + { id: 'dark', label: 'Dark', icon: Moon }, + { id: 'system', label: 'System', icon: Monitor } ] diff --git a/apps/desktop/src/app/settings/keys-settings.tsx b/apps/desktop/src/app/settings/keys-settings.tsx index 3f6290714..2dbf228e6 100644 --- a/apps/desktop/src/app/settings/keys-settings.tsx +++ b/apps/desktop/src/app/settings/keys-settings.tsx @@ -1,5 +1,4 @@ import { useEffect, useMemo, useState } from 'react' -import { useSearchParams } from 'react-router-dom' import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' @@ -13,6 +12,7 @@ import { CONTROL_TEXT } from './constants' import { asText, prettyName, providerGroup, providerPriority, redactedValue, withoutKey } from './helpers' import { LoadingState, Pill, SectionHeading, SettingsContent } from './primitives' import type { EnvPatch, EnvRowProps, ProviderGroup } from './types' +import { useDeepLinkHighlight } from './use-deep-link-highlight' interface EnvActionsProps { varKey: string @@ -224,10 +224,13 @@ export function KeysSettings() { const [revealed, setRevealed] = useState>({}) const [saving, setSaving] = useState(null) - // Deep-link target from the command palette (?key=): force-expand - // the matching provider group, scroll the row in, and flash it. - const [searchParams, setSearchParams] = useSearchParams() - const highlightKey = searchParams.get('key') + // Deep-link from the command palette (?key=): force-expand the + // matching provider group, scroll the row in, and flash it. + const highlightKey = useDeepLinkHighlight({ + elementId: key => `env-var-${key}`, + param: 'key', + ready: key => Boolean(vars?.[key]) + }) // We used to hide ~80% of rows behind a global "Show advanced" toggle, but // everything in this view is configuration-level — "advanced" was a poor @@ -259,38 +262,6 @@ export function KeysSettings() { return () => void (cancelled = true) }, []) - useEffect(() => { - if (!highlightKey || !vars || !vars[highlightKey]) { - return - } - - // Group expansion is async (state), so defer the scroll a frame to let the - // target row mount before we look it up. - const scrollTimeout = window.setTimeout(() => { - const element = document.getElementById(`env-var-${highlightKey}`) - - if (!element) { - return - } - - element.scrollIntoView({ behavior: 'smooth', block: 'center' }) - element.classList.add('setting-field-highlight') - window.setTimeout(() => element.classList.remove('setting-field-highlight'), 1600) - }, 80) - - setSearchParams( - previous => { - const next = new URLSearchParams(previous) - next.delete('key') - - return next - }, - { replace: true } - ) - - return () => window.clearTimeout(scrollTimeout) - }, [highlightKey, setSearchParams, vars]) - const providerGroups = useMemo(() => { if (!vars) { return [] diff --git a/apps/desktop/src/app/settings/mcp-settings.tsx b/apps/desktop/src/app/settings/mcp-settings.tsx index f5ae91652..7ff9a7c5e 100644 --- a/apps/desktop/src/app/settings/mcp-settings.tsx +++ b/apps/desktop/src/app/settings/mcp-settings.tsx @@ -1,6 +1,5 @@ import { useStore } from '@nanostores/react' import { useEffect, useMemo, useState } from 'react' -import { useSearchParams } from 'react-router-dom' import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' @@ -13,6 +12,7 @@ import { $activeSessionId } from '@/store/session' import type { HermesConfigRecord } from '@/types/hermes' import { EmptyState, LoadingState, Pill, SettingsContent } from './primitives' +import { useDeepLinkHighlight } from './use-deep-link-highlight' interface McpSettingsProps { gateway?: HermesGateway | null @@ -46,7 +46,6 @@ export function McpSettings({ gateway, onConfigSaved }: McpSettingsProps) { const activeSessionId = useStore($activeSessionId) const [config, setConfig] = useState(null) const [selected, setSelected] = useState(null) - const [searchParams, setSearchParams] = useSearchParams() const [name, setName] = useState('') const [body, setBody] = useState('') const [saving, setSaving] = useState(false) @@ -73,36 +72,13 @@ export function McpSettings({ gateway, onConfigSaved }: McpSettingsProps) { const servers = useMemo(() => getServers(config), [config]) const names = useMemo(() => Object.keys(servers).sort(), [servers]) - // Deep-link target from the command palette (?server=): select it and - // scroll the list entry into view. - const targetServer = searchParams.get('server') - - useEffect(() => { - if (!targetServer || !config || !(targetServer in servers)) { - return - } - - setSelected(targetServer) - - const scrollTimeout = window.setTimeout(() => { - const element = document.getElementById(`mcp-server-${targetServer}`) - element?.scrollIntoView({ behavior: 'smooth', block: 'nearest' }) - element?.classList.add('setting-field-highlight') - window.setTimeout(() => element?.classList.remove('setting-field-highlight'), 1600) - }, 80) - - setSearchParams( - previous => { - const next = new URLSearchParams(previous) - next.delete('server') - - return next - }, - { replace: true } - ) - - return () => window.clearTimeout(scrollTimeout) - }, [config, servers, setSearchParams, targetServer]) + useDeepLinkHighlight({ + block: 'nearest', + elementId: serverName => `mcp-server-${serverName}`, + onResolve: setSelected, + param: 'server', + ready: serverName => Boolean(config) && serverName in servers + }) useEffect(() => { const server = selected ? servers[selected] : null diff --git a/apps/desktop/src/app/settings/sessions-settings.tsx b/apps/desktop/src/app/settings/sessions-settings.tsx index 1ca7e29fa..caf7647ac 100644 --- a/apps/desktop/src/app/settings/sessions-settings.tsx +++ b/apps/desktop/src/app/settings/sessions-settings.tsx @@ -1,5 +1,4 @@ import { useCallback, useEffect, useState } from 'react' -import { useSearchParams } from 'react-router-dom' import { Button } from '@/components/ui/button' import { deleteSession, listSessions, setSessionArchived } from '@/hermes' @@ -11,6 +10,7 @@ import { setSessions } from '@/store/session' import type { SessionInfo } from '@/types/hermes' import { EmptyState, ListRow, LoadingState, SectionHeading, SettingsContent } from './primitives' +import { useDeepLinkHighlight } from './use-deep-link-highlight' const ARCHIVED_FETCH_LIMIT = 200 @@ -34,7 +34,6 @@ export function SessionsSettings() { const [sessions, setLocalSessions] = useState([]) const [loading, setLoading] = useState(true) const [busyId, setBusyId] = useState(null) - const [searchParams, setSearchParams] = useSearchParams() const load = useCallback(async () => { setLoading(true) @@ -88,39 +87,11 @@ export function SessionsSettings() { } }, []) - // Deep-link target from the command palette (?session=): scroll the row - // into view and flash it. - const targetSession = searchParams.get('session') - - useEffect(() => { - if (!targetSession || loading || !sessions.some(session => session.id === targetSession)) { - return - } - - const scrollTimeout = window.setTimeout(() => { - const element = document.getElementById(`archived-session-${targetSession}`) - - if (!element) { - return - } - - element.scrollIntoView({ behavior: 'smooth', block: 'center' }) - element.classList.add('setting-field-highlight') - window.setTimeout(() => element.classList.remove('setting-field-highlight'), 1600) - }, 80) - - setSearchParams( - previous => { - const next = new URLSearchParams(previous) - next.delete('session') - - return next - }, - { replace: true } - ) - - return () => window.clearTimeout(scrollTimeout) - }, [loading, sessions, setSearchParams, targetSession]) + useDeepLinkHighlight({ + elementId: id => `archived-session-${id}`, + param: 'session', + ready: id => !loading && sessions.some(session => session.id === id) + }) if (loading) { return @@ -152,31 +123,31 @@ export function SessionsSettings() {
- - -
- } +
+ + +
+ } description={session.preview || undefined} hint={label ? `${label} · ${session.message_count} messages` : `${session.message_count} messages`} title={sessionTitle(session)} @@ -213,7 +184,10 @@ function DefaultProjectDirSetting() { let alive = true void settings.getDefaultProjectDir().then(result => { - if (!alive) {return} + if (!alive) { + return + } + setDir(result.dir) setFallback(result.defaultLabel) }) @@ -226,7 +200,9 @@ function DefaultProjectDirSetting() { const choose = useCallback(async () => { const settings = window.hermesDesktop?.settings - if (!settings) {return} + if (!settings) { + return + } setBusy(true) @@ -250,7 +226,9 @@ function DefaultProjectDirSetting() { const clear = useCallback(async () => { const settings = window.hermesDesktop?.settings - if (!settings) {return} + if (!settings) { + return + } setBusy(true) diff --git a/apps/desktop/src/app/settings/use-deep-link-highlight.ts b/apps/desktop/src/app/settings/use-deep-link-highlight.ts new file mode 100644 index 000000000..a4cabce3a --- /dev/null +++ b/apps/desktop/src/app/settings/use-deep-link-highlight.ts @@ -0,0 +1,60 @@ +import { useEffect } from 'react' +import { useSearchParams } from 'react-router-dom' + +interface DeepLinkHighlightOptions { + param: string + ready: (target: string) => boolean + elementId: (target: string) => string + onResolve?: (target: string) => void + block?: ScrollLogicalPosition +} + +// Deep-link from the command palette (?=): once the target row is +// renderable, scroll it into view and flash it, then drop the param so it +// doesn't re-fire. Returns the pending target (null once consumed) so callers +// can force the row open before it mounts. +export function useDeepLinkHighlight({ + param, + ready, + elementId, + onResolve, + block = 'center' +}: DeepLinkHighlightOptions): null | string { + const [searchParams, setSearchParams] = useSearchParams() + const target = searchParams.get(param) + + useEffect(() => { + if (!target || !ready(target)) { + return + } + + onResolve?.(target) + + // Defer a frame so async state (expansion, selection) mounts the row first. + const scrollTimeout = window.setTimeout(() => { + const element = document.getElementById(elementId(target)) + + if (!element) { + return + } + + element.scrollIntoView({ behavior: 'smooth', block }) + element.classList.add('setting-field-highlight') + window.setTimeout(() => element.classList.remove('setting-field-highlight'), 1600) + }, 80) + + setSearchParams( + previous => { + const next = new URLSearchParams(previous) + next.delete(param) + + return next + }, + { replace: true } + ) + + return () => window.clearTimeout(scrollTimeout) + }, [block, elementId, onResolve, param, ready, setSearchParams, target]) + + return target +} diff --git a/apps/desktop/src/app/shell/statusbar-controls.tsx b/apps/desktop/src/app/shell/statusbar-controls.tsx index 43487fae7..5a97f1a7b 100644 --- a/apps/desktop/src/app/shell/statusbar-controls.tsx +++ b/apps/desktop/src/app/shell/statusbar-controls.tsx @@ -4,6 +4,11 @@ import { useNavigate } from 'react-router-dom' import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger } from '@/components/ui/dropdown-menu' import { cn } from '@/lib/utils' +// Shared chrome styling for interactive statusbar items (button / link / menu +// trigger). The 'text' variant intentionally omits hover/transition/disabled. +const STATUSBAR_ACTION_CLASS = + 'inline-flex h-full items-center gap-1 rounded-none px-1.5 text-[0.6875rem] text-(--ui-text-tertiary) transition-colors hover:bg-(--chrome-action-hover) hover:text-foreground disabled:cursor-default disabled:opacity-45' + export interface StatusbarMenuItem { id: string icon?: ReactNode @@ -93,10 +98,7 @@ function StatusbarItemView({ item, navigate }: { item: StatusbarItem; navigate: ))}
diff --git a/apps/desktop/src/components/ui/dropdown-menu.tsx b/apps/desktop/src/components/ui/dropdown-menu.tsx index 0997b9c02..7c4c7072c 100644 --- a/apps/desktop/src/components/ui/dropdown-menu.tsx +++ b/apps/desktop/src/components/ui/dropdown-menu.tsx @@ -110,7 +110,7 @@ function DropdownMenuItem({ return ( & VariantProps) { return ( - + ) }