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
8 changes: 8 additions & 0 deletions src/web-ui/src/app/scenes/settings/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ Follow `src/web-ui/AGENTS.md` and the settings control sizing rules in
rename generated protocol IDs to match sidebar labels.
- Moving a setting must preserve its execution host, remote capability gates,
unsupported states, persistence keys, and draft registration.
- Page preparation must retain the scene frame and use `SettingsPage` for its
fallback. First-load placeholders use `ConfigLoadingState` inside the content
frame; refreshes retain successful content. Seed editable forms only from a
complete `ConfigManager` snapshot, never from default values after a failed
read. Do not hydrate over edits or keep inactive page effects alive for caching.
- Runtime snapshots and scroll positions belong to the current device activation.
Background responses must not repopulate a previous activation; section links
take precedence over restored scroll positions.

## Focused verification

Expand Down
33 changes: 33 additions & 0 deletions src/web-ui/src/app/scenes/settings/SettingsNav.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ vi.mock('@/shared/utils/motionPreference', () => ({
}));

import SettingsNav from './SettingsNav';
import { preloadSettingsPage } from './settingsRegistry';
import { useSettingsStore } from './settingsStore';
import {
registerSettingsDraft,
Expand All @@ -58,6 +59,7 @@ describe('SettingsNav shared component composition', () => {

beforeEach(() => {
vi.useFakeTimers();
vi.mocked(preloadSettingsPage).mockReset().mockResolvedValue(undefined);
resetSettingsDraftRegistryForTests();
useSettingsStore.setState(useSettingsStore.getInitialState());
container = document.createElement('div');
Expand Down Expand Up @@ -183,4 +185,35 @@ describe('SettingsNav shared component composition', () => {
expect(useSettingsStore.getState().activePageId).toBe('application.pet');
expect(useSettingsStore.getState().activeSectionId).toBeNull();
});
it('marks a slow destination pending before showing its loading page', async () => {
vi.mocked(preloadSettingsPage).mockReturnValue(new Promise(() => {}));
const item = container.querySelector('[data-settings-page="application.appearance"]') as HTMLButtonElement;
await act(async () => item.click());
expect(item.getAttribute('aria-busy')).toBe('true');
expect(useSettingsStore.getState().activePageId).toBe('application.general');
await act(async () => vi.advanceTimersByTimeAsync(180));
expect(useSettingsStore.getState().activePageId).toBe('application.appearance');
expect(item.hasAttribute('aria-busy')).toBe(false);
});

it('ignores an earlier click when its preload completes after the latest selection', async () => {
let finishOld!: () => void;
vi.mocked(preloadSettingsPage).mockImplementation(pageId => pageId === 'application.appearance'
? new Promise<void>(resolve => { finishOld = resolve; }) : Promise.resolve());
await act(async () => (container.querySelector('[data-settings-page="application.appearance"]') as HTMLButtonElement).click());
await act(async () => (container.querySelector('[data-settings-page="application.pet"]') as HTMLButtonElement).click());
await act(async () => finishOld());
await act(async () => vi.advanceTimersByTimeAsync(180));
expect(useSettingsStore.getState().activePageId).toBe('application.pet');
});

it('does not override a newer programmatic destination with a pending click', async () => {
let finish!: () => void;
vi.mocked(preloadSettingsPage).mockReturnValue(new Promise<void>(resolve => { finish = resolve; }));
await act(async () => (container.querySelector('[data-settings-page="application.appearance"]') as HTMLButtonElement).click());
await act(async () => useSettingsStore.getState().openPage('application.pet'));
await act(async () => finish());
expect(useSettingsStore.getState().activePageId).toBe('application.pet');
});

});
39 changes: 31 additions & 8 deletions src/web-ui/src/app/scenes/settings/SettingsNav.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useSettingsDraftSnapshot } from '@/infrastructure/config/settingsDraftRegistry';
import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface';
import { useI18n } from '@/infrastructure/i18n/hooks/useI18n';
import { getInteractionMotion } from '@/shared/utils/motionPreference';
import {
Expand All @@ -11,10 +12,10 @@ import {
NavigationPanelSection,
OverflowText,
SearchField,
Spinner,
} from '@openbitfun/ui';
import type { i18n as I18nApi } from 'i18next';
import React, {
startTransition,
useCallback,
useEffect,
useMemo,
Expand Down Expand Up @@ -138,6 +139,13 @@ const SettingsNav: React.FC = () => {
const searchInputRef = useRef<HTMLInputElement>(null);
const resultsRef = useRef<HTMLDivElement>(null);
const activationRequestRef = useRef(0);
const [pendingPageId, setPendingPageId] = useState<SettingsPageId | null>(null);
const activationTimerRef = useRef<number | undefined>();

useEffect(() => () => {
activationRequestRef.current += 1;
window.clearTimeout(activationTimerRef.current);
}, []);

useEffect(() => {
const timer = window.setTimeout(() => setSearchQuery(draftQuery), SEARCH_DEBOUNCE_MS);
Expand Down Expand Up @@ -186,14 +194,27 @@ const SettingsNav: React.FC = () => {

const activate = useCallback((destination: SettingsDestination, clear = false) => {
const requestId = ++activationRequestRef.current;
const scope = getActiveSurfaceScope();
const navigationRequestId = useSettingsStore.getState().navigationRequestId;
const motion = getInteractionMotion();
window.clearTimeout(activationTimerRef.current);
setPendingPageId(destination.pageId);
let committed = false;
const commit = () => {
if (requestId !== activationRequestRef.current) return;
startTransition(() => {
openDestination(destination, motion);
if (clear) clearSearch();
});
if (committed || requestId !== activationRequestRef.current) return;
if (!scope.isCurrent() || useSettingsStore.getState().navigationRequestId !== navigationRequestId) {
window.clearTimeout(activationTimerRef.current);
setPendingPageId(null);
return;
}
committed = true;
window.clearTimeout(activationTimerRef.current);
setPendingPageId(null);
openDestination(destination, motion);
if (clear) clearSearch();
};
// Keep the current page during fast preparation; slow loads get a titled skeleton.
activationTimerRef.current = window.setTimeout(commit, 180);
void preloadSettingsPage(destination.pageId).then(commit, commit);
}, [clearSearch, openDestination]);

Expand Down Expand Up @@ -301,6 +322,7 @@ const SettingsNav: React.FC = () => {
id={`settings-nav-result-${index}`}
role="option"
aria-selected={active}
aria-busy={pendingPageId === row.destination.pageId || undefined}
selected={active}
data-openbitfun-component="settings-nav"
data-openbitfun-part="searchResult"
Expand All @@ -325,7 +347,7 @@ const SettingsNav: React.FC = () => {
{highlightFirstMatch(row.description, searchQuery)}
</OverflowText>
</span>
{dirtyMarker(row.destination.pageId)}
{pendingPageId === row.destination.pageId ? <Spinner size="sm" /> : dirtyMarker(row.destination.pageId)}
</NavigationPanelItem>
);
})}
Expand Down Expand Up @@ -362,12 +384,13 @@ const SettingsNav: React.FC = () => {
data-openbitfun-state={activePageId === page.id ? 'active' : undefined}
className="openbitfun-settings-nav__item"
selected={activePageId === page.id}
aria-busy={pendingPageId === page.id || undefined}
onClick={() => activate({ pageId: page.id })}
onPointerEnter={() => preload(page.id)}
onFocus={() => preload(page.id)}
>
<OverflowText className="openbitfun-settings-nav__item-label">{t(page.labelKey)}</OverflowText>
{dirtyMarker(page.id)}
{pendingPageId === page.id ? <Spinner size="sm" /> : dirtyMarker(page.id)}
</NavigationPanelItem>
))}
</div>
Expand Down
51 changes: 0 additions & 51 deletions src/web-ui/src/app/scenes/settings/SettingsScene.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,57 +9,6 @@
height: 100%;
overflow: hidden;

&__loading {
flex: 1;
padding: var(--openbitfun-space-6);
}

&__loading-line,
&__loading-block {
border-radius: var(--openbitfun-radius-md);
background: var(--openbitfun-color-action-quiet-hover);
opacity: 0.65;
}

&__loading-line {
height: 14px;
max-width: 360px;
margin-bottom: var(--openbitfun-space-3);

&--title {
height: 20px;
max-width: 220px;
margin-bottom: var(--openbitfun-space-5);
}
}

&__loading-block {
height: 160px;
max-width: 720px;
margin-top: var(--openbitfun-space-6);
}

&__content-stack {
flex: 1;
min-width: 0;
min-height: 0;
overflow: hidden;
}

&__content-transition {
flex: 1 1 auto;
width: 100%;
height: 100%;
min-width: 0;
min-height: 0;
overflow: hidden;

> .openbitfun-view-transition-boundary__view {
width: 100%;
height: 100%;
}
}

&__content-wrapper {
display: flex;
flex-direction: column;
Expand Down
66 changes: 64 additions & 2 deletions src/web-ui/src/app/scenes/settings/SettingsScene.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@ import React, { act } from 'react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { createRoot, type Root } from 'react-dom/client';

const preparation = vi.hoisted(() => ({
ready: vi.fn(() => true),
preload: vi.fn(async (_pageId: string) => undefined),
loading: vi.fn(),
}));
vi.mock('./pages/shared/SettingsPage', () => ({
SettingsPage: ({ pageId, children }: { pageId: string; children: React.ReactNode }) => <section><h2>{pageId}</h2>{children}</section>,
}));
vi.mock('@/infrastructure/config/components/common', () => ({
ConfigLoadingState: () => { preparation.loading(); return <div data-testid="pending-page" />; },
ConfigRetryState: ({ onRetry }: { onRetry: () => void }) => <button data-testid="retry-page" onClick={onRetry}>Retry</button>,
}));

vi.mock('./settingsRegistry', () => {
const pages = {
'application.general': {
Expand Down Expand Up @@ -33,12 +46,13 @@ vi.mock('./settingsRegistry', () => {
DEFAULT_SETTINGS_PAGE_ID: 'application.general',
getSettingsPageManifest: (pageId: keyof typeof pages) => pages[pageId] ?? pages['application.general'],
isSettingsPageId: (value: string) => value in pages,
isSettingsPageReady: () => true,
preloadSettingsPage: vi.fn(async () => undefined),
isSettingsPageReady: preparation.ready,
preloadSettingsPage: preparation.preload,
};
});

import SettingsScene from './SettingsScene';
import { activateSurface } from '@/infrastructure/peer-device/deviceSurface';
import { useSettingsStore } from './settingsStore';
import {
registerSettingsDraft,
Expand All @@ -50,6 +64,9 @@ describe('SettingsScene canonical page routing', () => {
let root: Root;

beforeEach(() => {
preparation.ready.mockReset().mockReturnValue(true);
preparation.preload.mockReset().mockResolvedValue(undefined);
preparation.loading.mockClear();
resetSettingsDraftRegistryForTests();
container = document.createElement('div');
document.body.appendChild(container);
Expand Down Expand Up @@ -135,4 +152,49 @@ describe('SettingsScene canonical page routing', () => {
expect(save).toHaveBeenCalledOnce();
expect(useSettingsStore.getState().activePageId).toBe('application.appearance');
});
it('never mounts loading UI while switching between prepared pages', async () => {
await act(async () => root.render(<SettingsScene />));
const frame = container.querySelector('.openbitfun-settings-scene__content-wrapper');
await act(async () => useSettingsStore.getState().openPage('application.appearance', 'pointer'));
await act(async () => useSettingsStore.getState().openPage('application.general', 'pointer'));
expect(preparation.loading).not.toHaveBeenCalled();
expect(container.querySelector('.openbitfun-settings-scene__content-wrapper')).toBe(frame);
expect(container.querySelectorAll('[data-testid="settings-scene-content"]')).toHaveLength(1);
});

it('keeps the latest destination when cold imports complete out of order', async () => {
let finishAppearance!: () => void;
let finishAutomation!: () => void;
preparation.ready.mockReturnValue(false);
preparation.preload.mockImplementation(pageId => new Promise<void>(resolve => {
if (pageId === 'application.appearance') finishAppearance = resolve;
else finishAutomation = resolve;
}));
useSettingsStore.getState().openPage('application.appearance');
await act(async () => root.render(<SettingsScene />));
await act(async () => useSettingsStore.getState().openPage('tools.automation'));
await act(async () => finishAppearance());
expect(container.querySelector('h2')?.textContent).toBe('tools.automation');
expect(container.querySelector('[data-testid="appearance-page"]')).toBeNull();
await act(async () => finishAutomation());
expect(container.querySelector('[data-testid="automation-page"]')).not.toBeNull();
});

it('shows an in-place retry when preparation fails and recovers without leaving settings', async () => {
preparation.ready.mockReturnValue(false);
preparation.preload.mockRejectedValueOnce(new Error('offline'));
await act(async () => root.render(<SettingsScene />));
expect(container.querySelector('[data-testid="retry-page"]')).not.toBeNull();
expect(container.querySelector('[data-testid="general-page"]')).toBeNull();
await act(async () => (container.querySelector('[data-testid="retry-page"]') as HTMLButtonElement).click());
expect(container.querySelector('[data-testid="general-page"]')).not.toBeNull();
});

it('replaces page-local state synchronously on device activation', async () => {
await act(async () => root.render(<SettingsScene />));
const oldPage = container.querySelector('[data-testid="general-page"]');
await act(async () => activateSurface('peer-settings-test'));
expect(container.querySelector('[data-testid="general-page"]')).not.toBe(oldPage);
});

});
Loading
Loading