Migrate ViewConfigPanel to Schema-Driven architecture (1655→170 lines)#735
Migrate ViewConfigPanel to Schema-Driven architecture (1655→170 lines)#735
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…idget Replace all inline useState expand/collapse patterns in view-config-schema.tsx render functions with the existing ExpandableWidget component. This eliminates 8 occurrences of React.useState inside render callbacks (sort, fields/columns, filter, quick filters, conditional formatting, row actions, bulk actions, and buildFieldMultiSelect helper). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Extract shared utilities (operator mappings, filter/sort bridge, field normalization) to view-config-utils.ts - Create schema factory (view-config-schema.tsx) with all sections: Page, Data, Appearance, UserActions, Sharing, Accessibility - Rewrite ViewConfigPanel as thin wrapper using useConfigDraft + ConfigPanelRenderer - Enhance ConfigPanelRenderer with accessibility props (panelRef, role, ariaLabel, tabIndex, testId) - Add ExpandableWidget component for proper hook-safe expandable sub-sections - Update test mock to support ConfigPanelRenderer and useConfigDraft - All 122 existing tests pass Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…props to ConfigPanelRenderer - Add closeTitle, footerTestId, saveTestId, discardTestId props to ConfigPanelRenderer - ViewConfigPanel passes view-specific test IDs for backward compatibility - Remove old backup file - All 2404 tests pass across 80 test files Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Add view-config-schema.test.tsx covering: - normalizeFieldType: all field type normalizations - parseSpecFilter: all filter parsing formats (triplet, and/or, nested, object) - toSpecFilter: filter conversion back to spec format - parseCommaSeparated / parseNumberList: basic parsing - deriveFieldOptions: field options from objectDef - toFilterGroup / toSortItems: bridge functions - Operator mapping constants - buildViewConfigSchema: schema structure, sections, collapsible flags - visibleWhen predicates for navigation and sharing fields Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 53 new tests for view-config-utils and view-config-schema - ROADMAP.md updated with P1.11 Schema-Driven View Config Panel Migration section - Full test suite: 2457 tests across 81 files, all pass Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
- Replace ternary in useMemo deps with computed stableKey variable Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR migrates the Console ViewConfigPanel from a large imperative implementation to the existing schema-driven config panel framework (ConfigPanelRenderer + useConfigDraft + ConfigPanelSchema), aiming to reduce complexity while preserving behavior and test coverage.
Changes:
- Added shared ViewConfigPanel utilities (
view-config-utils.ts) for filter/sort bridging, operator mappings, parsing helpers, and field option derivation. - Added a schema factory (
view-config-schema.tsx) that declaratively defines ViewConfigPanel sections/fields, with custom render escape hatches for complex widgets. - Rewrote
ViewConfigPanel.tsxinto a thin wrapper that builds schema + manages draft state viauseConfigDraft, and enhancedConfigPanelRendererwith customization + test id overrides.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/custom/config-panel-renderer.tsx | Adds panel customization props (refs/ARIA/test IDs) for reuse and backward-compatible testing. |
| apps/console/src/utils/view-config-utils.ts | Extracts operator mappings, filter/sort bridge helpers, parsing utilities, and constants for view config. |
| apps/console/src/utils/view-config-schema.tsx | Implements the schema factory for all ViewConfigPanel sections using schema-driven rendering. |
| apps/console/src/components/ViewConfigPanel.tsx | Replaces imperative UI with schema-driven wrapper using useConfigDraft + schema factory. |
| apps/console/src/tests/view-config-schema.test.tsx | Adds unit tests for utilities and schema factory behavior. |
| apps/console/src/tests/ViewConfigPanel.test.tsx | Updates mocks/tests for new schema-driven rendering approach. |
| ROADMAP.md | Updates roadmap to reflect completion of the schema-driven ViewConfigPanel migration. |
| function buildDataSection( | ||
| t: ViewSchemaFactoryOptions['t'], | ||
| fieldOptions: FieldOption[], | ||
| fieldSelectWithNone: Array<{ value: string; label: string }>, | ||
| objectDef: ViewSchemaFactoryOptions['objectDef'], | ||
| updateField: ViewSchemaFactoryOptions['updateField'], | ||
| filterGroupValue: FilterGroup, | ||
| sortItemsValue: SortItem[], | ||
| ): ConfigPanelSchema['sections'][number] { |
There was a problem hiding this comment.
fieldSelectWithNone is passed into buildDataSection/buildAppearanceSection but never referenced inside those functions. With noUnusedParameters: true, these unused parameters will cause a type-check failure. Either remove fieldSelectWithNone from the signatures/call sites (and delete the local if no longer needed), or actually use it to render the select options.
| onClick={onClose} | ||
| className="h-7 w-7 p-0" | ||
| data-testid="config-panel-close" | ||
| title={closeTitle} |
There was a problem hiding this comment.
The close button is icon-only, but it only receives a title. title is not a reliable accessible name for screen readers, so this control likely needs an explicit aria-label (e.g., derived from closeTitle or a default like "Close").
| title={closeTitle} | |
| title={closeTitle} | |
| aria-label={closeTitle ?? 'Close'} |
| // Stabilize source reference: only change when view ID changes. | ||
| // This prevents useConfigDraft from resetting on every parent re-render | ||
| // (same behavior as original useEffect with [activeView.id] dependency). | ||
| const stableKey = mode === 'create' ? 'create' : activeView.id; | ||
| const stableActiveView = useMemo( | ||
| () => ({ ...activeView }), | ||
| [stableKey], // eslint-disable-line react-hooks/exhaustive-deps | ||
| ); | ||
| const effectiveActiveView = mode === 'create' ? defaultNewView : stableActiveView; | ||
|
|
||
| // Reset draft when switching to a different view (by ID change only). | ||
| // We intentionally depend on activeView.id rather than the full activeView | ||
| // object so that real-time draft propagation (via onViewUpdate → parent | ||
| // setViewDraft → merged activeView) does not reset isDirty to false. | ||
| useEffect(() => { | ||
| setDraft({ ...effectiveActiveView }); | ||
| setIsDirty(mode === 'create'); | ||
| }, [mode, activeView.id]); // eslint-disable-line react-hooks/exhaustive-deps | ||
| // Schema-driven draft state management | ||
| const { draft, isDirty, updateField, discard } = useConfigDraft(effectiveActiveView, { | ||
| mode, | ||
| onUpdate: onViewUpdate, | ||
| }); |
There was a problem hiding this comment.
useConfigDraft.discard() resets the draft back to the source object. Here the source is effectiveActiveView, which is memoized to a snapshot keyed only by activeView.id. As a result, calling discard() after Save (and also on Discard) can revert the UI back to stale pre-save values and prevents the panel from picking up updated activeView props with the same id. Consider tracking a local “committedSource” state that you update on activeView.id changes and on successful save (e.g., set committedSource = draft), and pass that committedSource into useConfigDraft so discard/save reset correctly without losing changes.
| */ | ||
|
|
||
| import React from 'react'; | ||
| import { Input, Switch, Checkbox, FilterBuilder, SortBuilder, ConfigRow, SectionHeader } from '@object-ui/components'; |
There was a problem hiding this comment.
SectionHeader is imported but never used in this file. With noUnusedLocals: true in the console tsconfig, this will fail type-check/build. Remove the unused import (or use it if it was intended).
| import { Input, Switch, Checkbox, FilterBuilder, SortBuilder, ConfigRow, SectionHeader } from '@object-ui/components'; | |
| import { Input, Switch, Checkbox, FilterBuilder, SortBuilder, ConfigRow } from '@object-ui/components'; |
ViewConfigPanel was ~1655 lines of imperative UI code. Migrated to the existing Schema-Driven config panel framework (
ConfigPanelRenderer+useConfigDraft+ConfigPanelSchema).Changes
New files
apps/console/src/utils/view-config-utils.ts— Extracted shared utilities: operator mappings (SPEC_TO_BUILDER_OP/BUILDER_TO_SPEC_OP),normalizeFieldType,parseSpecFilter/toSpecFilter, field option derivationapps/console/src/utils/view-config-schema.tsx— Schema factory producing aConfigPanelSchemawith all 6 sections. Complex widgets (column selector, filter/sort builders, conditional formatting, quick filters) usetype: 'custom'render functions. IncludesExpandableWidgetcomponent to avoid hooks-in-render violations for expandable sub-sectionsRewritten
apps/console/src/components/ViewConfigPanel.tsx— Now a ~170-line wrapper:useConfigDraftfor state,buildViewConfigSchemafor structure,ConfigPanelRendererfor renderingEnhanced
ConfigPanelRenderer— Added optional props for panel customization:panelRef,role,ariaLabel,tabIndex,testId,closeTitle,footerTestId,saveTestId,discardTestId. Enables backward-compatible test IDs when wrapping in domain-specific panelsTests
ConfigPanelRenderer+useConfigDraft)Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.