Skip to content

Commit 29f8fc0

Browse files
authored
Merge pull request #805 from objectstack-ai/copilot/fix-view-config-panel-issues
2 parents 5f2c752 + e895318 commit 29f8fc0

File tree

19 files changed

+364
-38
lines changed

19 files changed

+364
-38
lines changed

ROADMAP.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,11 @@ ObjectUI is a universal Server-Driven UI (SDUI) engine built on React + Tailwind
404404
- [x] 18 new ViewConfigPanel interaction tests: collapseAllByDefault, showDescription, clickIntoRecordDetails, addDeleteRecordsInline toggles; sharing visibility conditional hide; navigation width/openNewTab conditional rendering; all 5 rowHeight button clicks; boundary tests (empty actions, long labels, special chars); pageSizeOptions input; densityMode/ARIA live enums; addRecord conditional sub-editor; sharing visibility select
405405
- [x] 8 new schema-driven spec tests: accessibility field ordering, emptyState compound field, switch field defaults, comprehensive visibleWhen predicates (sharing, navigation width, navigation openNewTab)
406406
- [x] All spec fields verified: Appearance/UserActions/Sharing/Accessibility sections 100% covered with UI controls, defaults, ordering, and conditional visibility
407+
- [x] Add `description` field to `NamedListView` protocol interface (spec alignment)
408+
- [x] Add `disabledWhen` predicate to `ConfigField` type — grid-only fields (striped/bordered/wrapHeaders/resizable) disabled for non-grid views
409+
- [x] Add `expandedSections` prop to `ConfigPanelRenderer` for external section collapse control (auto-focus/highlight)
410+
- [x] Add `helpText` to navigation-dependent fields (width/openNewTab) with i18n hints (all 11 locales)
411+
- [x] 24 new tests: expandedSections override (3), disabledWhen evaluation (2), grid-only disabledWhen predicates (16), helpText validation (2), description spec alignment (1)
407412

408413
**Code Reduction:** ~1655 lines imperative → ~170 lines declarative wrapper + ~1100 lines schema factory + ~180 lines shared utils = **>50% net reduction in component code** with significantly improved maintainability
409414

apps/console/src/__tests__/view-config-schema.test.tsx

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -701,11 +701,16 @@ describe('spec alignment', () => {
701701
it('documents UI extension fields not in NamedListView spec', () => {
702702
const keys = allFieldKeys();
703703
// These fields are UI extensions — documented as protocol suggestions
704-
const uiExtensions = ['description', '_source', '_groupBy', '_typeOptions'];
704+
const uiExtensions = ['_source', '_groupBy', '_typeOptions'];
705705
for (const ext of uiExtensions) {
706706
expect(keys).toContain(ext);
707707
}
708708
});
709+
710+
it('description is now a spec-aligned field in NamedListView', () => {
711+
const keys = allFieldKeys();
712+
expect(keys).toContain('description');
713+
});
709714
});
710715

