Skip to content

Commit fcad015

Browse files
authored
Merge pull request #3308 from GCWing/gcwing/settings-loading-polish-20261008
fix(settings): decouple model loading and polish settings presentation
2 parents 4f15900 + d0ea5e9 commit fcad015

31 files changed

Lines changed: 519 additions & 310 deletions

‎design-system/packages/ui/README.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,8 @@ inside one border and shadow. Flipping above the field keeps the search header
465465
beside the anchor. Labels, validation, and the field id follow the active input;
466466
Escape or selection restores the trigger, and Tab continues from its position
467467
in the form. Search, typed values, and multiple selection remain component-owned.
468+
Joined picker surfaces use the default neutral border on focus, matching the
469+
quiet search treatment while preserving validation and forced-color states.
468470
`SearchField variant="embedded"` removes its standalone pill surface for these
469471
compositions; its container must supply padding, height, and visible focus
470472
treatment. Standalone SearchField pills use a subtle neutral border, increasing
@@ -620,6 +622,11 @@ leading roles scale with user typography. FieldGroup uses the form group tint,
620622
retaining its existing row padding, dividers, and radius. The Patterns form
621623
specimen shows both orientations and long values over a tinted container.
622624

625+
FormSection keeps descriptions below the title by default. Use
626+
`descriptionLayout="inline"` for compact summaries that share one line with
627+
the title. The description uses OverflowText while the heading and actions
628+
remain independent.
629+
623630
Menu and Listbox row surfaces are separated by `overlay.menu.rowGap` (2px),
624631
including grouped options and the Listbox used by Select, Combobox and MultiSelect.
625632
This is distinct from `itemGap` (icon-to-label spacing) and `sectionGap` (8px).

‎design-system/packages/ui/src/components/Combobox/Combobox.meta.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export const comboboxMeta = {
2828
"color.action.neutral.content",
2929
"color.action.neutral.surface",
3030
"color.border.subtle",
31+
"color.border.default",
3132
"color.surface.panel",
3233
"color.focus.ring",
3334
"color.status.danger.border",

‎design-system/packages/ui/src/components/Combobox/Combobox.module.css‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@
305305
}
306306

307307
.popover:focus-within {
308-
border-color: var(--openbitfun-color-field-border-focus);
308+
border-color: var(--openbitfun-color-border-default);
309309
}
310310

