feat(vue-vuetify): improve mixed and additional property editors - #2610
kchobantonov wants to merge 14 commits into
Conversation
✅ Deploy Preview for jsonforms-examples ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@sdirix please review |
|
coverage: 84.286%. remained the same — kchobantonov:feat/vue-vuetify-mixed-additional-properties into eclipsesource:master |
EclipseSourceAI
left a comment
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. Noting why helps, since replies are read and taken into account in follow-up reviews.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
Reworks mixed-type and dynamic additionalProperties editing in @jsonforms/vue-vuetify: object/array mixed values now get a splitpanes tree + detail editor, nested mixed controls navigate into that tree instead of nesting renderers, and property renaming is added both in the tree and in the Additional Properties editor.
The extraction of src/util/dynamicProperties.ts is the strongest part: findPropertySchema and getPropertyNameSchema come straight out of AdditionalProperties.vue and are now shared by both rename paths, with unit tests. The non-mutating schema preparation in createMixedRenderInfos and replacing the JSON.stringify descriptor-rebuild check with haveAdditionalPropertyNamesChanged are both real fixes.
Where to focus:
getPathAncestorNodeIdsinMixedRenderer.vuemixes up absolute and relative paths, so "reveal in tree" never expands ancestors when the mixed control's own path is non-empty (the normaladditionalPropertiescase). Reproduced in the running example app on both the Additional Properties and Mixed Object examples.- Making
splitpanesa required peer dependency plus re-implementing its base CSS inVSplitpanes.sassis a packaging decision that deserves an explicit call. MixedRenderer.vueis now ~1600 lines; the pure tree-building layer would be better off insrc/utilwhere it can be tested.
pnpm test, pnpm lint, pnpm type-check and the full pnpm build all pass on the merged branch, and the CSS output includes the splitpanes rules.
| parentPath: string, | ||
| propertyName: string, | ||
| ): string => (parentPath ? `${parentPath}.${propertyName}` : propertyName); | ||
|
|
There was a problem hiding this comment.
Always dot-joining is right for core's current path handling, since resolveData and setDataAt split purely on . (setData.ts#L31), whereas core's compose drops the separator when the segment starts with [ (path.ts#L29). The downside is that a [foo] property created here won't resolve in material/vanilla/angular, which all still use composePaths. Fixing compose in core would make bracket support work everywhere instead of just this renderer set.
There was a problem hiding this comment.
This needs separate PR fix since that could impact other code
| const treeNodes = computed(() => | ||
| showTreeView.value | ||
| ? buildTreeFromData( | ||
| input.control.value.data, | ||
| resolvedSchema.value ?? input.control.value.schema, | ||
| input.control.value.rootSchema, | ||
| input.control.value.path, | ||
| vuetifyControl.computedLabel.value, | ||
| input.control.value.enabled, | ||
| input.control.value.readonly, | ||
| showPrimitivesInTree.value, | ||
| mixedTranslations.itemLabel, | ||
| ) | ||
| : [], | ||
| ); |
There was a problem hiding this comment.
treeNodes depends on control.data, so every keystroke in the detail pane rebuilds the whole tree and each node runs cloneDeep + cleanSchema (twice for arrays, via prepareArraySchema). Fine for the example data, but it could get noticeable on larger objects; caching per node path or keying the rebuild on the data shape rather than the value would help.
…rebuilding the tree on data changes
- Enforce mutation permissions and protect required/static keys - Preserve selected editors when tree rows are filtered out - Retain allowed types, detail UI, and matching parent constraints - Cancel stale renames and refresh dynamic property schemas - Add regression tests and include tests in type checking - Use draft-07 definitions in schema fixtures
Adjust selected paths when an earlier array item is deleted, preserving the selected item and any nested field. Add regression tests for index shifts, sibling boundaries, and selected-node or ancestor deletion.
Allow adding, renaming, and editing dotted keys in AdditionalProperties through isolated forms. Preserve property schemas, local references, validation, and read-only guards. Add regression tests for JSON value types, literal-key updates, and recursive schema references.
Keep empty Add inputs free of inline errors while preserving validation and disabled-button guards. Align Svelte renderers with Vuetify, document the behavior, and add regression tests.
Merge compatible scalar constraints from matching patternProperties and preserve complex or conflicting schemas with allOf. Keep combinator renderer selection intact and cache compiled patterns. Add utility and component regression tests for overlapping patterns.
This branch has not been deployed
3 participants
Summary
This PR improves the Vue Vuetify handling of mixed-type and dynamic additional properties.
It adds a tree-based editor for object and array values while keeping primitive values editable through the existing mixed renderer. The implementation is scoped to the Vue Vuetify package.
Changes
propertyNamespatternPropertiessplitpanesto the Vue Vuetify peer and development dependencies.Validation