711716
// ── Accessibility section field ordering ─────────────────────────────
@@ -800,4 +805,80 @@ describe('spec alignment', () => {
800805
expect(field.visibleWhen!({})).toBe(true);
801806
});
802807
});
808+
809+
// ── disabledWhen predicates for grid-only fields ─────────────────────
810+
describe('disabledWhen predicates for grid-only fields', () => {
811+
function buildSchema() {
812+
return buildViewConfigSchema({
813+
t: mockT,
814+
fieldOptions: mockFieldOptions,
815+
objectDef: mockObjectDef,
816+
updateField: mockUpdateField,
817+
filterGroupValue: mockFilterGroup,
818+
sortItemsValue: mockSortItems,
819+
});
820+
}
821+
822+
const gridOnlyFields = ['striped', 'bordered', 'wrapHeaders', 'resizable'];
823+
824+
for (const fieldKey of gridOnlyFields) {
825+
it(`${fieldKey} should have disabledWhen predicate`, () => {
826+
const schema = buildSchema();
827+
const section = schema.sections.find(s => s.key === 'appearance')!;
828+
const field = section.fields.find(f => f.key === fieldKey)!;
829+
expect(field.disabledWhen).toBeDefined();
830+
});
831+
832+
it(`${fieldKey} should be disabled when view type is kanban`, () => {
833+
const schema = buildSchema();
834+
const section = schema.sections.find(s => s.key === 'appearance')!;
835+
const field = section.fields.find(f => f.key === fieldKey)!;
836+
expect(field.disabledWhen!({ type: 'kanban' })).toBe(true);
837+
});
838+
839+
it(`${fieldKey} should not be disabled when view type is grid`, () => {
840+
const schema = buildSchema();
841+
const section = schema.sections.find(s => s.key === 'appearance')!;
842+
const field = section.fields.find(f => f.key === fieldKey)!;
843+
expect(field.disabledWhen!({ type: 'grid' })).toBe(false);
844+
});
845+
846+
it(`${fieldKey} should not be disabled when view type is undefined (default grid)`, () => {
847+
const schema = buildSchema();
848+
const section = schema.sections.find(s => s.key === 'appearance')!;
849+
const field = section.fields.find(f => f.key === fieldKey)!;
850+
expect(field.disabledWhen!({})).toBe(false);
851+
});
852+
}
853+
});
854+
855+
// ── helpText on navigation-dependent fields ──────────────────────────
856+
describe('helpText on navigation-dependent fields', () => {
857+
function buildSchema() {
858+
return buildViewConfigSchema({
859+
t: mockT,
860+
fieldOptions: mockFieldOptions,
861+
objectDef: mockObjectDef,
862+
updateField: mockUpdateField,
863+
filterGroupValue: mockFilterGroup,
864+
sortItemsValue: mockSortItems,
865+
});
866+
}
867+
868+
it('navigation width field has helpText', () => {
869+
const schema = buildSchema();
870+
const section = schema.sections.find(s => s.key === 'pageConfig')!;
871+
const field = section.fields.find(f => f.key === '_navigationWidth')!;
872+
expect(field.helpText).toBeDefined();
873+
expect(typeof field.helpText).toBe('string');
874+
});
875+
876+
it('navigation openNewTab field has helpText', () => {
877+
const schema = buildSchema();
878+
const section = schema.sections.find(s => s.key === 'pageConfig')!;
879+
const field = section.fields.find(f => f.key === '_navigationOpenNewTab')!;
880+
expect(field.helpText).toBeDefined();
881+
expect(typeof field.helpText).toBe('string');
882+
});
883+
});
803884
});

