Skip to content

Commit ad4fb35

Browse files
GCWingGCWing
andauthored
fix(settings): stabilize loading and reuse device-scoped snapshots (#3329)
* fix(settings): stabilize loading and isolate warm snapshots * fix(settings): complete hook dependencies for cached settings --------- Co-authored-by: GCWing <gcwing@foxmail.com>
1 parent fcf11fe commit ad4fb35

67 files changed

Lines changed: 1422 additions & 417 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/web-ui/src/app/scenes/settings/AGENTS.md‎

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

3038
## Focused verification
3139

‎src/web-ui/src/app/scenes/settings/SettingsNav.test.tsx‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ vi.mock('@/shared/utils/motionPreference', () => ({
4646
}));
4747

4848
import SettingsNav from './SettingsNav';
49+
import { preloadSettingsPage } from './settingsRegistry';
4950
import { useSettingsStore } from './settingsStore';
5051
import {
5152
registerSettingsDraft,
@@ -58,6 +59,7 @@ describe('SettingsNav shared component composition', () => {
5859

5960
beforeEach(() => {
6061
vi.useFakeTimers();
62+
vi.mocked(preloadSettingsPage).mockReset().mockResolvedValue(undefined);
6163
resetSettingsDraftRegistryForTests();
6264
useSettingsStore.setState(useSettingsStore.getInitialState());
6365
container = document.createElement('div');
@@ -183,4 +185,35 @@ describe('SettingsNav shared component composition', () => {
183185
expect(useSettingsStore.getState().activePageId).toBe('application.pet');
184186
expect(useSettingsStore.getState().activeSectionId).toBeNull();
185187
});
188+
it('marks a slow destination pending before showing its loading page', async () => {
189+
vi.mocked(preloadSettingsPage).mockReturnValue(new Promise(() => {}));
190+
const item = container.querySelector('[data-settings-page="application.appearance"]') as HTMLButtonElement;
191+
await act(async () => item.click());
192+
expect(item.getAttribute('aria-busy')).toBe('true');
193+
expect(useSettingsStore.getState().activePageId).toBe('application.general');
194+
await act(async () => vi.advanceTimersByTimeAsync(180));
195+
expect(useSettingsStore.getState().activePageId).toBe('application.appearance');
196+
expect(item.hasAttribute('aria-busy')).toBe(false);
197+
});
198+
199+
it('ignores an earlier click when its preload completes after the latest selection', async () => {
200+
let finishOld!: () => void;
201+
vi.mocked(preloadSettingsPage).mockImplementation(pageId => pageId === 'application.appearance'
202+
? new Promise<void>(resolve => { finishOld = resolve; }) : Promise.resolve());
203+
await act(async () => (container.querySelector('[data-settings-page="application.appearance"]') as HTMLButtonElement).click());
204+
await act(async () => (container.querySelector('[data-settings-page="application.pet"]') as HTMLButtonElement).click());
205+
await act(async () => finishOld());
206+
await act(async () => vi.advanceTimersByTimeAsync(180));
207+
expect(useSettingsStore.getState().activePageId).toBe('application.pet');
208+
});
209+
210+
it('does not override a newer programmatic destination with a pending click', async () => {
211+
let finish!: () => void;
212+
vi.mocked(preloadSettingsPage).mockReturnValue(new Promise<void>(resolve => { finish = resolve; }));
213+
await act(async () => (container.querySelector('[data-settings-page="application.appearance"]') as HTMLButtonElement).click());
214+
await act(async () => useSettingsStore.getState().openPage('application.pet'));
215+
await act(async () => finish());
216+
expect(useSettingsStore.getState().activePageId).toBe('application.pet');
217+
});
218+
186219
});

‎src/web-ui/src/app/scenes/settings/SettingsNav.tsx‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useSettingsDraftSnapshot } from '@/infrastructure/config/settingsDraftRegistry';
2+
import { getActiveSurfaceScope } from '@/infrastructure/peer-device/deviceSurface';
23
import { useI18n } from '@/infrastructure/i18n/hooks/useI18n';
34
import { getInteractionMotion } from '@/shared/utils/motionPreference';
45
import {
@@ -11,10 +12,10 @@ import {
1112
NavigationPanelSection,
1213
OverflowText,
1314
SearchField,
15+
Spinner,
1416
} from '@openbitfun/ui';
1517
import type { i18n as I18nApi } from 'i18next';
1618
import React, {
17-
startTransition,
1819
useCallback,
1920
useEffect,
2021
useMemo,
@@ -138,6 +139,13 @@ const SettingsNav: React.FC = () => {
138139
const searchInputRef = useRef<HTMLInputElement>(null);
139140
const resultsRef = useRef<HTMLDivElement>(null);
140141
const activationRequestRef = useRef(0);
142+
const [pendingPageId, setPendingPageId] = useState<SettingsPageId | null>(null);
143+
const activationTimerRef = useRef<number | undefined>();
144+
145+
useEffect(() => () => {
146+
activationRequestRef.current += 1;
147+
window.clearTimeout(activationTimerRef.current);
148+
}, []);
141149

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

187195
const activate = useCallback((destination: SettingsDestination, clear = false) => {
188196
const requestId = ++activationRequestRef.current;
197+
const scope = getActiveSurfaceScope();
198+
const navigationRequestId = useSettingsStore.getState().navigationRequestId;
189199
const motion = getInteractionMotion();
200+
window.clearTimeout(activationTimerRef.current);
201+
setPendingPageId(destination.pageId);
202+
let committed = false;
190203
const commit = () => {
191-
if (requestId !== activationRequestRef.current) return;
192-
startTransition(() => {
193-
openDestination(destination, motion);
194-
if (clear) clearSearch();
195-
});
204+
if (committed || requestId !== activationRequestRef.current) return;
205+
if (!scope.isCurrent() || useSettingsStore.getState().navigationRequestId !== navigationRequestId) {
206+
window.clearTimeout(activationTimerRef.current);
207+
setPendingPageId(null);
208+
return;
209+
}
210+
committed = true;
211+
window.clearTimeout(activationTimerRef.current);
212+
setPendingPageId(null);
213+
openDestination(destination, motion);
214+
if (clear) clearSearch();
196215
};
216+
// Keep the current page during fast preparation; slow loads get a titled skeleton.
217+
activationTimerRef.current = window.setTimeout(commit, 180);
197218
void preloadSettingsPage(destination.pageId).then(commit, commit);
198219
}, [clearSearch, openDestination]);
199220

@@ -301,6 +322,7 @@ const SettingsNav: React.FC = () => {
301322
id={`settings-nav-result-${index}`}
302323
role="option"
303324
aria-selected={active}
325+
aria-busy={pendingPageId === row.destination.pageId || undefined}
304326
selected={active}
305327
data-openbitfun-component="settings-nav"
306328
data-openbitfun-part="searchResult"
@@ -325,7 +347,7 @@ const SettingsNav: React.FC = () => {
325347
{highlightFirstMatch(row.description, searchQuery)}
326348
</OverflowText>
327349
</span>
328-
{dirtyMarker(row.destination.pageId)}
350+
{pendingPageId === row.destination.pageId ? <Spinner size="sm" /> : dirtyMarker(row.destination.pageId)}
329351
</NavigationPanelItem>
330352
);
331353
})}
@@ -362,12 +384,13 @@ const SettingsNav: React.FC = () => {
362384
data-openbitfun-state={activePageId === page.id ? 'active' : undefined}
363385
className="openbitfun-settings-nav__item"
364386
selected={activePageId === page.id}
387+
aria-busy={pendingPageId === page.id || undefined}
365388
onClick={() => activate({ pageId: page.id })}
366389
onPointerEnter={() => preload(page.id)}
367390
onFocus={() => preload(page.id)}
368391
>
369392
<OverflowText className="openbitfun-settings-nav__item-label">{t(page.labelKey)}</OverflowText>
370-
{dirtyMarker(page.id)}
393+
{pendingPageId === page.id ? <Spinner size="sm" /> : dirtyMarker(page.id)}
371394
</NavigationPanelItem>
372395
))}
373396
</div>

‎src/web-ui/src/app/scenes/settings/SettingsScene.scss‎

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -9,57 +9,6 @@
99
height: 100%;
1010
overflow: hidden;
1111

12-
&__loading {
13-
flex: 1;
14-
padding: var(--openbitfun-space-6);
15-
}
16-
17-
&__loading-line,
18-
&__loading-block {
19-
border-radius: var(--openbitfun-radius-md);
20-
background: var(--openbitfun-color-action-quiet-hover);
21-
opacity: 0.65;
22-
}
23-
24-
&__loading-line {
25-
height: 14px;
26-
max-width: 360px;
27-
margin-bottom: var(--openbitfun-space-3);
28-
29-
&--title {
30-
height: 20px;
31-
max-width: 220px;
32-
margin-bottom: var(--openbitfun-space-5);
33-
}
34-
}
35-
36-
&__loading-block {
37-
height: 160px;
38-
max-width: 720px;
39-
margin-top: var(--openbitfun-space-6);
40-
}
41-
42-
&__content-stack {
43-
flex: 1;
44-
min-width: 0;
45-
min-height: 0;
46-
overflow: hidden;
47-
}
48-
49-
&__content-transition {
50-
flex: 1 1 auto;
51-
width: 100%;
52-
height: 100%;
53-
min-width: 0;
54-
min-height: 0;
55-
overflow: hidden;
56-
57-
> .openbitfun-view-transition-boundary__view {
58-
width: 100%;
59-
height: 100%;
60-
}
61-
}
62-
6312
&__content-wrapper {
6413
display: flex;
6514
flex-direction: column;

‎src/web-ui/src/app/scenes/settings/SettingsScene.test.tsx‎

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,19 @@ import React, { act } from 'react';
44
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
55
import { createRoot, type Root } from 'react-dom/client';
66

7+
const preparation = vi.hoisted(() => ({
8+
ready: vi.fn(() => true),
9+
preload: vi.fn(async (_pageId: string) => undefined),
10+
loading: vi.fn(),
11+
}));
12+
vi.mock('./pages/shared/SettingsPage', () => ({
13+
SettingsPage: ({ pageId, children }: { pageId: string; children: React.ReactNode }) => <section><h2>{pageId}</h2>{children}</section>,
14+
}));
15+
vi.mock('@/infrastructure/config/components/common', () => ({
16+
ConfigLoadingState: () => { preparation.loading(); return <div data-testid="pending-page" />; },
17+
ConfigRetryState: ({ onRetry }: { onRetry: () => void }) => <button data-testid="retry-page" onClick={onRetry}>Retry</button>,
18+
}));
19+
720
vi.mock('./settingsRegistry', () => {
821
const pages = {
922
'application.general': {
@@ -33,12 +46,13 @@ vi.mock('./settingsRegistry', () => {
3346
DEFAULT_SETTINGS_PAGE_ID: 'application.general',
3447
getSettingsPageManifest: (pageId: keyof typeof pages) => pages[pageId] ?? pages['application.general'],
3548
isSettingsPageId: (value: string) => value in pages,
36-
isSettingsPageReady: () => true,
37-
preloadSettingsPage: vi.fn(async () => undefined),
49+
isSettingsPageReady: preparation.ready,
50+
preloadSettingsPage: preparation.preload,
3851
};
3952
});
4053

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

5266
beforeEach(() => {
67+
preparation.ready.mockReset().mockReturnValue(true);
68+
preparation.preload.mockReset().mockResolvedValue(undefined);
69+
preparation.loading.mockClear();
5370
resetSettingsDraftRegistryForTests();
5471
container = document.createElement('div');
5572
document.body.appendChild(container);
@@ -135,4 +152,49 @@ describe('SettingsScene canonical page routing', () => {
135152
expect(save).toHaveBeenCalledOnce();
136153
expect(useSettingsStore.getState().activePageId).toBe('application.appearance');
137154
});
155+
it('never mounts loading UI while switching between prepared pages', async () => {
156+
await act(async () => root.render(<SettingsScene />));
157+
const frame = container.querySelector('.openbitfun-settings-scene__content-wrapper');
158+
await act(async () => useSettingsStore.getState().openPage('application.appearance', 'pointer'));
159+
await act(async () => useSettingsStore.getState().openPage('application.general', 'pointer'));
160+
expect(preparation.loading).not.toHaveBeenCalled();
161+
expect(container.querySelector('.openbitfun-settings-scene__content-wrapper')).toBe(frame);
162+
expect(container.querySelectorAll('[data-testid="settings-scene-content"]')).toHaveLength(1);
163+
});
164+
165+
it('keeps the latest destination when cold imports complete out of order', async () => {
166+
let finishAppearance!: () => void;
167+
let finishAutomation!: () => void;
168+
preparation.ready.mockReturnValue(false);
169+
preparation.preload.mockImplementation(pageId => new Promise<void>(resolve => {
170+
if (pageId === 'application.appearance') finishAppearance = resolve;
171+
else finishAutomation = resolve;
172+
}));
173+
useSettingsStore.getState().openPage('application.appearance');
174+
await act(async () => root.render(<SettingsScene />));
175+
await act(async () => useSettingsStore.getState().openPage('tools.automation'));
176+
await act(async () => finishAppearance());
177+
expect(container.querySelector('h2')?.textContent).toBe('tools.automation');
178+
expect(container.querySelector('[data-testid="appearance-page"]')).toBeNull();
179+
await act(async () => finishAutomation());
180+
expect(container.querySelector('[data-testid="automation-page"]')).not.toBeNull();
181+
});
182+
183+
it('shows an in-place retry when preparation fails and recovers without leaving settings', async () => {
184+
preparation.ready.mockReturnValue(false);
185+
preparation.preload.mockRejectedValueOnce(new Error('offline'));
186+
await act(async () => root.render(<SettingsScene />));
187+
expect(container.querySelector('[data-testid="retry-page"]')).not.toBeNull();
188+
expect(container.querySelector('[data-testid="general-page"]')).toBeNull();
189+
await act(async () => (container.querySelector('[data-testid="retry-page"]') as HTMLButtonElement).click());
190+
expect(container.querySelector('[data-testid="general-page"]')).not.toBeNull();
191+
});
192+
193+
it('replaces page-local state synchronously on device activation', async () => {
194+
await act(async () => root.render(<SettingsScene />));
195+
const oldPage = container.querySelector('[data-testid="general-page"]');
196+
await act(async () => activateSurface('peer-settings-test'));
197+
expect(container.querySelector('[data-testid="general-page"]')).not.toBe(oldPage);
198+
});
199+
138200
});

0 commit comments

Comments
 (0)