diff --git a/surface/web/src/components/SettingsSurface.test.tsx b/surface/web/src/components/SettingsSurface.test.tsx index 6a070846..f3e1830d 100644 --- a/surface/web/src/components/SettingsSurface.test.tsx +++ b/surface/web/src/components/SettingsSurface.test.tsx @@ -65,6 +65,21 @@ describe('SettingsSurface sections', () => { expect(activeSectionLabel()).toBe('Connections'); }); + it('renders only the active section', async () => { + renderSettings(); + + // Appearance is the default section; its controls are present and the other + // sections' controls are not rendered at all. + expect(screen.getByRole('heading', { name: 'Appearance' })).toBeTruthy(); + expect(screen.queryByRole('combobox', { name: 'Message notifications' })).toBeNull(); + + fireEvent.click(screen.getByRole('button', { name: 'Notifications' })); + + await waitFor(() => expect(screen.getByRole('heading', { name: 'Notifications' })).toBeTruthy()); + expect(screen.queryByRole('button', { name: 'High contrast' })).toBeNull(); + expect(screen.getByRole('combobox', { name: 'Message notifications' })).toBeTruthy(); + }); + it('updates the active section on back navigation', async () => { renderSettings(); diff --git a/surface/web/src/components/SettingsSurface.tsx b/surface/web/src/components/SettingsSurface.tsx index d342e3b1..8dc5c316 100644 --- a/surface/web/src/components/SettingsSurface.tsx +++ b/surface/web/src/components/SettingsSurface.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState, type ReactNode } from 'react'; +import { useState, type ReactNode } from 'react'; import { ACCENTS, FONT_SCALES, @@ -74,6 +74,8 @@ const SETTINGS_SECTIONS = [ type SettingsSectionSlug = (typeof SETTINGS_SECTIONS)[number]['slug']; +const SECTION_HEADING_ID = 'settings-section-heading'; + const DEFAULT_SETTINGS_SECTION: SettingsSectionSlug = SETTINGS_SECTIONS[0].slug; function isSettingsSectionSlug(value: string | null | undefined): value is SettingsSectionSlug { @@ -108,19 +110,17 @@ export function SettingsSurface({

Settings

-
- -
+ ); } @@ -151,8 +151,7 @@ function SettingsControls({ const { prefs, setPrefs } = useTheme(); const requestedSection = route?.surface === 'settings' ? route.settingsSection : null; const activeSection = resolveSettingsSection(requestedSection); - const shouldScrollToSection = route?.surface === 'settings'; - const sectionRefs = useRef>>({}); + const activeLabel = SETTINGS_SECTIONS.find((section) => section.slug === activeSection)!.label; const segmentButton = (active: boolean) => `h-8 flex-1 rounded px-2 text-xs font-medium max-md:h-11 max-md:px-3 max-md:text-sm ${ active ? 'bg-accent text-on-accent' : 'text-fg-tertiary hover:bg-surface-overlay hover:text-fg-body' @@ -175,56 +174,46 @@ function SettingsControls({ setPrefs({ notifications: { ...prefs.notifications, sessions } }); const setNotificationCalls = (calls: boolean) => setPrefs({ notifications: { ...prefs.notifications, calls } }); const notificationsDisabled = notify === 'denied' || notify === 'unsupported'; - const setSectionRef = (section: SettingsSectionSlug) => (element: HTMLElement | null) => { - if (element) sectionRefs.current[section] = element; - else delete sectionRefs.current[section]; - }; const openSection = (section: SettingsSectionSlug) => { navigate(`/settings/${section}`); }; - useEffect(() => { - if (!shouldScrollToSection) return; - sectionRefs.current[activeSection]?.scrollIntoView?.({ block: 'start' }); - }, [activeSection, shouldScrollToSection]); - return ( -
-
- - -
-
-
-
+ + +
+
+

+ {activeLabel} +

+
+ {activeSection === 'appearance' ? ( + <>
{THEME_OPTIONS.map((option) => ( @@ -307,15 +296,11 @@ function SettingsControls({ ))}
-
- - + + ) : null} -
+ {activeSection === 'notifications' ? ( + <> -
- - + + ) : null} -
+ {activeSection === 'connections' ? ( + <> -
- - + + ) : null} -
+ {activeSection === 'agents' ? ( + <> -
- - - -
-
- Atrium is AGPL-3.0-or-later.{' '} - - Source - - {' · '} - - License - -
-
-
-
+ + ) : null} + + {activeSection === 'about' ? ( +
+ Atrium is AGPL-3.0-or-later.{' '} + + Source + + {' · '} + + License + +
+ ) : null} +
); } -function SettingsSectionDivider() { - return
; -} - function SettingRow({ label, children }: { label: string; children: ReactNode }) { return (
diff --git a/surface/web/test/notificationSettings.test.tsx b/surface/web/test/notificationSettings.test.tsx index c761a36e..327d3584 100644 --- a/surface/web/test/notificationSettings.test.tsx +++ b/surface/web/test/notificationSettings.test.tsx @@ -24,7 +24,13 @@ vi.mock('../src/notify', () => ({ toggleNotifications: vi.fn(async () => 'on'), })); -function renderSettings(props: Partial[0]> = {}) { +// Settings renders one section at a time, keyed off /settings/:section, so each +// test has to open the section holding the controls it asserts on. +function renderSettings( + section: 'appearance' | 'notifications' | 'connections' | 'agents' | 'about', + props: Partial[0]> = {}, +) { + window.history.replaceState(null, '', `/settings/${section}`); render( [0]> = } beforeEach(() => { + window.history.replaceState(null, '', '/settings'); mockTheme.prefs = { ...DEFAULT_PREFS, notifications: { ...DEFAULT_PREFS.notifications } }; mockTheme.setPrefs.mockClear(); }); @@ -44,9 +51,9 @@ afterEach(cleanup); describe('notification settings', () => { it('renders device and per-type notification controls', () => { - renderSettings(); + renderSettings('notifications'); - expect(screen.getByText('Notifications')).toBeTruthy(); + expect(screen.getByRole('heading', { name: 'Notifications' })).toBeTruthy(); expect(screen.getByRole('button', { name: /Device notifications off/ })).toBeTruthy(); expect(screen.getByRole('combobox', { name: 'Message notifications' })).toHaveProperty('value', 'dm_mention'); expect(screen.getByRole('button', { name: 'Agent notifications' })).toBeTruthy(); @@ -54,7 +61,7 @@ describe('notification settings', () => { }); it('writes complete notification preference objects', () => { - renderSettings(); + renderSettings('notifications'); fireEvent.change(screen.getByRole('combobox', { name: 'Message notifications' }), { target: { value: 'all' }, @@ -75,19 +82,21 @@ describe('notification settings', () => { }); it('shows unavailable GitHub connection state without breaking provider settings', () => { - renderSettings({ connectionsAvailable: false }); - + renderSettings('connections', { connectionsAvailable: false }); expect(screen.getByRole('button', { name: /Unavailable/ })).toBeTruthy(); + cleanup(); + + renderSettings('agents', { connectionsAvailable: false }); expect(screen.getByText('Claude Code')).toBeTruthy(); expect(screen.getByText('Codex')).toBeTruthy(); }); it('shows explicit GitHub fallback and needs-auth states', () => { - renderSettings({ connectionsAvailable: true }); + renderSettings('connections', { connectionsAvailable: true }); expect(screen.getByRole('button', { name: /Public read/ })).toBeTruthy(); cleanup(); - renderSettings({ + renderSettings('connections', { connectionsAvailable: true, githubConnection: { id: 'github:app_installation', @@ -130,7 +139,7 @@ describe('notification settings', () => { updatedAt: null, }; - renderSettings({ githubConnection }); + renderSettings('connections', { githubConnection }); expect(screen.getByRole('button', { name: /acme · App/ })).toBeTruthy(); });