apps/console/src/utils/view-config-schema.tsx

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,7 @@ function buildPageConfigSection(
123123
</ConfigRow>
124124
),
125125
},
126-
// UI extension: description — not in NamedListView spec.
127-
// Protocol suggestion: add optional 'description' to NamedListView.
126+
// spec: NamedListView.description — optional view description
128127
{
129128
key: 'description',
130129
label: t('console.objectView.description'),
@@ -284,6 +283,7 @@ function buildPageConfigSection(
284283
key: '_navigationWidth',
285284
label: t('console.objectView.navigationWidth'),
286285
type: 'custom',
286+
helpText: t('console.objectView.navigationWidthHint'),
287287
visibleWhen: (draft) => ['drawer', 'modal', 'split'].includes(draft.navigation?.mode || 'page'),
288288
render: (_value, _onChange, draft) => {
289289
const navMode = draft.navigation?.mode || 'page';
@@ -307,6 +307,7 @@ function buildPageConfigSection(
307307
key: '_navigationOpenNewTab',
308308
label: t('console.objectView.openNewTab'),
309309
type: 'custom',
310+
helpText: t('console.objectView.openNewTabHint'),
310311
visibleWhen: (draft) => ['page', 'new_window'].includes(draft.navigation?.mode || 'page'),
311312
render: (_value, _onChange, draft) => {
312313
const navMode = draft.navigation?.mode || 'page';
@@ -972,10 +973,12 @@ function buildAppearanceSection(
972973
title: t('console.objectView.appearance'),
973974
collapsible: true,
974975
fields: [
975-
// spec: NamedListView.striped
976-
buildSwitchField('striped', t('console.objectView.striped'), 'toggle-striped', false, true),
977-
// spec: NamedListView.bordered
978-
buildSwitchField('bordered', t('console.objectView.bordered'), 'toggle-bordered', false, true),
976+
// spec: NamedListView.striped (grid-only)
977+
buildSwitchField('striped', t('console.objectView.striped'), 'toggle-striped', false, true,
978+
(draft) => draft.type != null && draft.type !== 'grid'),
979+
// spec: NamedListView.bordered (grid-only)
980+
buildSwitchField('bordered', t('console.objectView.bordered'), 'toggle-bordered', false, true,
981+
(draft) => draft.type != null && draft.type !== 'grid'),
979982
// spec: NamedListView.color — field for row/card coloring
980983
{
981984
key: 'color',
@@ -997,8 +1000,9 @@ function buildAppearanceSection(
9971000
</ConfigRow>
9981001
),
9991002
},
1000-
// spec: NamedListView.wrapHeaders
1001-
buildSwitchField('wrapHeaders', t('console.objectView.wrapHeaders'), 'toggle-wrapHeaders', false, true),
1003+
// spec: NamedListView.wrapHeaders (grid-only)
1004+
buildSwitchField('wrapHeaders', t('console.objectView.wrapHeaders'), 'toggle-wrapHeaders', false, true,
1005+
(draft) => draft.type != null && draft.type !== 'grid'),
10021006
// spec: NamedListView.collapseAllByDefault
10031007
buildSwitchField('collapseAllByDefault', t('console.objectView.collapseAllByDefault'), 'toggle-collapseAllByDefault', false, true),
10041008
// spec: NamedListView.fieldTextColor
@@ -1024,8 +1028,9 @@ function buildAppearanceSection(
10241028
},
10251029
// spec: NamedListView.showDescription
10261030
buildSwitchField('showDescription', t('console.objectView.showFieldDescriptions'), 'toggle-showDescription', true),
1027-
// spec: NamedListView.resizable
1028-
buildSwitchField('resizable', t('console.objectView.resizableColumns'), 'toggle-resizable', false, true),
1031+
// spec: NamedListView.resizable (grid-only)
1032+
buildSwitchField('resizable', t('console.objectView.resizableColumns'), 'toggle-resizable', false, true,
1033+
(draft) => draft.type != null && draft.type !== 'grid'),
10291034
// spec: NamedListView.densityMode — compact/comfortable/spacious
10301035
{
10311036
key: 'densityMode',
@@ -1440,17 +1445,27 @@ function buildAccessibilitySection(
14401445
* Build a standard Switch toggle field with custom testId.
14411446
* @param defaultOn - if true, treat undefined/absent as enabled (checked = value !== false)
14421447
* @param explicitTrue - if true, only check when value === true
1448+
* @param disabledWhen - optional predicate to disable the switch based on draft state
14431449
*/
1444-
function buildSwitchField(key: string, label: string, testId: string, defaultOn = false, explicitTrue = false): ConfigField {
1450+
function buildSwitchField(
1451+
key: string,
1452+
label: string,
1453+
testId: string,
1454+
defaultOn = false,
1455+
explicitTrue = false,
1456+
disabledWhen?: (draft: Record<string, any>) => boolean,
1457+
): ConfigField {
14451458
return {
14461459
key,
14471460
label,
14481461
type: 'custom',
1449-
render: (value, onChange) => (
1462+
disabledWhen,
1463+
render: (value, onChange, draft) => (
14501464
<ConfigRow label={label}>
14511465
<Switch
14521466
data-testid={testId}
14531467
checked={explicitTrue ? value === true : (defaultOn ? value !== false : !!value)}
1468+
disabled={disabledWhen ? disabledWhen(draft) : false}
14541469
onCheckedChange={(checked: boolean) => onChange(checked)}
14551470
className="scale-75"
14561471
/>

packages/components/src/__tests__/config-field-renderer.test.tsx

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,4 +271,37 @@ describe('ConfigFieldRenderer', () => {
271271
expect(screen.getByText('Add sort')).toBeDefined();
272272
});
273273
});
274+
275+
describe('helpText rendering', () => {
276+
it('should render helpText below field when provided', () => {
277+
const field: ConfigField = {
278+
key: 'width',
279+
label: 'Width',
280+
type: 'input',
281+
helpText: 'Available for drawer, modal, and split modes',
282+
};
283+
render(<ConfigFieldRenderer field={field} value="" onChange={vi.fn()} draft={defaultDraft} />);
284+
expect(screen.getByText('Available for drawer, modal, and split modes')).toBeDefined();
285+
});
286+
287+
it('should not render helpText paragraph when not provided', () => {
288+
const field: ConfigField = { key: 'name', label: 'Name', type: 'input' };
289+
const { container } = render(<ConfigFieldRenderer field={field} value="" onChange={vi.fn()} draft={defaultDraft} />);
290+
expect(container.querySelectorAll('p').length).toBe(0);
291+
});
292+
293+
it('should render helpText for custom field type', () => {
294+
const React = require('react');
295+
const field: ConfigField = {
296+
key: 'custom',
297+
label: 'Custom',
298+
type: 'custom',
299+
helpText: 'Custom help text',
300+
render: (value, onChange) => React.createElement('div', { 'data-testid': 'custom-content' }, 'Custom'),
301+
};
302+
render(<ConfigFieldRenderer field={field} value="" onChange={vi.fn()} draft={defaultDraft} />);
303+
expect(screen.getByText('Custom help text')).toBeDefined();
304+
expect(screen.getByTestId('custom-content')).toBeDefined();
305+
});
306+
});
274307
});

packages/components/src/__tests__/config-panel-renderer.test.tsx

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,4 +317,110 @@ describe('ConfigPanelRenderer', () => {
317317
expect(onFieldChange).toHaveBeenCalledWith('name', 'NewName');
318318
});
319319
});
320+
321+
describe('expandedSections prop', () => {
322+
it('should override defaultCollapsed when section is in expandedSections', () => {
323+
render(
324+
<ConfigPanelRenderer
325+
{...defaultProps}
326+
schema={collapsibleSchema}
327+
draft={{ columns: '3', theme: 'dark', source: 'api' }}
328+
expandedSections={['appearance']}
329+
/>,
330+
);
331+
// Appearance is defaultCollapsed=true but expandedSections overrides it
332+
expect(screen.getByText('Theme')).toBeDefined();
333+
});
334+
335+
it('should not affect sections not in expandedSections', () => {
336+
render(
337+
<ConfigPanelRenderer
338+
{...defaultProps}
339+
schema={collapsibleSchema}
340+
draft={{ columns: '3', theme: 'dark', source: 'api' }}
341+
expandedSections={['appearance']}
342+
/>,
343+
);
344+
// Data section is not in expandedSections, should remain expanded (its default)
345+
expect(screen.getByText('Source')).toBeDefined();
346+
});
347+
348+
it('should allow local toggle to still work alongside expandedSections', () => {
349+
render(
350+
<ConfigPanelRenderer
351+
{...defaultProps}
352+
schema={collapsibleSchema}
353+
draft={{ columns: '3', theme: 'dark', source: 'api' }}
354+
expandedSections={['appearance']}
355+
/>,
356+
);
357+
// Appearance is forced expanded by expandedSections
358+
expect(screen.getByText('Theme')).toBeDefined();
359+
360+
// Data section can still be toggled locally
361+
expect(screen.getByText('Source')).toBeDefined();
362+
fireEvent.click(screen.getByTestId('section-header-data'));
363+
expect(screen.queryByText('Source')).toBeNull();
364+
});
365+
});
366+
367+
describe('disabledWhen on fields', () => {
368+
it('should disable input field when disabledWhen returns true', () => {
369+
const schemaWithDisabledWhen: ConfigPanelSchema = {
370+
breadcrumb: ['Test'],
371+
sections: [
372+
{
373+
key: 'sec',
374+
title: 'Section',
375+
fields: [
376+
{
377+
key: 'name',
378+
label: 'Name',
379+
type: 'input',
380+
disabledWhen: (draft) => draft.locked === true,
381+
},
382+
],
383+
},
384+
],
385+
};
386+
render(
387+
<ConfigPanelRenderer
388+
{...defaultProps}
389+
schema={schemaWithDisabledWhen}
390+
draft={{ name: 'Test', locked: true }}
391+
/>,
392+
);
393+
const input = screen.getByTestId('config-field-name');
394+
expect((input as HTMLInputElement).disabled).toBe(true);
395+
});
396+
397+
it('should enable input field when disabledWhen returns false', () => {
398+
const schemaWithDisabledWhen: ConfigPanelSchema = {
399+
breadcrumb: ['Test'],
400+
sections: [
401+
{
402+
key: 'sec',
403+
title: 'Section',
404+
fields: [
405+
{
406+
key: 'name',
407+
label: 'Name',
408+
type: 'input',
409+
disabledWhen: (draft) => draft.locked === true,
410+
},
411+
],
412+
},
413+
],
414+
};
415+
render(
416+
<ConfigPanelRenderer
417+
{...defaultProps}
418+
schema={schemaWithDisabledWhen}
419+
draft={{ name: 'Test', locked: false }}
420+
/>,
421+
);
422+
const input = screen.getByTestId('config-field-name');
423+
expect((input as HTMLInputElement).disabled).toBe(false);
424+
});
425+
});
320426
});

0 commit comments

Comments
 (0)