From 4bc8e62e91c8f650cedd60895e0222284b42bb25 Mon Sep 17 00:00:00 2001 From: Cyrus Gray <144336577+graycyrus@users.noreply.github.com> Date: Wed, 10 Jun 2026 17:52:08 +0530 Subject: [PATCH] fix(settings): normalize all panels to the settings UI contract (#3575) --- .../settings/LogoutAndClearActions.tsx | 2 +- app/src/components/settings/SettingsHome.tsx | 34 +---- .../settings/SettingsSectionPage.tsx | 22 ++- .../settings/__tests__/SettingsHome.test.tsx | 15 +- .../settings/components/SettingsHeader.tsx | 134 +++++++++-------- .../settings/hooks/useSettingsNavigation.ts | 50 +++++- .../settings/panels/AgentAccessPanel.tsx | 2 +- .../panels/AgentActivityPanel.test.tsx | 110 ++++++++++++++ .../settings/panels/AgentActivityPanel.tsx | 142 ++++++++++-------- .../settings/panels/ApprovalHistoryPanel.tsx | 2 +- .../settings/panels/BillingPanel.test.tsx | 12 +- .../settings/panels/BillingPanel.tsx | 121 +++++++-------- .../settings/panels/ComposioTriagePanel.tsx | 4 +- .../settings/panels/DevicesPanel.tsx | 26 ++-- .../settings/panels/PermissionsPanel.tsx | 2 +- .../settings/panels/PrivacyPanel.tsx | 2 +- .../settings/panels/RecoveryPhrasePanel.tsx | 2 +- .../settings/panels/SandboxSettingsPanel.tsx | 6 +- .../settings/panels/SecurityPanel.tsx | 2 +- .../settings/panels/TeamInvitesPanel.tsx | 2 +- .../settings/panels/TeamManagementPanel.tsx | 6 +- .../settings/panels/TeamMembersPanel.tsx | 2 +- .../components/settings/panels/TeamPanel.tsx | 2 +- .../settings/panels/VoiceDebugPanel.tsx | 2 +- .../settings/panels/WalletBalancesPanel.tsx | 58 +++---- app/src/pages/Settings.tsx | 13 +- 26 files changed, 458 insertions(+), 317 deletions(-) create mode 100644 app/src/components/settings/panels/AgentActivityPanel.test.tsx diff --git a/app/src/components/settings/LogoutAndClearActions.tsx b/app/src/components/settings/LogoutAndClearActions.tsx index 26dabd38a..827741bb9 100644 --- a/app/src/components/settings/LogoutAndClearActions.tsx +++ b/app/src/components/settings/LogoutAndClearActions.tsx @@ -65,7 +65,7 @@ const LogoutAndClearActions = () => { const showInlineError = error !== null && !showLogoutAndClearModal; return ( -
+
); -const PrivacyIcon = ( - - - -); - const NotificationsIcon = ( { ], }; - // --- 🔒 Privacy group (Security + Approvals moved to Developer & Diagnostics) --- - const privacySecurityGroup: SettingsGroup = { - id: 'privacy-security', - label: t('settings.privacySecurity.privacy'), - items: [ - { - id: 'privacy', - title: t('settings.privacySecurity.privacy'), - description: t('settings.privacySecurity.privacyDesc'), - icon: PrivacyIcon, - onClick: () => navigateToSettings('privacy'), - }, - ], - }; + // Privacy is reached from the Account hub (Settings → Account → Privacy); + // it is intentionally not duplicated as a top-level row here. // --- 🔔 Notifications group --- const notificationsGroup: SettingsGroup = { @@ -299,12 +276,7 @@ const SettingsHome = () => { }; // --- Always-visible groups --- - const visibleGroups: SettingsGroup[] = [ - accountGroup, - assistantGroup, - privacySecurityGroup, - notificationsGroup, - ]; + const visibleGroups: SettingsGroup[] = [accountGroup, assistantGroup, notificationsGroup]; // Billing / Rewards / Wallet are NOT in Settings — per the design doc they // live in the avatar menu (monetisation out of the settings tree). diff --git a/app/src/components/settings/SettingsSectionPage.tsx b/app/src/components/settings/SettingsSectionPage.tsx index 854851d91..1549adad2 100644 --- a/app/src/components/settings/SettingsSectionPage.tsx +++ b/app/src/components/settings/SettingsSectionPage.tsx @@ -39,14 +39,15 @@ const SettingsSectionPage = ({ title, description, items, footer }: SettingsSect breadcrumbs={breadcrumbs} /> -
+ {/* Mirror the SettingsHome layout: padded container, items in a single + rounded-border card, and the optional footer in its own matching card + so section pages and the home list look identical. */} +
{description && ( -

- {description} -

+

{description}

)} -
+
{items.map((item, index) => ( - {footer} + {footer && ( + <> + {/* Divider + card, mirroring how SettingsHome separates its + trailing groups (e.g. the destructive logout/clear card). */} +
+
+ {footer} +
+ + )}
); diff --git a/app/src/components/settings/__tests__/SettingsHome.test.tsx b/app/src/components/settings/__tests__/SettingsHome.test.tsx index fe3d61c58..10328d440 100644 --- a/app/src/components/settings/__tests__/SettingsHome.test.tsx +++ b/app/src/components/settings/__tests__/SettingsHome.test.tsx @@ -145,10 +145,11 @@ describe('SettingsHome', () => { expect(screen.queryByTestId('settings-nav-companion')).not.toBeInTheDocument(); }); - it('renders the Privacy group items', () => { + it('does not surface Privacy/Security/Approvals on the home list', () => { renderSettingsHome(); - // Privacy stays; Security + Approvals moved to Developer & Diagnostics. - expect(screen.getByTestId('settings-nav-privacy')).toBeInTheDocument(); + // Privacy is reached via the Account hub; Security + Approvals live under + // Developer & Diagnostics. None of them are top-level home rows. + expect(screen.queryByTestId('settings-nav-privacy')).not.toBeInTheDocument(); expect(screen.queryByTestId('settings-nav-security')).not.toBeInTheDocument(); expect(screen.queryByTestId('settings-nav-approval-history')).not.toBeInTheDocument(); }); @@ -236,14 +237,6 @@ describe('SettingsHome', () => { expect(mockNavigateToSettings).toHaveBeenCalledWith('mascot'); }); - it('navigates to privacy when Privacy is clicked', async () => { - const user = userEvent.setup(); - renderSettingsHome(); - - await user.click(screen.getByTestId('settings-nav-privacy')); - expect(mockNavigateToSettings).toHaveBeenCalledWith('privacy'); - }); - it('navigates to notifications-hub when Notifications is clicked', async () => { const user = userEvent.setup(); renderSettingsHome(); diff --git a/app/src/components/settings/components/SettingsHeader.tsx b/app/src/components/settings/components/SettingsHeader.tsx index 0431c7573..a64984a27 100644 --- a/app/src/components/settings/components/SettingsHeader.tsx +++ b/app/src/components/settings/components/SettingsHeader.tsx @@ -1,3 +1,5 @@ +import type { ReactNode } from 'react'; + import { useT } from '../../../lib/i18n/I18nContext'; interface BreadcrumbItem { @@ -11,6 +13,13 @@ interface SettingsHeaderProps { showBackButton?: boolean; onBack?: () => void; breadcrumbs?: BreadcrumbItem[]; + /** + * Optional right-aligned action (e.g. a refresh or pair-device button). + * Rendered at the end of the header row so panels keep the canonical + * "SettingsHeader as first child" structure instead of wrapping the header + * in an ad-hoc flex row. + */ + action?: ReactNode; } const SettingsHeader = ({ @@ -19,73 +28,78 @@ const SettingsHeader = ({ showBackButton = false, onBack, breadcrumbs, + action, }: SettingsHeaderProps) => { const { t } = useT(); return (
-
- {/* Back button */} - {showBackButton && onBack && ( - - )} +
+
+ {/* Back button */} + {showBackButton && onBack && ( + + )} - {/* Breadcrumbs */} - {breadcrumbs && breadcrumbs.length > 0 && ( - - )} + {/* Breadcrumbs */} + {breadcrumbs && breadcrumbs.length > 0 && ( + + )} - {/* Title */} -

- {title ?? t('nav.settings')} -

+ {/* Title */} +

+ {title ?? t('nav.settings')} +

+
+ + {action &&
{action}
}
); diff --git a/app/src/components/settings/hooks/useSettingsNavigation.ts b/app/src/components/settings/hooks/useSettingsNavigation.ts index f597ceda7..2011b6c44 100644 --- a/app/src/components/settings/hooks/useSettingsNavigation.ts +++ b/app/src/components/settings/hooks/useSettingsNavigation.ts @@ -51,8 +51,22 @@ export type SettingsRoute = | 'dev-workflow' | 'sandbox-settings' | 'permissions' + | 'activity-level' | 'devices' - | 'heartbeat'; + | 'heartbeat' + | 'security' + | 'migration' + | 'companion' + | 'embeddings' + | 'ledger-usage' + | 'cost-dashboard' + | 'search' + | 'skills-runner' + | 'event-log' + | 'model-health' + | 'analysis-views' + | 'tool-policy-diagnostics' + | 'about'; export interface BreadcrumbItem { label: string; @@ -145,12 +159,28 @@ export const useSettingsNavigation = (): SettingsNavigationHook => { // shorter `agents` (the manage-agents registry panel) so it isn't swallowed. if (path.includes('/settings/agents-settings')) return 'agents-settings'; if (path.includes('/settings/sandbox-settings')) return 'sandbox-settings'; + if (path.includes('/settings/activity-level')) return 'activity-level'; if (path.includes('/settings/permissions')) return 'permissions'; if (path.includes('/settings/agent-access')) return 'agent-access'; if (path.includes('/settings/agents')) return 'agents'; if (path.includes('/settings/mcp-server')) return 'mcp-server'; if (path.includes('/settings/dev-workflow')) return 'dev-workflow'; if (path.includes('/settings/heartbeat')) return 'heartbeat'; + // `tool-policy-diagnostics` must precede the shorter `tools` check above is + // unaffected (distinct prefix), but keep it explicit here for clarity. + if (path.includes('/settings/tool-policy-diagnostics')) return 'tool-policy-diagnostics'; + if (path.includes('/settings/security')) return 'security'; + if (path.includes('/settings/migration')) return 'migration'; + if (path.includes('/settings/companion')) return 'companion'; + if (path.includes('/settings/embeddings')) return 'embeddings'; + if (path.includes('/settings/ledger-usage')) return 'ledger-usage'; + if (path.includes('/settings/cost-dashboard')) return 'cost-dashboard'; + if (path.includes('/settings/skills-runner')) return 'skills-runner'; + if (path.includes('/settings/event-log')) return 'event-log'; + if (path.includes('/settings/model-health')) return 'model-health'; + if (path.includes('/settings/analysis-views')) return 'analysis-views'; + if (path.includes('/settings/search')) return 'search'; + if (path.includes('/settings/about')) return 'about'; return 'home'; }; @@ -241,6 +271,7 @@ export const useSettingsNavigation = (): SettingsNavigationHook => { case 'agents': case 'agent-access': case 'sandbox-settings': + case 'activity-level': case 'autonomy': case 'persona': return [settingsCrumb, agentsCrumb]; @@ -253,6 +284,8 @@ export const useSettingsNavigation = (): SettingsNavigationHook => { // Leaf panels under account case 'team': case 'privacy': + case 'security': + case 'migration': return [settingsCrumb, accountCrumb]; case 'billing': @@ -263,11 +296,15 @@ export const useSettingsNavigation = (): SettingsNavigationHook => { case 'autocomplete': case 'messaging': case 'tools': + case 'companion': return [settingsCrumb, featuresCrumb]; // Leaf panels under AI case 'voice': case 'llm': + case 'embeddings': + case 'ledger-usage': + case 'cost-dashboard': return [settingsCrumb, aiCrumb]; // Team sub-pages @@ -293,6 +330,12 @@ export const useSettingsNavigation = (): SettingsNavigationHook => { case 'mcp-server': case 'dev-workflow': case 'heartbeat': + case 'search': + case 'skills-runner': + case 'event-log': + case 'model-health': + case 'analysis-views': + case 'tool-policy-diagnostics': case 'notifications-hub': // Notifications hub section page lives under Advanced. return [settingsCrumb, developerCrumb]; @@ -308,6 +351,11 @@ export const useSettingsNavigation = (): SettingsNavigationHook => { case 'devices': return [settingsCrumb]; + // About sits at the top level of Settings (and hosts the Developer Mode + // toggle), so its trail is just Settings. + case 'about': + return [settingsCrumb]; + // Data Sync is a top-level leaf in the Account group (#3301). case 'memory-sync': return [settingsCrumb]; diff --git a/app/src/components/settings/panels/AgentAccessPanel.tsx b/app/src/components/settings/panels/AgentAccessPanel.tsx index 7c3266bbb..2ae213b1c 100644 --- a/app/src/components/settings/panels/AgentAccessPanel.tsx +++ b/app/src/components/settings/panels/AgentAccessPanel.tsx @@ -220,7 +220,7 @@ const AgentAccessPanel = () => { }; return ( -
+
({ + useSettingsNavigation: () => ({ + navigateBack, + breadcrumbs: [{ label: 'Settings' }, { label: 'Agents' }], + }), +})); + +// Mock SettingsHeader so the test does not depend on i18n resolution of the +// header title (which varies with the active locale / brand rename). We assert +// the panel wiring instead: that the shared header renders with a back button +// and that the level options drive the RPC. +vi.mock('../components/SettingsHeader', () => ({ + default: ({ title, onBack }: { title: string; onBack?: () => void }) => ( +
+ {title} + +
+ ), +})); + +const callCoreRpc = vi.fn(); +vi.mock('../../../services/coreRpcClient', () => ({ + callCoreRpc: (arg: { method: string; params: unknown }) => callCoreRpc(arg), +})); + +function settingsResult(level = 2) { + return { + result: { + level, + level_label: 'moderate', + sync_interval_secs: 3600, + heartbeat_enabled: true, + subconscious_enabled: true, + token_budget_per_cycle: null, + estimated_monthly_cost_min_usd: 1, + estimated_monthly_cost_max_usd: 5, + }, + }; +} + +const costResult = { result: { month: '2026-06', total_cost_usd: 0, total_syncs: 0 } }; + +/** All buttons except the mocked SettingsHeader back button = the level options. */ +function levelButtons() { + return screen + .getAllByRole('button') + .filter(b => b.getAttribute('data-testid') !== 'settings-header-back'); +} + +beforeEach(() => { + vi.clearAllMocks(); + callCoreRpc.mockImplementation((arg: { method: string }) => { + switch (arg.method) { + case 'openhuman.config_get_activity_level_settings': + return Promise.resolve(settingsResult()); + case 'openhuman.memory_sources_monthly_cost_summary': + return Promise.resolve(costResult); + case 'openhuman.config_update_activity_level_settings': + return Promise.resolve(settingsResult(4)); + default: + return Promise.reject(new Error(`unexpected method ${arg.method}`)); + } + }); +}); + +describe('', () => { + it('renders the shared SettingsHeader and the five level options once loaded', async () => { + render(); + + // Header only renders after the initial load resolves (loading state has no + // header), so this also asserts the panel left the loading state. + await screen.findByTestId('settings-header'); + expect(levelButtons()).toHaveLength(5); + }); + + it('invokes the back handler from the SettingsHeader', async () => { + render(); + await screen.findByTestId('settings-header'); + + fireEvent.click(screen.getByTestId('settings-header-back')); + expect(navigateBack).toHaveBeenCalledTimes(1); + }); + + it('persists a new level selection via the update RPC', async () => { + render(); + await screen.findByTestId('settings-header'); + + // The last option is "Always-on" (level 4 -> api key "always_on"). + const options = levelButtons(); + fireEvent.click(options[options.length - 1]); + + await waitFor(() => { + expect(callCoreRpc).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'openhuman.config_update_activity_level_settings', + params: { level: 'always_on' }, + }) + ); + }); + }); +}); diff --git a/app/src/components/settings/panels/AgentActivityPanel.tsx b/app/src/components/settings/panels/AgentActivityPanel.tsx index dc8058d9c..e6ce8aba0 100644 --- a/app/src/components/settings/panels/AgentActivityPanel.tsx +++ b/app/src/components/settings/panels/AgentActivityPanel.tsx @@ -2,6 +2,8 @@ import { useCallback, useEffect, useState } from 'react'; import { useT } from '../../../lib/i18n/I18nContext'; import { callCoreRpc } from '../../../services/coreRpcClient'; +import SettingsHeader from '../components/SettingsHeader'; +import { useSettingsNavigation } from '../hooks/useSettingsNavigation'; interface ActivityLevelSettings { level: number; @@ -47,6 +49,7 @@ function getCostMax(level: number): number { export default function AgentActivityPanel() { const { t } = useT(); + const { navigateBack, breadcrumbs } = useSettingsNavigation(); const [settings, setSettings] = useState(null); const [monthlyCost, setMonthlyCost] = useState(null); const [status, setStatus] = useState('loading'); @@ -102,80 +105,87 @@ export default function AgentActivityPanel() { } return ( -
-
-

- {t('activityLevel.title')} -

-

+

+ {/* Header title intentionally reuses the menu-row label key + (settings.assistant.backgroundActivity) so the page heading always + matches the entry the user clicked — including the "Subconscious" + brand rename — instead of the internal "Agent activity level" copy. */} + +
+

{t('activityLevel.description')}

-
- {monthlyCost && monthlyCost.total_cost_usd > 0 && ( -
- - {t('activityLevel.currentMonth').replace( - '{amount}', - monthlyCost.total_cost_usd.toFixed(2) - )} - -
- )} + {monthlyCost && monthlyCost.total_cost_usd > 0 && ( +
+ + {t('activityLevel.currentMonth').replace( + '{amount}', + monthlyCost.total_cost_usd.toFixed(2) + )} + +
+ )} -
- {LEVELS.map(({ key, value }) => { - const isSelected = settings?.level === value; - const apiKey = key === 'alwaysOn' ? 'always_on' : (key as string); - const costMin = getCostMin(value); - const costMax = getCostMax(value); - return ( -
- - ); - })} -
+ + ); + })} +
-
- {status === 'saving' && ( - {t('autonomy.statusSaving')} - )} - {status === 'saved' && ( - {t('activityLevel.saved')} - )} - {error && {error}} +
+ {status === 'saving' && ( + {t('autonomy.statusSaving')} + )} + {status === 'saved' && ( + {t('activityLevel.saved')} + )} + {error && {error}} +
); diff --git a/app/src/components/settings/panels/ApprovalHistoryPanel.tsx b/app/src/components/settings/panels/ApprovalHistoryPanel.tsx index beac6d4a7..a084d60ca 100644 --- a/app/src/components/settings/panels/ApprovalHistoryPanel.tsx +++ b/app/src/components/settings/panels/ApprovalHistoryPanel.tsx @@ -82,7 +82,7 @@ const ApprovalHistoryPanel = () => { }; return ( -
+
({ }), })); -vi.mock('../components/PageBackButton', () => ({ - default: ({ label, onClick }: { label: string; onClick: () => void }) => ( - - ), -})); - const openUrlMock = vi.fn(); vi.mock('../../../utils/openUrl', () => ({ openUrl: (url: string) => openUrlMock(url) })); @@ -83,7 +75,9 @@ describe('', () => { render(); await waitFor(() => expect(openUrlMock).toHaveBeenCalledTimes(1)); - fireEvent.click(screen.getByTestId('page-back-button')); + // The SettingsHeader back button (aria-label "Back") and the inline + // "Back to settings" button both route through navigateBack. + fireEvent.click(screen.getByRole('button', { name: 'Back' })); fireEvent.click(screen.getByRole('button', { name: 'Back to settings' })); expect(navigateBack).toHaveBeenCalledTimes(2); }); diff --git a/app/src/components/settings/panels/BillingPanel.tsx b/app/src/components/settings/panels/BillingPanel.tsx index 88ccf1642..d1b70870c 100644 --- a/app/src/components/settings/panels/BillingPanel.tsx +++ b/app/src/components/settings/panels/BillingPanel.tsx @@ -4,7 +4,7 @@ import { useEffect, useState } from 'react'; import { useT } from '../../../lib/i18n/I18nContext'; import { BILLING_DASHBOARD_URL } from '../../../utils/links'; import { openUrl } from '../../../utils/openUrl'; -import PageBackButton from '../components/PageBackButton'; +import SettingsHeader from '../components/SettingsHeader'; import { useSettingsNavigation } from '../hooks/useSettingsNavigation'; const log = createDebug('openhuman:billing-panel'); @@ -40,75 +40,60 @@ const BillingPanel = () => { }, []); return ( -
-
- 0 ? ( -
- {breadcrumbs.map((crumb, index) => ( - - ))} -
- ) : null - } - /> +
+ -
-
-
-

- {t('settings.billing.movedToWeb')} -

-

- {t('settings.billing.openDashboard')} -

-

- {t('settings.billing.movedToWebDesc')} -

-
- -
- - -
- - {status === 'opening' && ( -

- {t('settings.billing.openingBrowser')} -

- )} - {status === 'idle' && ( -

- {t('settings.billing.browserNotOpen')} -

- )} - {status === 'error' && ( -

- {t('settings.billing.browserOpenFailed')} -

- )} +
+
+
+

+ {t('settings.billing.movedToWeb')} +

+

+ {t('settings.billing.openDashboard')} +

+

+ {t('settings.billing.movedToWebDesc')} +

+ +
+ + +
+ + {status === 'opening' && ( +

+ {t('settings.billing.openingBrowser')} +

+ )} + {status === 'idle' && ( +

+ {t('settings.billing.browserNotOpen')} +

+ )} + {status === 'error' && ( +

+ {t('settings.billing.browserOpenFailed')} +

+ )}
diff --git a/app/src/components/settings/panels/ComposioTriagePanel.tsx b/app/src/components/settings/panels/ComposioTriagePanel.tsx index fa5f4f5b9..2300878af 100644 --- a/app/src/components/settings/panels/ComposioTriagePanel.tsx +++ b/app/src/components/settings/panels/ComposioTriagePanel.tsx @@ -75,7 +75,7 @@ const ComposioTriagePanel = () => { if (loading) { return ( -
+
{ } return ( -
+
{ return (
-
- 0} - onBack={navigateBack} - breadcrumbs={breadcrumbs} - /> - -
+ 0} + onBack={navigateBack} + breadcrumbs={breadcrumbs} + action={ + + } + />
diff --git a/app/src/components/settings/panels/PermissionsPanel.tsx b/app/src/components/settings/panels/PermissionsPanel.tsx index 5068c84e2..a9ef83a38 100644 --- a/app/src/components/settings/panels/PermissionsPanel.tsx +++ b/app/src/components/settings/panels/PermissionsPanel.tsx @@ -187,7 +187,7 @@ const PermissionsPanel = () => { }; return ( -
+
{ }; return ( -
+
{ const canSave = mode === 'generate' ? confirmed : isImportComplete; return ( -
+
{ if (!isTauri()) { return ( -
+
{ if (isLoading) { return ( -
+
{ } return ( -
+
{ }; return ( -
+
{ }; return ( -
+
{ if (!teamEntry) { return ( -
+
{ if (!isAdmin) { return ( -
+
{ const { team } = teamEntry; return ( -
+
{ }; return ( -
+
{ }; return ( -
+
{ }; return ( -
+
{ }; return ( -
-
- - -
+
+ void loadBalances()} + disabled={loading} + aria-label={t('walletBalances.refresh')} + className="flex items-center gap-1.5 px-3 py-1.5 text-xs font-medium text-primary-600 dark:text-primary-400 hover:text-primary-700 dark:hover:text-primary-300 disabled:opacity-50 transition-colors"> + + + + {t('walletBalances.refresh')} + + } + />
{renderContent()} diff --git a/app/src/pages/Settings.tsx b/app/src/pages/Settings.tsx index 383231457..8ed6fc491 100644 --- a/app/src/pages/Settings.tsx +++ b/app/src/pages/Settings.tsx @@ -578,8 +578,7 @@ const Settings = () => { /> )} /> )} /> - {/* BillingPanel intentionally uses its own wider layout. */} - } /> + )} /> )} /> )} /> )} /> @@ -660,8 +659,14 @@ const Settings = () => { path="analysis-views" element={wrapSettingsPage(, { maxWidthClass: 'max-w-4xl' })} /> - } /> - } /> + , { maxWidthClass: 'max-w-4xl' })} + /> + , { maxWidthClass: 'max-w-4xl' })} + /> )} /> )} /> {/* Mobile devices */}