311311
.popover[data-invalid="true"] {

‎design-system/packages/ui/src/components/FieldGroup/FieldGroup.meta.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export const fieldGroupMeta = {
88
props: [
99
{ name: "FormSection.title", type: "ReactNode" },
1010
{ name: "FormSection.description", type: "ReactNode" },
11+
{ defaultValue: "stacked", name: "FormSection.descriptionLayout", type: "stacked | inline" },
1112
{ name: "FormSection.leading", type: "ReactNode" },
1213
{ name: "FormSection.actions", type: "ReactNode" },
1314
{ defaultValue: "h2", name: "FormSection.headingAs", type: "h2 | h3 | h4" },

‎design-system/packages/ui/src/components/FieldGroup/FieldGroup.module.css‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,26 @@
7171
letter-spacing: var(--openbitfun-type-body-sm-letter-spacing);
7272
}
7373

74+
.section[data-description-layout="inline"] .header,
75+
.section[data-description-layout="inline"] .headingRegion {
76+
align-items: center;
77+
}
78+
79+
.section[data-description-layout="inline"] .headingContent {
80+
flex-direction: row;
81+
align-items: baseline;
82+
gap: var(--openbitfun-layout-form-section-header-gap);
83+
}
84+
85+
.section[data-description-layout="inline"] .title {
86+
flex: 0 0 auto;
87+
}
88+
89+
.section[data-description-layout="inline"] .description {
90+
flex: 1 1 auto;
91+
min-inline-size: 0;
92+
}
93+
7494
.actions {
7595
display: flex;
7696
flex: 0 0 auto;

‎design-system/packages/ui/src/components/FieldGroup/FieldGroup.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
} from "react";
77
import { classNames } from "../../internal/classNames";
88
import { FieldSurfaceContext, type FieldSurface } from "../../internal/fieldSurface";
9+
import { OverflowText } from "../../primitives/OverflowText";
910
import styles from "./FieldGroup.module.css";
1011

1112
export type FieldGroupAppearance = "plain" | "subtle";
@@ -18,6 +19,8 @@ export interface FormSectionProps
1819
extends Omit<HTMLAttributes<HTMLElement>, "title"> {
1920
actions?: ReactNode;
2021
description?: ReactNode;
22+
/** Keep compact section summaries beside the title instead of below it. */
23+
descriptionLayout?: "stacked" | "inline";
2124
headingAs?: FormSectionHeading;
2225
leading?: ReactNode;
2326
title?: ReactNode;
@@ -41,6 +44,7 @@ export const FormSection = forwardRef<HTMLElement, FormSectionProps>(
4144
children,
4245
className,
4346
description,
47+
descriptionLayout = "stacked",
4448
headingAs = "h2",
4549
leading,
4650
title,
@@ -51,6 +55,7 @@ export const FormSection = forwardRef<HTMLElement, FormSectionProps>(
5155
{...props}
5256
className={classNames(styles.section, className)}
5357
data-openbitfun-component="form-section"
58+
data-description-layout={descriptionLayout}
5459
ref={ref}
5560
>
5661
{(title !== undefined && title !== null)
@@ -73,11 +78,15 @@ export const FormSection = forwardRef<HTMLElement, FormSectionProps>(
7378
{ className: styles.title, "data-openbitfun-part": "title" },
7479
title,
7580
)}
76-
{description !== undefined && description !== null && (
81+
{description !== undefined && description !== null && (descriptionLayout === "inline" ? (
82+
<OverflowText as="div" className={styles.description} data-openbitfun-part="description">
83+
{description}
84+
</OverflowText>
85+
) : (
7786
<div className={styles.description} data-openbitfun-part="description">
7887
{description}
7988
</div>
80-
)}
89+
))}
8190
</div>
8291
) : null}
8392
</div>

‎design-system/packages/ui/tests/field-group.test.mjs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ test("form grouping composes semantic sections, grouped surfaces, and independen
4141
);
4242

4343
assert.match(markup, /<section[^>]+data-openbitfun-component="form-section"/);
44+
assert.match(markup, /data-description-layout="stacked"/);
4445
assert.match(markup, /<h3[^>]+data-openbitfun-part="title"[^>]*>Provider<\/h3>/);
4546
assert.match(markup, /data-openbitfun-part="heading-region"/);
4647
assert.match(markup, /data-openbitfun-part="leading"[^>]*><svg aria-hidden="true"><\/svg>/);
@@ -56,6 +57,25 @@ test("form grouping composes semantic sections, grouped surfaces, and independen
5657
assert.match(markup, /data-control-width="fill"/);
5758
});
5859

60+
test("inline summaries preserve separate headings, descriptions, and actions", () => {
61+
const markup = renderToStaticMarkup(
62+
createElement(FormSection, {
63+
actions: createElement("button", { type: "button", "aria-expanded": false }, "Expand"),
64+
description: "Connection settings",
65+
descriptionLayout: "inline",
66+
headingAs: "h3",
67+
title: "Provider",
68+
}),
69+
);
70+
71+
assert.match(markup, /data-description-layout="inline"/);
72+
assert.match(markup, /<h3[^>]+data-openbitfun-part="title"[^>]*>Provider<\/h3>/);
73+
assert.match(markup, /data-openbitfun-part="description"/);
74+
assert.match(markup, /Connection settings/);
75+
assert.match(markup, /data-openbitfun-part="actions"[^>]*><button[^>]*aria-expanded="false"/);
76+
assert.doesNotMatch(markup, /<h3[^>]*>[\s\S]*Connection settings[\s\S]*<\/h3>/);
77+
});
78+
5979
test("form grouping styles consume only shared public composition tokens", async () => {
6080
const styles = await readFile(new URL("../dist/styles.css", import.meta.url), "utf8");
6181

‎src/web-ui/src/app/scenes/settings/pages/ai/ExecutionSettingsPage.tsx‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,25 @@ const ExecutionSettingsPage: React.FC = () => {
316316
) : (
317317
<>
318318

319+
<ConfigPageSection
320+
title={t('deferredToolLoading.sectionTitle')}
321+
description={t('deferredToolLoading.sectionDescription')}
322+
>
323+
<ConfigPageRow
324+
label={t('common.enable')}
325+
description={!enableDeferredToolLoading ? t('deferredToolLoading.warning') : undefined}
326+
align="center"
327+
>
328+
<div className="openbitfun-runtime-settings__row-control" data-openbitfun-component="runtime-settings" data-openbitfun-part="control">
329+
<Switch
330+
checked={enableDeferredToolLoading}
331+
onChange={(event) => handleDeferredToolLoadingChange(event.target.checked)}
332+
disabled={deferredToolLoadingConfigSaving}
333+
/>
334+
</div>
335+
</ConfigPageRow>
336+
</ConfigPageSection>
337+
319338
{/* ── Tool execution behavior ────────────────────────────── */}
320339
<ConfigPageSection
321340
title={t('toolExecution.sectionTitle')}
@@ -453,25 +472,6 @@ const ExecutionSettingsPage: React.FC = () => {
453472
</ConfigPageRow>
454473
</ConfigPageSection>
455474

456-
<ConfigPageSection
457-
title={t('deferredToolLoading.sectionTitle')}
458-
description={t('deferredToolLoading.sectionDescription')}
459-
>
460-
<ConfigPageRow
461-
label={t('common.enable')}
462-
description={!enableDeferredToolLoading ? t('deferredToolLoading.warning') : undefined}
463-
align="center"
464-
>
465-
<div className="openbitfun-runtime-settings__row-control" data-openbitfun-component="runtime-settings" data-openbitfun-part="control">
466-
<Switch
467-
checked={enableDeferredToolLoading}
468-
onChange={(event) => handleDeferredToolLoadingChange(event.target.checked)}
469-
disabled={deferredToolLoadingConfigSaving}
470-
/>
471-
</div>
472-
</ConfigPageRow>
473-
</ConfigPageSection>
474-
475475
<ToolJsonRepairSection />
476476
<ReviewCapacitySection />
477477

‎src/web-ui/src/app/scenes/settings/pages/ai/MemorySettingsSection.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,7 @@ const MemorySettingsSection: React.FC = () => {
451451
<ConfigPageSection
452452
title={t('sections.advanced.title')}
453453
description={t('sections.advanced.description')}
454+
descriptionLayout={advancedOpen ? 'stacked' : 'inline'}
454455
extra={(
455456
<Tooltip
456457
content={t(advancedOpen ? 'actions.collapseAdvanced' : 'actions.expandAdvanced')}
@@ -463,7 +464,7 @@ const MemorySettingsSection: React.FC = () => {
463464
onClick={() => setAdvancedOpen((open) => !open)}
464465
aria-label={t(advancedOpen ? 'actions.collapseAdvanced' : 'actions.expandAdvanced')}
465466
aria-expanded={advancedOpen}
466-
icon={advancedOpen ? <Icon name="chevron-up" size="lg" /> : <Icon name="chevron-down" size="lg" />}
467+
icon={<Icon name={advancedOpen ? 'chevron-down' : 'chevron-right'} size="lg" />}
467468
/>
468469
</Tooltip>
469470
)}
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
// @vitest-environment jsdom
2+
3+
import React, { act } from 'react';
4+
import { createRoot, type Root } from 'react-dom/client';
5+
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
6+
import { aiApi } from '@/infrastructure/api';
7+
import { configManager } from '@/infrastructure/config/services/ConfigManager';
8+
import type { AIModelConfig } from '@/infrastructure/config/types';
9+
import { i18nService } from '@/infrastructure/i18n';
10+
import ModelSettingsPage from './ModelSettingsPage';
11+
12+
globalThis.IS_REACT_ACT_ENVIRONMENT = true;
13+
14+
function deferred() {
15+
let resolve!: (value: unknown) => void;
16+
let reject!: (reason: Error) => void;
17+
const promise = new Promise<unknown>((resolvePromise, rejectPromise) => {
18+
resolve = resolvePromise;
19+
reject = rejectPromise;
20+
});
21+
return { promise, resolve, reject };
22+
}
23+
24+
const model: AIModelConfig = {
25+
id: 'loading-test-model',
26+
name: 'Test provider',
27+
model_name: 'loading-test-model',
28+
provider: 'openai',
29+
base_url: 'https://example.test/v1',
30+
api_key: '',
31+
enabled: true,
32+
category: 'general_chat',
33+
capabilities: ['text_chat'],
34+
};
35+
36+
// Service reads are controlled to exercise pending, failed, and recovered states.
37+
// The actual settings and design-system components render; this is not visual QA.
38+
describe('ModelSettingsPage independent loading', () => {
39+
let container: HTMLDivElement;
40+
let root: Root;
41+
let reads: Map<string, ReturnType<typeof deferred>>;
42+
const copy = (key: string) => String(i18nService.getI18nInstance().t(key, { ns: 'settings/models' }));
43+
const button = (key: string) => [...container.querySelectorAll<HTMLButtonElement>('button')]
44+
.find(element => element.textContent === copy(key))!;
45+
const selectors = () => [...container.querySelectorAll<HTMLButtonElement>(
46+
'.default-model-config [role="combobox"]',
47+
)];
48+
49+
beforeAll(async () => {
50+
await Promise.all((['settings', 'settings/models', 'settings/default-model', 'components'] as const).map(
51+
namespace => i18nService.loadNamespace(namespace),
52+
));
53+
});
54+
55+
beforeEach(() => {
56+
reads = new Map([
57+
'ai.models', 'ai.default_models', 'ai.proxy',
58+
'ai.stream_idle_timeout_secs', 'ai.stream_ttft_timeout_secs',
59+
].map(path => [path, deferred()]));
60+
vi.spyOn(configManager, 'getOptionalConfig').mockImplementation(
61+
<T,>(path: string) => reads.get(path)!.promise as Promise<T | undefined>,
62+
);
63+
vi.spyOn(configManager, 'getConfig').mockImplementation(
64+
<T,>(path?: string) => reads.get(path!)!.promise as Promise<T>,
65+
);
66+
vi.spyOn(aiApi, 'getModelCatalog').mockImplementation(() => new Promise(() => {}));
67+
vi.spyOn(aiApi, 'getLocalModelsDevCatalogs').mockImplementation(() => new Promise(() => {}));
68+
vi.spyOn(aiApi, 'getModelsDevCatalogStatus').mockImplementation(() => new Promise(() => {}));
69+
vi.spyOn(aiApi, 'listSubscriptionAccounts').mockImplementation(() => new Promise(() => {}));
70+
vi.spyOn(aiApi, 'onModelCatalogUpdated').mockReturnValue(() => {});
71+
container = document.createElement('div');
72+
document.body.append(container);
73+
root = createRoot(container);
74+
});
75+
76+
afterEach(() => {
77+
act(() => root.unmount());
78+
container.remove();
79+
vi.restoreAllMocks();
80+
});
81+
82+
it('shows stable content immediately and releases models without waiting for catalogs or network settings', async () => {
83+
await act(async () => root.render(<ModelSettingsPage />));
84+
85+
for (const key of ['sections.acquisition', 'sections.selectionModes', 'sections.pool', 'sections.more']) {
86+
expect(container.textContent).toContain(copy(key));
87+
}
88+
for (const key of ['acquisition.noneConfigured', 'acquisition.noneImported', 'pool.empty']) {
89+
expect(container.textContent).not.toContain(copy(key));
90+
}
91+
expect(selectors()).toHaveLength(4);
92+
expect(selectors().every(element => element.disabled)).toBe(true);
93+
expect(button('actions.addModel').disabled).toBe(true);
94+
expect(button('acquisition.import').disabled).toBe(false);
95+
96+
await act(async () => {
97+
reads.get('ai.models')!.resolve([model]);
98+
reads.get('ai.default_models')!.resolve({ primary: model.id });
99+
});
100+
101+
expect(container.querySelector('[data-testid="settings-model-pool"]')?.textContent).toContain(model.model_name);
102+
expect(selectors()[0].disabled).toBe(false);
103+
expect(selectors()[0].textContent).toContain(model.model_name);
104+
expect(button('actions.addModel').disabled).toBe(false);
105+
});
106+
107+
it('isolates a failed model read, keeps network settings usable, and recovers through the section retry', async () => {
108+
await act(async () => root.render(<ModelSettingsPage />));
109+
await act(async () => {
110+
reads.get('ai.models')!.reject(new Error('Model host unavailable'));
111+
reads.get('ai.default_models')!.resolve({});
112+
reads.get('ai.proxy')!.resolve({ enabled: true, url: 'http://localhost:7890' });
113+
reads.get('ai.stream_idle_timeout_secs')!.resolve(120);
114+
reads.get('ai.stream_ttft_timeout_secs')!.resolve(60);
115+
});
116+
117+
expect(container.textContent).toContain(copy('messages.loadFailedLocked'));
118+
expect(container.textContent).toContain(copy('sections.selectionModes'));
119+
expect(container.textContent).not.toContain(copy('pool.empty'));
120+
expect(selectors().every(element => element.disabled)).toBe(true);
121+
await act(async () => button('sections.more').click());
122+
const proxy = [...container.querySelectorAll<HTMLInputElement>('input')]
123+
.find(element => element.value === 'http://localhost:7890');
124+
expect(proxy?.disabled).toBe(false);
125+
expect([...container.querySelectorAll<HTMLInputElement>('input')].some(
126+
element => element.value === '120' && !element.disabled,
127+
)).toBe(true);
128+
129+
reads.set('ai.models', deferred());
130+
await act(async () => button('messages.retry').click());
131+
await act(async () => reads.get('ai.models')!.resolve([model]));
132+
expect(container.textContent).not.toContain(copy('messages.loadFailedLocked'));
133+
expect(container.querySelector('[data-testid="settings-model-pool"]')?.textContent).toContain(model.model_name);
134+
expect(button('actions.addModel').disabled).toBe(false);
135+
});
136+
});

0 commit comments

Comments
 (0)