Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions surface/web/src/components/SettingsSurface.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
203 changes: 87 additions & 116 deletions surface/web/src/components/SettingsSurface.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useRef, useState, type ReactNode } from 'react';
import { useState, type ReactNode } from 'react';
import {
ACCENTS,
FONT_SCALES,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -108,19 +110,17 @@ export function SettingsSurface({
<div className="border-b border-edge px-4 py-3">
<h2 className="text-sm font-bold text-fg">Settings</h2>
</div>
<div className="min-h-0 flex-1">
<SettingsControls
notify={notify}
setNotify={setNotify}
githubConnection={githubConnection}
connectionsAvailable={connectionsAvailable}
claudeStatus={claudeStatus}
codexStatus={codexStatus}
onConnectGitHub={onConnectGitHub}
onConnectClaude={onConnectClaude}
onConnectCodex={onConnectCodex}
/>
</div>
<SettingsControls
notify={notify}
setNotify={setNotify}
githubConnection={githubConnection}
connectionsAvailable={connectionsAvailable}
claudeStatus={claudeStatus}
codexStatus={codexStatus}
onConnectGitHub={onConnectGitHub}
onConnectClaude={onConnectClaude}
onConnectCodex={onConnectCodex}
/>
</div>
);
}
Expand Down Expand Up @@ -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<Partial<Record<SettingsSectionSlug, HTMLElement>>>({});
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'
Expand All @@ -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 (
<div className="min-h-0 flex-1 overflow-y-auto">
<div className="flex min-h-full flex-col md:flex-row">
<nav
aria-label="Settings sections"
className="sticky top-0 z-sticky shrink-0 border-b border-edge bg-surface px-3 py-2 md:w-44 md:self-start md:border-b-0 md:border-r md:px-2 md:py-4"
>
<div className="flex gap-1 overflow-x-auto [scrollbar-width:none] md:flex-col md:overflow-visible [&::-webkit-scrollbar]:hidden">
{SETTINGS_SECTIONS.map((section) => {
const active = activeSection === section.slug;
return (
<button
key={section.slug}
type="button"
aria-current={active ? 'page' : undefined}
onClick={() => openSection(section.slug)}
className={`shrink-0 rounded px-2.5 py-1.5 text-sm font-medium transition-colors focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-1 focus-visible:outline-edge-focus md:w-full md:text-left ${
active
? 'bg-surface-raised text-fg shadow-sm'
: 'text-fg-muted hover:bg-surface-overlay hover:text-fg-secondary'
}`}
>
{section.label}
</button>
);
})}
</div>
</nav>

<div className="min-w-0 flex-1">
<div className="max-w-2xl px-4 py-4">
<div className="space-y-4">
<section
ref={setSectionRef('appearance')}
aria-label="Appearance"
className="scroll-mt-16 space-y-3 md:scroll-mt-4"
<div className="flex min-h-0 flex-1 flex-col md:flex-row">
<nav
aria-label="Settings sections"
className="shrink-0 border-b border-edge bg-surface px-3 py-2 md:w-44 md:border-b-0 md:border-r md:px-2 md:py-4"
>
<div className="flex gap-1 overflow-x-auto [scrollbar-width:none] md:flex-col md:overflow-visible [&::-webkit-scrollbar]:hidden">
{SETTINGS_SECTIONS.map((section) => {
const active = activeSection === section.slug;
return (
<button
key={section.slug}
type="button"
aria-current={active ? 'page' : undefined}
onClick={() => openSection(section.slug)}
className={`shrink-0 rounded px-2.5 py-1.5 text-sm font-medium transition-colors focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-1 focus-visible:outline-edge-focus md:w-full md:text-left ${
active
? 'bg-surface-raised text-fg shadow-sm'
: 'text-fg-muted hover:bg-surface-overlay hover:text-fg-secondary'
}`}
>
{section.label}
</button>
);
})}
</div>
</nav>

<div className="min-h-0 min-w-0 flex-1 overflow-y-auto">
<div className="max-w-2xl px-4 py-4">
<h3 id={SECTION_HEADING_ID} className="mb-3 text-sm font-semibold text-fg">
{activeLabel}
</h3>
<section aria-labelledby={SECTION_HEADING_ID} className="space-y-3">
{activeSection === 'appearance' ? (
<>
<SettingRow label="Theme">
<div className="flex rounded-md border border-edge bg-surface p-0.5">
{THEME_OPTIONS.map((option) => (
Expand Down Expand Up @@ -307,15 +296,11 @@ function SettingsControls({
))}
</div>
</SettingRow>
</section>

<SettingsSectionDivider />
</>
) : null}

<section
ref={setSectionRef('notifications')}
aria-label="Notifications"
className="scroll-mt-16 space-y-3 md:scroll-mt-4"
>
{activeSection === 'notifications' ? (
<>
<SettingRow label="Device">
<Tooltip content={BELL_TITLES[notify]}>
<button
Expand Down Expand Up @@ -376,15 +361,11 @@ function SettingsControls({
<span className="size-5 rounded-full bg-current" />
</button>
</SettingRow>
</section>

<SettingsSectionDivider />
</>
) : null}

<section
ref={setSectionRef('connections')}
aria-label="Connections"
className="scroll-mt-16 space-y-3 md:scroll-mt-4"
>
{activeSection === 'connections' ? (
<>
<SettingRow label="GitHub">
<Tooltip
content={connectionsAvailable ? 'Manage GitHub connection' : 'GitHub connections unavailable'}
Expand All @@ -411,15 +392,11 @@ function SettingsControls({
</button>
</Tooltip>
</SettingRow>
</section>

<SettingsSectionDivider />
</>
) : null}

<section
ref={setSectionRef('agents')}
aria-label="Agents"
className="scroll-mt-16 space-y-3 md:scroll-mt-4"
>
{activeSection === 'agents' ? (
<>
<SettingRow label="Claude Code">
<button type="button" onClick={onConnectClaude} className={connectionButtonClass}>
<span className={`size-2 rounded-full ${claudeStatus?.connected ? 'bg-success' : 'bg-warning'}`} />
Expand All @@ -433,44 +410,38 @@ function SettingsControls({
<span>{codexStatus?.connected ? 'Connected' : 'Connect'}</span>
</button>
</SettingRow>
</section>

<SettingsSectionDivider />

<section ref={setSectionRef('about')} aria-label="About" className="scroll-mt-16 md:scroll-mt-4">
<div className="text-2xs leading-5 text-fg-muted">
Atrium is AGPL-3.0-or-later.{' '}
<a
href={SOURCE_URL}
target="_blank"
rel="noreferrer"
className="font-medium text-accent-text hover:underline"
>
Source
</a>
{' · '}
<a
href={LICENSE_URL}
target="_blank"
rel="noreferrer"
className="font-medium text-accent-text hover:underline"
>
License
</a>
</div>
</section>
</div>
</div>
</>
) : null}

{activeSection === 'about' ? (
<div className="text-2xs leading-5 text-fg-muted">
Atrium is AGPL-3.0-or-later.{' '}
<a
href={SOURCE_URL}
target="_blank"
rel="noreferrer"
className="font-medium text-accent-text hover:underline"
>
Source
</a>
{' · '}
<a
href={LICENSE_URL}
target="_blank"
rel="noreferrer"
className="font-medium text-accent-text hover:underline"
>
License
</a>
</div>
) : null}
</section>
</div>
</div>
</div>
);
}

function SettingsSectionDivider() {
return <div className="border-t border-edge" />;
}

function SettingRow({ label, children }: { label: string; children: ReactNode }) {
return (
<div className="grid grid-cols-[5.75rem_minmax(0,1fr)] items-center gap-2 max-md:grid-cols-1 max-md:items-stretch max-md:gap-1.5">
Expand Down
27 changes: 18 additions & 9 deletions surface/web/test/notificationSettings.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ vi.mock('../src/notify', () => ({
toggleNotifications: vi.fn(async () => 'on'),
}));

function renderSettings(props: Partial<Parameters<typeof SettingsSurface>[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<Parameters<typeof SettingsSurface>[0]> = {},
) {
window.history.replaceState(null, '', `/settings/${section}`);
render(
<SettingsSurface
connectionsAvailable
Expand All @@ -36,6 +42,7 @@ function renderSettings(props: Partial<Parameters<typeof SettingsSurface>[0]> =
}

beforeEach(() => {
window.history.replaceState(null, '', '/settings');
mockTheme.prefs = { ...DEFAULT_PREFS, notifications: { ...DEFAULT_PREFS.notifications } };
mockTheme.setPrefs.mockClear();
});
Expand All @@ -44,17 +51,17 @@ 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();
expect(screen.getByRole('button', { name: 'Call notifications' })).toBeTruthy();
});

it('writes complete notification preference objects', () => {
renderSettings();
renderSettings('notifications');

fireEvent.change(screen.getByRole('combobox', { name: 'Message notifications' }), {
target: { value: 'all' },
Expand 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',
Expand Down Expand Up @@ -130,7 +139,7 @@ describe('notification settings', () => {
updatedAt: null,
};

renderSettings({ githubConnection });
renderSettings('connections', { githubConnection });

expect(screen.getByRole('button', { name: /acme · App/ })).toBeTruthy();
});
Expand Down
Loading