From d4dc5d474fb176442afa8b2ef0a2e4d685ff9222 Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Wed, 30 Oct 2024 14:23:55 +0100 Subject: [PATCH 01/19] Changed roster implementation, Network parametric editor works partially. --- .../components/FirstClassEditorSelector.tsx | 4 + .../NephioNetworkParametricEditor.tsx | 57 +++++++++++ .../nodes/PxeSectionNode.tsx | 2 + .../nodes/widget/PxeRosterWidgetNode.tsx | 94 +++++++++++-------- 4 files changed, 116 insertions(+), 41 deletions(-) create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/FirstClassEditorSelector.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/FirstClassEditorSelector.tsx index 12b95518..623dd2c1 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/FirstClassEditorSelector.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/FirstClassEditorSelector.tsx @@ -33,6 +33,7 @@ import { PackageVariantSetEditor } from './FirstClassEditors/PackageVariantSetEd import { NephioCapacityParametricEditor } from './ParametricFirstClassEditors/NephioCapacityParametricEditor'; import { NephioTokenParametricEditor } from './ParametricFirstClassEditors/NephioTokenParametricEditor'; import { NephioWorkloadClusterParametricEditor } from './ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor'; +import { NephioNetworkParametricEditor } from './ParametricFirstClassEditors/NephioNetworkParametricEditor'; type OnUpdatedYamlFn = (yaml: string) => void; type OnNoNamedEditorFn = () => void; @@ -83,6 +84,9 @@ export const FirstClassEditorSelector = ({ case 'infra.nephio.org/v1alpha1/Token': return ; + case 'infra.nephio.org/v1alpha1/Network': + return ; + case 'infra.nephio.org/v1alpha1/WorkloadCluster': return ; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx new file mode 100644 index 00000000..9b7934b6 --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx @@ -0,0 +1,57 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createEditorFromConfiguration } from '../PxeParametricEditor/createEditorFromConfiguration'; +import { PxeConfigurationFactory } from '../PxeParametricEditor/configuration'; +import { metadataEditorSection } from './partial/metadataEditorSection'; + +const { section, rowLayout, arrayTypeRoster, objectTypeRoster, singleLineText } = PxeConfigurationFactory; + +export const NephioNetworkParametricEditor = createEditorFromConfiguration({ + topLevelProperties: ['metadata', 'spec'], + entries: [ + metadataEditorSection({ isNamespacedResource: true }), + section({ name: 'Topology' }, singleLineText({ path: 'spec.topology', isRequired: true })), + section( + { name: 'Routing tables' }, + arrayTypeRoster( + { path: 'spec.routingTables', isRequired: false }, + section( + { name: 'Routing table' }, + singleLineText({ path: 'value.name', isRequired: true }), + section( + { name: 'Prefixes' }, + arrayTypeRoster( + { path: 'value.prefixes', isRequired: true }, + section( + { name: 'Prefix' }, + singleLineText({ path: 'value.prefix', isRequired: true }), + section( + { name: 'Labels' }, + objectTypeRoster( + { path: 'value.labels', isRequired: false }, + rowLayout(singleLineText({ path: 'key', isRequired: true }), singleLineText({ path: 'value' })), + ), + ), + ), + ), + ), + ), + ), + ), + section({ name: 'Bridge domains' }), + ], +}); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx index f029dc52..6bace869 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx @@ -29,6 +29,7 @@ export const PxeSectionNode: React.FC = ({ resourceChunk, onResourceChangeRequest, parentExpandedSectionState, + children, }) => { const configurationEntry = configurationEntryUncasted as PxeSectionEntry; const { name, entries: childEntries } = configurationEntry; @@ -51,6 +52,7 @@ export const PxeSectionNode: React.FC = ({ onResourceChangeRequest={onResourceChangeRequest} parentExpandedSectionState={[expandedSection, setExpandedSection]} /> + {children} ); }; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx index 1e5eac32..fc065e6d 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx @@ -21,13 +21,12 @@ import { get } from 'lodash'; import React, { Fragment } from 'react'; import { IconButton } from '../../../../../Controls'; import { useEditorStyles } from '../../../FirstClassEditors/styles'; -import { PxeParametricEditorNodeProps } from '../../PxeParametricEditorNode'; -import { PxeRosterWidgetEntry, PxeValueType } from '../../types/PxeConfiguration.types'; +import { PxeParametricEditorNode, PxeParametricEditorNodeProps } from '../../PxeParametricEditorNode'; +import { PxeConfigurationEntryType, PxeRosterWidgetEntry, PxeValueType } from '../../types/PxeConfiguration.types'; import { PxeResourceChangeRequest, PxeValue } from '../../types/PxeParametricEditor.types'; import { createResourceChunkAfterChangeRequest } from '../../utils/createResourceChunkAfterChangeRequest'; import { generateValueLabel } from '../../utils/generateLabelsForWidgets'; import { arrayWithItemRemoved, arrayWithItemReplaced } from '../../utils/general/immutableArrays'; -import { PxeParametricEditorNodeList } from '../../PxeParametricEditorNodeList'; type RosterItemResourceChunk = { readonly key: string; @@ -41,6 +40,7 @@ export const PxeRosterWidgetNode: React.FC = ({ configurationEntry, resourceChunk, onResourceChangeRequest, + parentExpandedSectionState, }) => { const { values, itemEntries } = configurationEntry as PxeRosterWidgetEntry; const valueDescriptor = values[0]; @@ -54,10 +54,8 @@ export const PxeRosterWidgetNode: React.FC = ({ changeRequest, ) as RosterItemResourceChunk; - onResourceChangeRequest({ - valueDescriptor, - newValue: valueFromItemChunks(arrayWithItemReplaced(itemChunks, itemIndex, newItemChunk), rosterValueType), - }); + const newValue = valueFromItemChunks(arrayWithItemReplaced(itemChunks, itemIndex, newItemChunk), rosterValueType); + onResourceChangeRequest({ valueDescriptor, newValue }); }; const handleItemAddition = () => { @@ -66,48 +64,62 @@ export const PxeRosterWidgetNode: React.FC = ({ value: undefined, }; - onResourceChangeRequest({ - valueDescriptor, - newValue: valueFromItemChunks([...itemChunks, newItemChunk], rosterValueType), - }); + const newValue = valueFromItemChunks([...itemChunks, newItemChunk], rosterValueType); + onResourceChangeRequest({ valueDescriptor, newValue }); }; const handleItemDeletion = (itemIndex: number) => { - onResourceChangeRequest({ - valueDescriptor, - newValue: valueFromItemChunks(arrayWithItemRemoved(itemChunks, itemIndex), rosterValueType), - }); + const newValue = valueFromItemChunks(arrayWithItemRemoved(itemChunks, itemIndex), rosterValueType); + onResourceChangeRequest({ valueDescriptor, newValue }); }; const editorClasses = useEditorStyles(); const rosterClasses = useStyles(); - return ( - - {itemChunks.map((itemChunk, itemIndex) => ( -
-
- handleResourceChangeRequestForItem(itemIndex, changeRequest)} - /> -
-
- handleItemDeletion(itemIndex)} - > - - + + if (itemEntries.length === 0) { + return ; + } else if (itemEntries.length > 1) { + // TODO With proper styling this should be actually possible. Reconsider handling this. + throw new Error('Roster Widget does not support multiple configuration entries per item.'); + } else { + // FIXME Refactor by extraction? + return ( + + {itemChunks.map((itemChunk, itemIndex) => ( +
+
+ handleResourceChangeRequestForItem(itemIndex, changeRequest)} + parentExpandedSectionState={parentExpandedSectionState} + > + {itemEntries[0].type === PxeConfigurationEntryType.Section && ( + + )} + +
+ {itemEntries[0].type !== PxeConfigurationEntryType.Section && ( +
+ handleItemDeletion(itemIndex)} + > + + +
+ )}
-
- ))} - - - ); + ))} + + + ); + } }; const itemChunksFromValue = (value: PxeValue): readonly RosterItemResourceChunk[] => From 1bb35c60eabb77d183f360ff9a060dda76df9aac Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Wed, 6 Nov 2024 12:10:06 +0100 Subject: [PATCH 02/19] Rework of roster widget factory methods. --- .../NephioNetworkParametricEditor.tsx | 33 ++++++--------- .../NephioWorkloadClusterParametricEditor.tsx | 5 +-- .../partial/metadataEditorSection.ts | 18 +++----- .../configuration/rosterConfigurationEntry.ts | 42 ++++++++++++------- 4 files changed, 46 insertions(+), 52 deletions(-) diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx index 9b7934b6..832da7cd 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx @@ -25,28 +25,19 @@ export const NephioNetworkParametricEditor = createEditorFromConfiguration({ entries: [ metadataEditorSection({ isNamespacedResource: true }), section({ name: 'Topology' }, singleLineText({ path: 'spec.topology', isRequired: true })), - section( - { name: 'Routing tables' }, - arrayTypeRoster( - { path: 'spec.routingTables', isRequired: false }, - section( - { name: 'Routing table' }, - singleLineText({ path: 'value.name', isRequired: true }), + arrayTypeRoster( + { name: 'Routing tables', path: 'spec.routingTables', isRequired: false }, + section( + { name: 'Routing table' }, + singleLineText({ path: 'value.name', isRequired: true }), + arrayTypeRoster( + { name: 'Prefixes', path: 'value.prefixes', isRequired: true }, section( - { name: 'Prefixes' }, - arrayTypeRoster( - { path: 'value.prefixes', isRequired: true }, - section( - { name: 'Prefix' }, - singleLineText({ path: 'value.prefix', isRequired: true }), - section( - { name: 'Labels' }, - objectTypeRoster( - { path: 'value.labels', isRequired: false }, - rowLayout(singleLineText({ path: 'key', isRequired: true }), singleLineText({ path: 'value' })), - ), - ), - ), + { name: 'Prefix' }, + singleLineText({ path: 'value.prefix', isRequired: true }), + objectTypeRoster( + { name: 'Labels', path: 'value.labels', isRequired: false }, + rowLayout(singleLineText({ path: 'key', isRequired: true }), singleLineText({ path: 'value' })), ), ), ), diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx index ae9b1275..38302c46 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx @@ -28,10 +28,7 @@ export const NephioWorkloadClusterParametricEditor = createEditorFromConfigurati { name: 'Configuration' }, singleLineText({ path: 'spec.clusterName', isRequired: true }), singleLineText({ path: 'spec.masterInterface' }), - section( - { name: 'CNIs' }, - arrayTypeRoster({ path: 'spec.cnis', isRequired: false, name: 'CNIs' }, singleLineText({ path: 'value' })), - ), + arrayTypeRoster({ name: 'CNIs', path: 'spec.cnis', isRequired: false }, singleLineText({ path: 'value' })), ), ], }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts index 9f4320df..87d7d725 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts @@ -24,18 +24,12 @@ export const metadataEditorSection = ({ isNamespacedResource }: { isNamespacedRe { name: 'Resource Metadata' }, singleLineText({ path: 'metadata.name', isRequired: true }), ...(isNamespacedResource ? [singleLineText({ path: 'metadata.namespace' })] : []), - section( - { name: 'Labels' }, - objectTypeRoster( - { path: 'metadata.labels', isRequired: false }, - rowLayout(singleLineText({ path: 'key', isRequired: true }), singleLineText({ path: 'value' })), - ), + objectTypeRoster( + { name: 'Labels', path: 'metadata.labels', isRequired: false }, + rowLayout(singleLineText({ path: 'key', isRequired: true }), singleLineText({ path: 'value' })), ), - section( - { name: 'Annotations' }, - objectTypeRoster( - { path: 'metadata.annotations', isRequired: false }, - rowLayout(singleLineText({ path: 'key', isRequired: true }), singleLineText({ path: 'value' })), - ), + objectTypeRoster( + { name: 'Annotations', path: 'metadata.annotations', isRequired: false }, + rowLayout(singleLineText({ path: 'key', isRequired: true }), singleLineText({ path: 'value' })), ), ); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts index 1db4256d..c67e7871 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts @@ -17,40 +17,52 @@ import { PxeConfigurationEntry, PxeConfigurationEntryType, - PxeRosterWidgetEntry, + PxeSectionEntry, PxeValueType, } from '../types/PxeConfiguration.types'; +import { sectionConfigurationEntry } from './sectionConfigurationEntry'; +// TODO With the current UI every roster entry is wrapped in implicit section entry. +// Also, name parameter is mandatory as it iss used for displaying section name. +// Change this after UI redesign. export const objectTypeRosterConfigurationEntry = ( { + name, path, isRequired = false, - name, }: { + name: string; path: string; isRequired?: boolean; - name?: string; }, ...itemEntries: PxeConfigurationEntry[] -): PxeRosterWidgetEntry => ({ - type: PxeConfigurationEntryType.Roster, - values: [{ path, type: PxeValueType.Object, isRequired, display: { name } }], - itemEntries, -}); +): PxeSectionEntry => + sectionConfigurationEntry( + { name }, + { + type: PxeConfigurationEntryType.Roster, + values: [{ path, type: PxeValueType.Object, isRequired, display: { name } }], + itemEntries, + }, + ); export const arrayTypeRosterConfigurationEntry = ( { + name, path, isRequired = false, - name, }: { + name: string; path: string; isRequired?: boolean; - name?: string; }, ...itemEntries: PxeConfigurationEntry[] -): PxeRosterWidgetEntry => ({ - type: PxeConfigurationEntryType.Roster, - values: [{ path, type: PxeValueType.Array, isRequired, display: { name } }], - itemEntries, -}); +): PxeSectionEntry => + sectionConfigurationEntry( + { name }, + { + type: PxeConfigurationEntryType.Roster, + values: [{ path, type: PxeValueType.Array, isRequired, display: { name } }], + itemEntries, + }, + ); From 02613e689fe4cf4e387d7f7db884fb981a6e7f5a Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Wed, 6 Nov 2024 13:46:53 +0100 Subject: [PATCH 03/19] Finally, working setup for Jest tests. --- jest.setup.js | 4 ++++ package.json | 6 ++++++ 2 files changed, 10 insertions(+) create mode 100644 jest.setup.js diff --git a/jest.setup.js b/jest.setup.js new file mode 100644 index 00000000..e1d9e108 --- /dev/null +++ b/jest.setup.js @@ -0,0 +1,4 @@ +const { TextEncoder, TextDecoder } = require('util'); + +global.TextEncoder = TextEncoder; +global.TextDecoder = TextDecoder; diff --git a/package.json b/package.json index c0e49a8c..3bda864e 100644 --- a/package.json +++ b/package.json @@ -51,6 +51,12 @@ "prettier --write" ] }, + "jest": { + "setupFilesAfterEnv": ["/../../../jest.setup.js","@testing-library/jest-dom"], + "moduleNameMapper": { + "monaco-editor": "/../../../node_modules/monaco-editor/monaco.d.ts" + } + }, "dependencies": { "@types/react": "^17", "@types/react-dom": "^17" From 238d1c31f960ea8cf9589bdda6f2e9b82fcb1b1f Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Tue, 12 Nov 2024 13:55:04 +0100 Subject: [PATCH 04/19] Roster tests and fixes. More tests and refactoring still needed. --- .../NephioNetworkParametricEditor.tsx | 2 +- .../NephioWorkloadClusterParametricEditor.tsx | 2 +- .../partial/metadataEditorSection.ts | 4 +- .../ParametricEditorRosterWidget.test.tsx | 333 ++++++++++++++++++ .../ParametricEditorSingleTextWidget.test.tsx | 85 +++++ .../__tests__/testUtils/findEditorElement.ts | 42 +++ .../testUtils/getLastResourceState.ts | 21 ++ .../configuration/rosterConfigurationEntry.ts | 28 ++ .../selectValueWidgetConfigurationEntry.ts | 4 +- .../nodes/widget/PxeRosterWidgetNode.tsx | 34 +- .../widget/PxeSingleLineTextWidgetNode.tsx | 1 + .../types/PxeConfiguration.types.ts | 3 +- .../types/PxeParametricEditor.types.ts | 4 +- .../utils/defaultValueForType.ts | 33 ++ .../utils/findInConfigurationEntries.ts | 36 ++ 15 files changed, 613 insertions(+), 19 deletions(-) create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorSingleTextWidget.test.tsx create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/findEditorElement.ts create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/getLastResourceState.ts create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/defaultValueForType.ts create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/findInConfigurationEntries.ts diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx index 832da7cd..996c08d2 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx @@ -37,7 +37,7 @@ export const NephioNetworkParametricEditor = createEditorFromConfiguration({ singleLineText({ path: 'value.prefix', isRequired: true }), objectTypeRoster( { name: 'Labels', path: 'value.labels', isRequired: false }, - rowLayout(singleLineText({ path: 'key', isRequired: true }), singleLineText({ path: 'value' })), + rowLayout(singleLineText({ path: '$key', isRequired: true }), singleLineText({ path: '$value' })), ), ), ), diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx index 38302c46..16b60e30 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx @@ -28,7 +28,7 @@ export const NephioWorkloadClusterParametricEditor = createEditorFromConfigurati { name: 'Configuration' }, singleLineText({ path: 'spec.clusterName', isRequired: true }), singleLineText({ path: 'spec.masterInterface' }), - arrayTypeRoster({ name: 'CNIs', path: 'spec.cnis', isRequired: false }, singleLineText({ path: 'value' })), + arrayTypeRoster({ name: 'CNIs', path: 'spec.cnis', isRequired: false }, singleLineText({ path: '$value' })), ), ], }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts index 87d7d725..4ae032d0 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts @@ -26,10 +26,10 @@ export const metadataEditorSection = ({ isNamespacedResource }: { isNamespacedRe ...(isNamespacedResource ? [singleLineText({ path: 'metadata.namespace' })] : []), objectTypeRoster( { name: 'Labels', path: 'metadata.labels', isRequired: false }, - rowLayout(singleLineText({ path: 'key', isRequired: true }), singleLineText({ path: 'value' })), + rowLayout(singleLineText({ path: '$key', isRequired: true }), singleLineText({ path: '$value' })), ), objectTypeRoster( { name: 'Annotations', path: 'metadata.annotations', isRequired: false }, - rowLayout(singleLineText({ path: 'key', isRequired: true }), singleLineText({ path: 'value' })), + rowLayout(singleLineText({ path: '$key', isRequired: true }), singleLineText({ path: '$value' })), ), ); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx new file mode 100644 index 00000000..5a06206f --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx @@ -0,0 +1,333 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { renderWithEffects } from '@backstage/test-utils'; +import { RenderResult } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; +import { get } from 'lodash'; +import React from 'react'; +import { PxeParametricEditor } from '../PxeParametricEditor'; +import { PxeConfiguration } from '../types/PxeConfiguration.types'; +import { PxeConfigurationFactory } from '../configuration'; +import { findRosterAddButton, findRosterItem, findTextFieldInput } from './testUtils/findEditorElement'; +import { getLastResourceState } from './testUtils/getLastResourceState'; + +const { arrayTypeRoster, objectTypeRoster, rowLayout, singleLineText } = PxeConfigurationFactory; + +const CONFIGURATION: PxeConfiguration = { + topLevelProperties: ['spec'], + entries: [ + arrayTypeRoster( + { path: 'spec.arr.string.opt.nonEmpty', name: 'Test' }, + singleLineText({ path: '$value', isRequired: false }), + ), + arrayTypeRoster( + { path: 'spec.arr.string.opt.empty', name: 'Test' }, + singleLineText({ path: '$value', isRequired: false }), + ), + arrayTypeRoster( + { path: 'spec.arr.string.req.nonEmpty', name: 'Test' }, + singleLineText({ path: '$value', isRequired: true }), + ), + arrayTypeRoster( + { path: 'spec.arr.string.req.empty', name: 'Test' }, + singleLineText({ path: '$value', isRequired: true }), + ), + objectTypeRoster( + { path: 'spec.obj.string.opt.nonEmpty', name: 'Test' }, + rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: false })), + ), + objectTypeRoster( + { path: 'spec.obj.string.opt.empty', name: 'Test' }, + rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: false })), + ), + objectTypeRoster( + { path: 'spec.obj.string.req.nonEmpty', name: 'Test' }, + rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: true })), + ), + objectTypeRoster( + { path: 'spec.obj.string.req.empty', name: 'Test' }, + rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: true })), + ), + ], +}; + +const YAML = ` +spec: + arr: + string: + opt: + nonEmpty: ['foo'] + empty: [] + req: + nonEmpty: ['foo'] + empty: [] + obj: + string: + opt: + nonEmpty: { foo: 'bar' } + empty: {} + req: + nonEmpty: { foo: 'bar' } + empty: {} +`; + +describe('ParametricEditorRosterWidget', () => { + let editor: RenderResult; + let resourceChangeHandler: jest.Mock; + + beforeEach(async () => { + resourceChangeHandler = jest.fn(); + + editor = await renderWithEffects( + , + ); + }); + + describe('array-based', () => { + describe('(optional string item, initially non-empty)', () => { + const rosterPath = 'spec.arr.string.opt.nonEmpty'; + + it('should change item value on text input', async () => { + const textInput = findTextFieldInput(findRosterItem(editor, rosterPath, 0), '$value'); + + await userEvent.type(textInput, 'bar'); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual(['foobar']); + }); + + it('should add null item on Add button click', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual(['foo', null]); + }); + + it('should add null item on Add button click (x2)', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual(['foo', null, null]); + }); + }); + + describe('(optional string item, initially empty)', () => { + const rosterPath = 'spec.arr.string.opt.empty'; + + it('should add null item on Add button click', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual([null]); + }); + + it('should add null item on Add button click (x2)', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual([null, null]); + }); + }); + + describe('(required string item, initially non-empty)', () => { + const rosterPath = 'spec.arr.string.req.nonEmpty'; + + it('should change item value on text input', async () => { + const textInput = findTextFieldInput(findRosterItem(editor, rosterPath, 0), '$value'); + + await userEvent.type(textInput, 'bar'); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual(['foobar']); + }); + + it('should add empty string item on Add button click', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual(['foo', '']); + }); + + it('should add empty string item on Add button click (x2)', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual(['foo', '', '']); + }); + }); + + describe('(required string item, initially empty)', () => { + const rosterPath = 'spec.arr.string.req.empty'; + + it('should add empty string item on Add button click', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual(['']); + }); + + it('should add empty string item on Add button click (x2)', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual(['', '']); + }); + }); + }); + + describe('object-based', () => { + describe('(optional string item, initially non-empty)', () => { + const rosterPath = 'spec.obj.string.opt.nonEmpty'; + + it('should change item key on text input', async () => { + const textInput = findTextFieldInput(findRosterItem(editor, rosterPath, 0), '$key'); + + await userEvent.type(textInput, 'fizz'); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual({ foofizz: 'bar' }); + }); + + it('should change item value on text input', async () => { + const textInput = findTextFieldInput(findRosterItem(editor, rosterPath, 0), '$value'); + + await userEvent.type(textInput, 'fizz'); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual({ foo: 'barfizz' }); + }); + + it('should add null item on Add button click', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual({ foo: 'bar', '': null }); + }); + + it('should disable Add button when item with empty key is added', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + expect(addButton.disabled).toBeTruthy(); + }); + }); + + describe('(optional string item, initially empty)', () => { + const rosterPath = 'spec.obj.string.opt.empty'; + + it('should add null item on Add button click', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual({ '': null }); + }); + + it('should disable Add button when item with empty key is added', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + expect(addButton.disabled).toBeTruthy(); + }); + }); + + describe('(required string item, initially non-empty)', () => { + const rosterPath = 'spec.obj.string.req.nonEmpty'; + + it('should change item key on text input', async () => { + const textInput = findTextFieldInput(findRosterItem(editor, rosterPath, 0), '$key'); + + await userEvent.type(textInput, 'fizz'); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual({ foofizz: 'bar' }); + }); + + it('should change item value on text input', async () => { + const textInput = findTextFieldInput(findRosterItem(editor, rosterPath, 0), '$value'); + + await userEvent.type(textInput, 'fizz'); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual({ foo: 'barfizz' }); + }); + + it('should add empty string item on Add button click', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual({ foo: 'bar', '': '' }); + }); + + it('should disable Add button when item with empty key is added', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + expect(addButton.disabled).toBeTruthy(); + }); + }); + + describe('(required string item, initially empty)', () => { + const rosterPath = 'spec.obj.string.req.empty'; + + it('should add empty string item on Add button click', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual({ '': '' }); + }); + + it('should disable Add button when item with empty key is added', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + + expect(addButton.disabled).toBeTruthy(); + }); + }); + }); +}); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorSingleTextWidget.test.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorSingleTextWidget.test.tsx new file mode 100644 index 00000000..4e932388 --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorSingleTextWidget.test.tsx @@ -0,0 +1,85 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { renderWithEffects } from '@backstage/test-utils'; +import { RenderResult } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; +import React from 'react'; +import { PxeParametricEditor } from '../PxeParametricEditor'; +import { PxeConfiguration } from '../types/PxeConfiguration.types'; +import { PxeConfigurationFactory } from '../configuration'; +import { findTextFieldInput } from './testUtils/findEditorElement'; +import { getLastResourceState } from './testUtils/getLastResourceState'; + +const { singleLineText } = PxeConfigurationFactory; + +const CONFIGURATION: PxeConfiguration = { + topLevelProperties: ['spec'], + entries: [ + singleLineText({ path: 'spec.textOptional' }), + singleLineText({ path: 'spec.textRequired', isRequired: true }), + ], +}; + +const YAML = ` +spec: + textOptional: "foo" + textRequired: "foo" +`; + +describe('ParametricEditorSingleTextWidget', () => { + let editor: RenderResult; + let resourceChangeHandler: jest.Mock; + + beforeEach(async () => { + resourceChangeHandler = jest.fn(); + + editor = await renderWithEffects( + , + ); + }); + + ['textOptional', 'textRequired'].forEach(property => { + it(`should change value on text input (${property})`, async () => { + const textInput = findTextFieldInput(editor, `spec.${property}`); + + await userEvent.type(textInput, 'bar'); + + const resource = getLastResourceState(resourceChangeHandler); + expect(resource.spec[property]).toBe('foobar'); + }); + }); + + it('should change value on text clear (optional => undefined)', async () => { + const textInput = findTextFieldInput(editor, 'spec.textOptional'); + + await userEvent.clear(textInput); + + const resource = getLastResourceState(resourceChangeHandler); + expect(resource.spec.textOptional).toBeUndefined(); + }); + + it('should change value on text clear (required => empty string)', async () => { + const textInput = findTextFieldInput(editor, 'spec.textRequired'); + + await userEvent.clear(textInput); + + const resource = getLastResourceState(resourceChangeHandler); + expect(resource.spec.textRequired).toBe(''); + }); +}); + +// TODO Add tests for other options of single text widget. diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/findEditorElement.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/findEditorElement.ts new file mode 100644 index 00000000..21964e13 --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/findEditorElement.ts @@ -0,0 +1,42 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* eslint @backstage/no-undeclared-imports: off */ +import { RenderResult } from '@testing-library/react'; + +export const findTextFieldInput = (searchRoot: RenderResult | HTMLElement, path: string): HTMLInputElement => { + const searchRootElement = 'baseElement' in searchRoot ? searchRoot.baseElement : searchRoot; + const inputElement = searchRootElement?.querySelector(`[data-testid="TextField_${path}"]`)?.querySelector('input'); + + if (inputElement) return inputElement; + else throw new Error(`Text field ${path} not found.`); +}; + +export const findRosterItem = (searchRoot: RenderResult | HTMLElement, path: string, index: number): HTMLElement => { + const searchRootElement = 'baseElement' in searchRoot ? searchRoot.baseElement : searchRoot; + const rosterItemElement = searchRootElement?.querySelector(`[data-testid="RosterItem_${path}_${index}"]`); + + if (rosterItemElement) return rosterItemElement as HTMLElement; + else throw new Error(`Roster item ${path} [${index}] not found.`); +}; + +export const findRosterAddButton = (searchRoot: RenderResult | HTMLElement, path: string): HTMLButtonElement => { + const searchRootElement = 'baseElement' in searchRoot ? searchRoot.baseElement : searchRoot; + const rosterAddButtonElement = searchRootElement?.querySelector(`[data-testid="RosterAddButton_${path}"]`); + + if (rosterAddButtonElement) return rosterAddButtonElement as HTMLButtonElement; + else throw new Error(`Roster add button ${path} not found.`); +}; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/getLastResourceState.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/getLastResourceState.ts new file mode 100644 index 00000000..ede995fa --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/getLastResourceState.ts @@ -0,0 +1,21 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { load } from 'js-yaml'; +import { PxeResourceChunk } from '../../types/PxeParametricEditor.types'; + +export const getLastResourceState = (resourceChangeHandler: jest.Mock): PxeResourceChunk & any => + load(resourceChangeHandler.mock.lastCall[0]); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts index c67e7871..fd97f0ba 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts @@ -18,9 +18,12 @@ import { PxeConfigurationEntry, PxeConfigurationEntryType, PxeSectionEntry, + PxeValueDescriptor, PxeValueType, + PxeWidgetEntry, } from '../types/PxeConfiguration.types'; import { sectionConfigurationEntry } from './sectionConfigurationEntry'; +import { findInConfigurationEntries } from '../utils/findInConfigurationEntries'; // TODO With the current UI every roster entry is wrapped in implicit section entry. // Also, name parameter is mandatory as it iss used for displaying section name. @@ -30,10 +33,12 @@ export const objectTypeRosterConfigurationEntry = ( name, path, isRequired = false, + itemValue, }: { name: string; path: string; isRequired?: boolean; + itemValue?: PxeValueDescriptor; }, ...itemEntries: PxeConfigurationEntry[] ): PxeSectionEntry => @@ -42,6 +47,7 @@ export const objectTypeRosterConfigurationEntry = ( { type: PxeConfigurationEntryType.Roster, values: [{ path, type: PxeValueType.Object, isRequired, display: { name } }], + itemValue: resolveItemValueDescriptor(name, itemValue ?? null, itemEntries), itemEntries, }, ); @@ -51,10 +57,12 @@ export const arrayTypeRosterConfigurationEntry = ( name, path, isRequired = false, + itemValue, }: { name: string; path: string; isRequired?: boolean; + itemValue?: PxeValueDescriptor; }, ...itemEntries: PxeConfigurationEntry[] ): PxeSectionEntry => @@ -63,6 +71,26 @@ export const arrayTypeRosterConfigurationEntry = ( { type: PxeConfigurationEntryType.Roster, values: [{ path, type: PxeValueType.Array, isRequired, display: { name } }], + itemValue: resolveItemValueDescriptor(name, itemValue ?? null, itemEntries), itemEntries, }, ); + +const resolveItemValueDescriptor = ( + rosterName: string, + explicitItemValueDescriptor: PxeValueDescriptor | null, + itemEntries: PxeConfigurationEntry[], +): PxeValueDescriptor => { + const itemValueEntry = findInConfigurationEntries( + itemEntries, + entry => 'values' in entry && entry.values[0]?.path === '$value', + ) as PxeWidgetEntry | null; + + if (explicitItemValueDescriptor && itemValueEntry) { + throw new Error(`Redundant itemValue definition in roster ${rosterName}. Descriptor inherited from $value entry.`); + } else if (!explicitItemValueDescriptor && !itemValueEntry) { + throw new Error(`No itemValue definition in roster ${rosterName}.`); + } else { + return explicitItemValueDescriptor || itemValueEntry?.values[0]!; + } +}; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts index d215e11e..8854a4f3 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts @@ -23,13 +23,13 @@ import { export const selectValueWidgetConfigurationEntry = ({ path, - type = PxeValueType.Any, + type, isRequired = false, name, options, }: { path: string; - type?: PxeValueType; + type: PxeValueType; isRequired?: boolean; name?: string; options: readonly PxeValueOption[]; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx index fc065e6d..437d629e 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx @@ -27,10 +27,11 @@ import { PxeResourceChangeRequest, PxeValue } from '../../types/PxeParametricEdi import { createResourceChunkAfterChangeRequest } from '../../utils/createResourceChunkAfterChangeRequest'; import { generateValueLabel } from '../../utils/generateLabelsForWidgets'; import { arrayWithItemRemoved, arrayWithItemReplaced } from '../../utils/general/immutableArrays'; +import { defaultValueForType } from '../../utils/defaultValueForType'; type RosterItemResourceChunk = { - readonly key: string; - readonly value: PxeValue; + readonly $key: string; + readonly $value: PxeValue; }; type RosterValueType = PxeValueType.Object | PxeValueType.Array; @@ -42,7 +43,7 @@ export const PxeRosterWidgetNode: React.FC = ({ onResourceChangeRequest, parentExpandedSectionState, }) => { - const { values, itemEntries } = configurationEntry as PxeRosterWidgetEntry; + const { values, itemEntries, itemValue: itemValueDescriptor } = configurationEntry as PxeRosterWidgetEntry; const valueDescriptor = values[0]; const rosterValueType = valueDescriptor.type as RosterValueType; @@ -60,8 +61,8 @@ export const PxeRosterWidgetNode: React.FC = ({ const handleItemAddition = () => { const newItemChunk: RosterItemResourceChunk = { - key: rosterValueType === PxeValueType.Array ? String(itemChunks.length) : '', - value: undefined, + $key: rosterValueType === PxeValueType.Array ? String(itemChunks.length) : '', + $value: itemValueDescriptor.isRequired ? defaultValueForType(itemValueDescriptor.type) : null, }; const newValue = valueFromItemChunks([...itemChunks, newItemChunk], rosterValueType); @@ -73,6 +74,7 @@ export const PxeRosterWidgetNode: React.FC = ({ onResourceChangeRequest({ valueDescriptor, newValue }); }; + const isAddButtonEnabled = rosterValueType === PxeValueType.Array || itemChunks.every(({ $key }) => $key !== ''); const editorClasses = useEditorStyles(); const rosterClasses = useStyles(); @@ -86,7 +88,11 @@ export const PxeRosterWidgetNode: React.FC = ({ return ( {itemChunks.map((itemChunk, itemIndex) => ( -
+
= ({ )}
))} - @@ -124,14 +136,14 @@ export const PxeRosterWidgetNode: React.FC = ({ const itemChunksFromValue = (value: PxeValue): readonly RosterItemResourceChunk[] => Object.entries(value ?? {}).map(([itemKey, itemValue]) => ({ - key: itemKey, - value: itemValue as PxeValue, + $key: itemKey, + $value: itemValue as PxeValue, })); const valueFromItemChunks = (itemChunks: readonly RosterItemResourceChunk[], rosterType: RosterValueType): PxeValue => rosterType === PxeValueType.Object - ? Object.fromEntries(itemChunks.map(itemChunk => [itemChunk.key, itemChunk.value])) - : itemChunks.map(itemChunk => itemChunk.value); + ? Object.fromEntries(itemChunks.map(itemChunk => [itemChunk.$key, itemChunk.$value])) + : itemChunks.map(itemChunk => itemChunk.$value); const useStyles = makeStyles(() => ({ item: { diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx index 9a815266..3f8a27c9 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx @@ -33,6 +33,7 @@ export const PxeSingleLineTextWidgetNode: React.FC return ( { + switch (valueType) { + case PxeValueType.String: + return ''; + case PxeValueType.Number: + return 0; + case PxeValueType.Object: + return {}; + case PxeValueType.Array: + return []; + default: + throw new Error(`Unsupported value type ${valueType}`); + } +}; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/findInConfigurationEntries.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/findInConfigurationEntries.ts new file mode 100644 index 00000000..c5c75af4 --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/findInConfigurationEntries.ts @@ -0,0 +1,36 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { PxeConfigurationEntry } from '../types/PxeConfiguration.types'; +import { isArray } from 'lodash'; + +export const findInConfigurationEntries = ( + searchIn: PxeConfigurationEntry | readonly PxeConfigurationEntry[], + isSuccess: (entry: PxeConfigurationEntry) => boolean, +): PxeConfigurationEntry | null => { + const entries = isArray(searchIn) ? searchIn : [searchIn]; + + for (const entry of entries) { + if (isSuccess(entry)) { + return entry; + } else if ('entries' in entry) { + const foundEntry = findInConfigurationEntries(entry.entries, isSuccess); + if (foundEntry) { + return foundEntry; + } + } + } + return null; +}; From 77617f254a61715000e131710eb13d3c2c851ffc Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Tue, 12 Nov 2024 17:24:30 +0100 Subject: [PATCH 05/19] Name refactoring; redundant tests removed. --- .../NephioNetworkParametricEditor.tsx | 17 ++- .../ParametricEditorRosterWidget.test.tsx | 144 ++---------------- .../configuration/rosterConfigurationEntry.ts | 12 +- .../selectValueWidgetConfigurationEntry.ts | 2 +- .../singleLineTextWidgetConfigurationEntry.ts | 2 +- .../nodes/widget/PxeRosterWidgetNode.tsx | 7 +- .../nodes/widget/PxeSelectValueWidgetNode.tsx | 2 +- .../widget/PxeSingleLineTextWidgetNode.tsx | 2 +- .../types/PxeConfiguration.types.ts | 4 +- .../utils/generateLabelsForWidgets.ts | 2 +- 10 files changed, 49 insertions(+), 145 deletions(-) diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx index 996c08d2..3f750f76 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx @@ -17,6 +17,7 @@ import { createEditorFromConfiguration } from '../PxeParametricEditor/createEditorFromConfiguration'; import { PxeConfigurationFactory } from '../PxeParametricEditor/configuration'; import { metadataEditorSection } from './partial/metadataEditorSection'; +import { PxeValueType } from '../PxeParametricEditor/types/PxeConfiguration.types'; const { section, rowLayout, arrayTypeRoster, objectTypeRoster, singleLineText } = PxeConfigurationFactory; @@ -26,12 +27,24 @@ export const NephioNetworkParametricEditor = createEditorFromConfiguration({ metadataEditorSection({ isNamespacedResource: true }), section({ name: 'Topology' }, singleLineText({ path: 'spec.topology', isRequired: true })), arrayTypeRoster( - { name: 'Routing tables', path: 'spec.routingTables', isRequired: false }, + { + name: 'Routing tables', + path: 'spec.routingTables', + isRequired: false, + // FIXME Not a best way of doing this. E.g path is redundant. + itemValue: { type: PxeValueType.Object, isRequired: true, path: '$value' }, + }, section( { name: 'Routing table' }, singleLineText({ path: 'value.name', isRequired: true }), arrayTypeRoster( - { name: 'Prefixes', path: 'value.prefixes', isRequired: true }, + { + name: 'Prefixes', + path: 'value.prefixes', + isRequired: true, + // FIXME Not a best way of doing this. E.g path is redundant. + itemValue: { type: PxeValueType.Object, isRequired: true, path: '$value' }, + }, section( { name: 'Prefix' }, singleLineText({ path: 'value.prefix', isRequired: true }), diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx index 5a06206f..d48dde9c 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx @@ -31,35 +31,19 @@ const CONFIGURATION: PxeConfiguration = { topLevelProperties: ['spec'], entries: [ arrayTypeRoster( - { path: 'spec.arr.string.opt.nonEmpty', name: 'Test' }, + { path: 'spec.arr.string.opt', name: 'Test' }, singleLineText({ path: '$value', isRequired: false }), ), arrayTypeRoster( - { path: 'spec.arr.string.opt.empty', name: 'Test' }, - singleLineText({ path: '$value', isRequired: false }), - ), - arrayTypeRoster( - { path: 'spec.arr.string.req.nonEmpty', name: 'Test' }, - singleLineText({ path: '$value', isRequired: true }), - ), - arrayTypeRoster( - { path: 'spec.arr.string.req.empty', name: 'Test' }, + { path: 'spec.arr.string.req', name: 'Test' }, singleLineText({ path: '$value', isRequired: true }), ), objectTypeRoster( - { path: 'spec.obj.string.opt.nonEmpty', name: 'Test' }, + { path: 'spec.obj.string.opt', name: 'Test' }, rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: false })), ), objectTypeRoster( - { path: 'spec.obj.string.opt.empty', name: 'Test' }, - rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: false })), - ), - objectTypeRoster( - { path: 'spec.obj.string.req.nonEmpty', name: 'Test' }, - rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: true })), - ), - objectTypeRoster( - { path: 'spec.obj.string.req.empty', name: 'Test' }, + { path: 'spec.obj.string.req', name: 'Test' }, rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: true })), ), ], @@ -69,20 +53,12 @@ const YAML = ` spec: arr: string: - opt: - nonEmpty: ['foo'] - empty: [] - req: - nonEmpty: ['foo'] - empty: [] + opt: ['foo'] + req: ['foo'] obj: string: - opt: - nonEmpty: { foo: 'bar' } - empty: {} - req: - nonEmpty: { foo: 'bar' } - empty: {} + opt: { foo: 'bar' } + req: { foo: 'bar' } `; describe('ParametricEditorRosterWidget', () => { @@ -98,8 +74,8 @@ describe('ParametricEditorRosterWidget', () => { }); describe('array-based', () => { - describe('(optional string item, initially non-empty)', () => { - const rosterPath = 'spec.arr.string.opt.nonEmpty'; + describe('(optional string item)', () => { + const rosterPath = 'spec.arr.string.opt'; it('should change item value on text input', async () => { const textInput = findTextFieldInput(findRosterItem(editor, rosterPath, 0), '$value'); @@ -130,31 +106,8 @@ describe('ParametricEditorRosterWidget', () => { }); }); - describe('(optional string item, initially empty)', () => { - const rosterPath = 'spec.arr.string.opt.empty'; - - it('should add null item on Add button click', async () => { - const addButton = findRosterAddButton(editor, rosterPath); - - await userEvent.click(addButton); - - const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); - expect(roster).toEqual([null]); - }); - - it('should add null item on Add button click (x2)', async () => { - const addButton = findRosterAddButton(editor, rosterPath); - - await userEvent.click(addButton); - await userEvent.click(addButton); - - const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); - expect(roster).toEqual([null, null]); - }); - }); - - describe('(required string item, initially non-empty)', () => { - const rosterPath = 'spec.arr.string.req.nonEmpty'; + describe('(required string item)', () => { + const rosterPath = 'spec.arr.string.req'; it('should change item value on text input', async () => { const textInput = findTextFieldInput(findRosterItem(editor, rosterPath, 0), '$value'); @@ -184,34 +137,11 @@ describe('ParametricEditorRosterWidget', () => { expect(roster).toEqual(['foo', '', '']); }); }); - - describe('(required string item, initially empty)', () => { - const rosterPath = 'spec.arr.string.req.empty'; - - it('should add empty string item on Add button click', async () => { - const addButton = findRosterAddButton(editor, rosterPath); - - await userEvent.click(addButton); - - const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); - expect(roster).toEqual(['']); - }); - - it('should add empty string item on Add button click (x2)', async () => { - const addButton = findRosterAddButton(editor, rosterPath); - - await userEvent.click(addButton); - await userEvent.click(addButton); - - const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); - expect(roster).toEqual(['', '']); - }); - }); }); describe('object-based', () => { - describe('(optional string item, initially non-empty)', () => { - const rosterPath = 'spec.obj.string.opt.nonEmpty'; + describe('(optional string item)', () => { + const rosterPath = 'spec.obj.string.opt'; it('should change item key on text input', async () => { const textInput = findTextFieldInput(findRosterItem(editor, rosterPath, 0), '$key'); @@ -249,29 +179,8 @@ describe('ParametricEditorRosterWidget', () => { }); }); - describe('(optional string item, initially empty)', () => { - const rosterPath = 'spec.obj.string.opt.empty'; - - it('should add null item on Add button click', async () => { - const addButton = findRosterAddButton(editor, rosterPath); - - await userEvent.click(addButton); - - const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); - expect(roster).toEqual({ '': null }); - }); - - it('should disable Add button when item with empty key is added', async () => { - const addButton = findRosterAddButton(editor, rosterPath); - - await userEvent.click(addButton); - - expect(addButton.disabled).toBeTruthy(); - }); - }); - - describe('(required string item, initially non-empty)', () => { - const rosterPath = 'spec.obj.string.req.nonEmpty'; + describe('(required string item)', () => { + const rosterPath = 'spec.obj.string.req'; it('should change item key on text input', async () => { const textInput = findTextFieldInput(findRosterItem(editor, rosterPath, 0), '$key'); @@ -308,26 +217,5 @@ describe('ParametricEditorRosterWidget', () => { expect(addButton.disabled).toBeTruthy(); }); }); - - describe('(required string item, initially empty)', () => { - const rosterPath = 'spec.obj.string.req.empty'; - - it('should add empty string item on Add button click', async () => { - const addButton = findRosterAddButton(editor, rosterPath); - - await userEvent.click(addButton); - - const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); - expect(roster).toEqual({ '': '' }); - }); - - it('should disable Add button when item with empty key is added', async () => { - const addButton = findRosterAddButton(editor, rosterPath); - - await userEvent.click(addButton); - - expect(addButton.disabled).toBeTruthy(); - }); - }); }); }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts index fd97f0ba..2abab7e8 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts @@ -46,8 +46,8 @@ export const objectTypeRosterConfigurationEntry = ( { name }, { type: PxeConfigurationEntryType.Roster, - values: [{ path, type: PxeValueType.Object, isRequired, display: { name } }], - itemValue: resolveItemValueDescriptor(name, itemValue ?? null, itemEntries), + valueDescriptors: [{ path, type: PxeValueType.Object, isRequired, display: { name } }], + itemValueDescriptor: resolveItemValueDescriptor(name, itemValue ?? null, itemEntries), itemEntries, }, ); @@ -70,8 +70,8 @@ export const arrayTypeRosterConfigurationEntry = ( { name }, { type: PxeConfigurationEntryType.Roster, - values: [{ path, type: PxeValueType.Array, isRequired, display: { name } }], - itemValue: resolveItemValueDescriptor(name, itemValue ?? null, itemEntries), + valueDescriptors: [{ path, type: PxeValueType.Array, isRequired, display: { name } }], + itemValueDescriptor: resolveItemValueDescriptor(name, itemValue ?? null, itemEntries), itemEntries, }, ); @@ -83,7 +83,7 @@ const resolveItemValueDescriptor = ( ): PxeValueDescriptor => { const itemValueEntry = findInConfigurationEntries( itemEntries, - entry => 'values' in entry && entry.values[0]?.path === '$value', + entry => 'valueDescriptors' in entry && entry.valueDescriptors[0]?.path === '$value', ) as PxeWidgetEntry | null; if (explicitItemValueDescriptor && itemValueEntry) { @@ -91,6 +91,6 @@ const resolveItemValueDescriptor = ( } else if (!explicitItemValueDescriptor && !itemValueEntry) { throw new Error(`No itemValue definition in roster ${rosterName}.`); } else { - return explicitItemValueDescriptor || itemValueEntry?.values[0]!; + return explicitItemValueDescriptor || itemValueEntry?.valueDescriptors[0]!; } }; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts index 8854a4f3..a3ee75c0 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts @@ -35,6 +35,6 @@ export const selectValueWidgetConfigurationEntry = ({ options: readonly PxeValueOption[]; }): PxeSelectValueWidgetEntry => ({ type: PxeConfigurationEntryType.SelectValue, - values: [{ path, type, isRequired, display: { name } }], + valueDescriptors: [{ path, type, isRequired, display: { name } }], options, }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/singleLineTextWidgetConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/singleLineTextWidgetConfigurationEntry.ts index 260b0b7f..e5ecb295 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/singleLineTextWidgetConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/singleLineTextWidgetConfigurationEntry.ts @@ -31,7 +31,7 @@ export const singleLineTextWidgetConfigurationEntry = ({ textFilter?: TextFilter; }): PxeSingleLineTextWidgetEntry => ({ type: PxeConfigurationEntryType.SingleLineText, - values: [{ path, type, isRequired, display: { name } }], + valueDescriptors: [{ path, type, isRequired, display: { name } }], textFilter, }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx index 437d629e..4253446d 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx @@ -43,8 +43,11 @@ export const PxeRosterWidgetNode: React.FC = ({ onResourceChangeRequest, parentExpandedSectionState, }) => { - const { values, itemEntries, itemValue: itemValueDescriptor } = configurationEntry as PxeRosterWidgetEntry; - const valueDescriptor = values[0]; + const { + valueDescriptors: [valueDescriptor], + itemValueDescriptor: itemValueDescriptor, + itemEntries, + } = configurationEntry as PxeRosterWidgetEntry; const rosterValueType = valueDescriptor.type as RosterValueType; const itemChunks = itemChunksFromValue(get(resourceChunk, valueDescriptor.path)); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx index 5af98385..18c8a742 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx @@ -29,7 +29,7 @@ export const PxeSelectValueWidgetNode: React.FC = resourceChunk, }) => { const widgetEntry = configurationEntry as PxeSelectValueWidgetEntry; - const valueDescriptor = widgetEntry.values[0]; + const [valueDescriptor] = widgetEntry.valueDescriptors; const selectItems = widgetEntry.options.map(({ value, label }) => ({ value: value !== undefined ? String(value) : DEFAULT_VALUE, diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx index 3f8a27c9..3a99cdbe 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx @@ -28,7 +28,7 @@ export const PxeSingleLineTextWidgetNode: React.FC }) => { const { textFilter, - values: [valueDescriptor], + valueDescriptors: [valueDescriptor], } = configurationEntry as PxeSingleLineTextWidgetEntry; return ( diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeConfiguration.types.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeConfiguration.types.ts index f87988c0..8ddbabcc 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeConfiguration.types.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeConfiguration.types.ts @@ -77,12 +77,12 @@ export type PxeWidgetEntry = PxeRosterWidgetEntry | PxeSingleLineTextWidgetEntry type PxeWidgetEntryBase = { readonly type: PxeConfigurationEntryType; - readonly values: readonly PxeValueDescriptor[]; + readonly valueDescriptors: readonly PxeValueDescriptor[]; }; export interface PxeRosterWidgetEntry extends PxeWidgetEntryBase { readonly type: PxeConfigurationEntryType.Roster; - readonly itemValue: PxeValueDescriptor; + readonly itemValueDescriptor: PxeValueDescriptor; readonly itemEntries: readonly PxeConfigurationEntry[]; } diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts index 05e5fc75..20cffc2c 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts @@ -47,7 +47,7 @@ export const generateValueLabel = (valueDescriptor: PxeValueDescriptor, uppercas }; const generateValueDescriptionsForWidget = (widgetEntry: PxeWidgetEntry, resourceChunk: PxeResourceChunk): string[] => - widgetEntry.values + widgetEntry.valueDescriptors .map(valueDescriptor => generateValueDescription(valueDescriptor, resourceChunk)) .filter(segment => segment !== null) as string[]; From 463c67ee0c34a11f5c14f23b8ab8b329d6416dfd Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Tue, 12 Nov 2024 19:09:14 +0100 Subject: [PATCH 06/19] Green tests; key conflict in roster fixed. Roster refactoring would be welcome. --- .../ParametricEditorRosterWidget.test.tsx | 48 +++++++++++++++++++ .../nodes/widget/PxeRosterWidgetNode.tsx | 24 ++++++++-- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx index d48dde9c..0db3bfe6 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorRosterWidget.test.tsx @@ -177,6 +177,30 @@ describe('ParametricEditorRosterWidget', () => { expect(addButton.disabled).toBeTruthy(); }); + + it('should not update resource when roster is in illegal state (duplicate key)', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + const keyTextInput = findTextFieldInput(findRosterItem(editor, rosterPath, 1), '$key'); + await userEvent.type(keyTextInput, 'foo'); + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + + expect(resourceChangeHandler.mock.calls.length).toBe(3); + expect(roster).toEqual({ foo: 'bar', fo: null }); + }); + + it('should recover from illegal state (duplicate key) when duplicate key is edited', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + const keyTextInput = findTextFieldInput(findRosterItem(editor, rosterPath, 1), '$key'); + await userEvent.type(keyTextInput, 'foo2'); + + expect(resourceChangeHandler.mock.calls.length).toBe(4); + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual({ foo: 'bar', foo2: null }); + }); }); describe('(required string item)', () => { @@ -216,6 +240,30 @@ describe('ParametricEditorRosterWidget', () => { expect(addButton.disabled).toBeTruthy(); }); + + it('should not update resource when roster is in illegal state (duplicate key)', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + const keyTextInput = findTextFieldInput(findRosterItem(editor, rosterPath, 1), '$key'); + await userEvent.type(keyTextInput, 'foo'); + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + + expect(resourceChangeHandler.mock.calls.length).toBe(3); + expect(roster).toEqual({ foo: 'bar', fo: '' }); + }); + + it('should recover from illegal state (duplicate key) when duplicate key is edited', async () => { + const addButton = findRosterAddButton(editor, rosterPath); + + await userEvent.click(addButton); + const keyTextInput = findTextFieldInput(findRosterItem(editor, rosterPath, 1), '$key'); + await userEvent.type(keyTextInput, 'foo2'); + + expect(resourceChangeHandler.mock.calls.length).toBe(4); + const roster = get(getLastResourceState(resourceChangeHandler), rosterPath); + expect(roster).toEqual({ foo: 'bar', foo2: '' }); + }); }); }); }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx index 4253446d..a88361d3 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx @@ -18,7 +18,7 @@ import { Button, makeStyles } from '@material-ui/core'; import AddIcon from '@material-ui/icons/Add'; import DeleteIcon from '@material-ui/icons/Delete'; import { get } from 'lodash'; -import React, { Fragment } from 'react'; +import React, { Fragment, useEffect, useState } from 'react'; import { IconButton } from '../../../../../Controls'; import { useEditorStyles } from '../../../FirstClassEditors/styles'; import { PxeParametricEditorNode, PxeParametricEditorNodeProps } from '../../PxeParametricEditorNode'; @@ -50,7 +50,13 @@ export const PxeRosterWidgetNode: React.FC = ({ } = configurationEntry as PxeRosterWidgetEntry; const rosterValueType = valueDescriptor.type as RosterValueType; - const itemChunks = itemChunksFromValue(get(resourceChunk, valueDescriptor.path)); + const [itemChunks, setItemChunks] = useState( + itemChunksFromValue(get(resourceChunk, valueDescriptor.path)), + ); + + useEffect(() => { + setItemChunks(itemChunksFromValue(get(resourceChunk, valueDescriptor.path))); + }, [resourceChunk, valueDescriptor]); const handleResourceChangeRequestForItem = (itemIndex: number, changeRequest: PxeResourceChangeRequest) => { const newItemChunk = createResourceChunkAfterChangeRequest( @@ -58,8 +64,13 @@ export const PxeRosterWidgetNode: React.FC = ({ changeRequest, ) as RosterItemResourceChunk; - const newValue = valueFromItemChunks(arrayWithItemReplaced(itemChunks, itemIndex, newItemChunk), rosterValueType); - onResourceChangeRequest({ valueDescriptor, newValue }); + const newItemChunks = arrayWithItemReplaced(itemChunks, itemIndex, newItemChunk); + const newValue = valueFromItemChunks(newItemChunks, rosterValueType); + if (Object.values(newValue).length === itemChunks.length) { + onResourceChangeRequest({ valueDescriptor, newValue }); + } else { + setItemChunks(newItemChunks); + } }; const handleItemAddition = () => { @@ -143,7 +154,10 @@ const itemChunksFromValue = (value: PxeValue): readonly RosterItemResourceChunk[ $value: itemValue as PxeValue, })); -const valueFromItemChunks = (itemChunks: readonly RosterItemResourceChunk[], rosterType: RosterValueType): PxeValue => +const valueFromItemChunks = ( + itemChunks: readonly RosterItemResourceChunk[], + rosterType: RosterValueType, +): object | readonly any[] => rosterType === PxeValueType.Object ? Object.fromEntries(itemChunks.map(itemChunk => [itemChunk.$key, itemChunk.$value])) : itemChunks.map(itemChunk => itemChunk.$value); From a208989355bc45b26c5244b9b456e7b1c019d173 Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Wed, 13 Nov 2024 12:16:54 +0100 Subject: [PATCH 07/19] Editor configuration fixes. --- .../NephioWorkloadClusterParametricEditor.tsx | 5 ++++- .../partial/metadataEditorSection.ts | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx index 16b60e30..9fc9dbee 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioWorkloadClusterParametricEditor.tsx @@ -28,7 +28,10 @@ export const NephioWorkloadClusterParametricEditor = createEditorFromConfigurati { name: 'Configuration' }, singleLineText({ path: 'spec.clusterName', isRequired: true }), singleLineText({ path: 'spec.masterInterface' }), - arrayTypeRoster({ name: 'CNIs', path: 'spec.cnis', isRequired: false }, singleLineText({ path: '$value' })), + arrayTypeRoster( + { name: 'CNIs', path: 'spec.cnis', isRequired: false }, + singleLineText({ path: '$value', isRequired: true }), + ), ), ], }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts index 4ae032d0..950d632a 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/partial/metadataEditorSection.ts @@ -26,10 +26,10 @@ export const metadataEditorSection = ({ isNamespacedResource }: { isNamespacedRe ...(isNamespacedResource ? [singleLineText({ path: 'metadata.namespace' })] : []), objectTypeRoster( { name: 'Labels', path: 'metadata.labels', isRequired: false }, - rowLayout(singleLineText({ path: '$key', isRequired: true }), singleLineText({ path: '$value' })), + rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: true })), ), objectTypeRoster( { name: 'Annotations', path: 'metadata.annotations', isRequired: false }, - rowLayout(singleLineText({ path: '$key', isRequired: true }), singleLineText({ path: '$value' })), + rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: true })), ), ); From f4aa020fc772d90f9d38bc066f3c16313760dbaf Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Thu, 14 Nov 2024 19:55:42 +0100 Subject: [PATCH 08/19] First working version of nephio.infra Network editor. --- .../NephioNetworkParametricEditor.tsx | 142 +++++++++++++++--- .../configuration/rosterConfigurationEntry.ts | 32 ++-- .../selectValueWidgetConfigurationEntry.ts | 4 +- .../utils/generateLabelsForWidgets.ts | 5 +- 4 files changed, 145 insertions(+), 38 deletions(-) diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx index 3f750f76..8e0df446 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/ParametricFirstClassEditors/NephioNetworkParametricEditor.tsx @@ -19,7 +19,24 @@ import { PxeConfigurationFactory } from '../PxeParametricEditor/configuration'; import { metadataEditorSection } from './partial/metadataEditorSection'; import { PxeValueType } from '../PxeParametricEditor/types/PxeConfiguration.types'; -const { section, rowLayout, arrayTypeRoster, objectTypeRoster, singleLineText } = PxeConfigurationFactory; +const { section, rowLayout, arrayTypeRoster, objectTypeRoster, selectValue, singleLineText } = PxeConfigurationFactory; + +const INTERFACE_KIND_OPTIONS = [ + { value: 'interface', label: 'Interface' }, + { value: 'bridgedomain', label: 'Bridge domain' }, +]; + +const INTERFACE_ATTACHMENT_TYPE_OPTIONS = [ + { value: undefined, label: 'None' }, + { value: 'vlan', label: 'VLAN' }, +]; + +const SELECTOR_OPERATOR_OPTIONS = [ + { value: 'In', label: 'In' }, + { value: 'NotIn', label: 'Not In' }, + { value: 'Exists', label: 'Exists' }, + { value: 'DoesNotExist', label: 'Does Not Exist' }, +]; export const NephioNetworkParametricEditor = createEditorFromConfiguration({ topLevelProperties: ['metadata', 'spec'], @@ -31,31 +48,110 @@ export const NephioNetworkParametricEditor = createEditorFromConfiguration({ name: 'Routing tables', path: 'spec.routingTables', isRequired: false, - // FIXME Not a best way of doing this. E.g path is redundant. - itemValue: { type: PxeValueType.Object, isRequired: true, path: '$value' }, + item: { type: PxeValueType.Object, isRequired: true }, + }, + routingTableItemConfiguration(), + ), + arrayTypeRoster( + { + name: 'Bridge domains', + path: 'spec.bridgeDomains', + isRequired: false, + item: { type: PxeValueType.Object, isRequired: true }, + }, + bridgeDomainItemConfiguration(), + ), + ], +}); + +function routingTableItemConfiguration() { + return section( + { name: 'Routing table' }, + singleLineText({ path: '$value.name', isRequired: true }), + arrayTypeRoster( + { + name: 'Prefixes', + path: '$value.prefixes', + isRequired: true, + item: { type: PxeValueType.Object, isRequired: true }, + }, + prefixItemConfiguration(), + ), + arrayTypeRoster( + { + name: 'Interfaces', + path: '$value.interfaces', + item: { type: PxeValueType.Object, isRequired: true }, + }, + interfaceItemConfiguration(), + ), + ); +} + +function bridgeDomainItemConfiguration() { + return section( + { name: 'Bridge domain' }, + singleLineText({ path: '$value.name', isRequired: true }), + arrayTypeRoster( + { + name: 'Interfaces', + path: '$value.interfaces', + item: { type: PxeValueType.Object, isRequired: true }, + }, + interfaceItemConfiguration(), + ), + ); +} + +function interfaceItemConfiguration() { + return section( + { name: 'Interface' }, + // TODO Different options depending on where used. + selectValue({ path: '$value.kind', isRequired: true, options: INTERFACE_KIND_OPTIONS }), + singleLineText({ path: '$value.interfaceName' }), + singleLineText({ path: '$value.bridgeDomainName' }), + singleLineText({ path: '$value.nodeName' }), + selectValue({ path: '$value.attachmentType', options: INTERFACE_ATTACHMENT_TYPE_OPTIONS }), + ...selectorConfiguration('$value.selector'), + ); +} + +function prefixItemConfiguration() { + return section( + { name: 'Prefix' }, + singleLineText({ path: '$value.prefix', isRequired: true }), + objectTypeRoster( + { name: 'Labels', path: '$value.labels' }, + rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value', isRequired: true })), + ), + ); +} + +// FIXME Extract. +function selectorConfiguration(pathPrefix: string) { + return [ + objectTypeRoster( + { name: 'Match labels', path: `${pathPrefix}.matchLabels` }, + rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value' })), + ), + arrayTypeRoster( + { + name: 'Match expressions', + path: `${pathPrefix}.matchExpressions`, + item: { type: PxeValueType.Object, isRequired: true }, }, section( - { name: 'Routing table' }, - singleLineText({ path: 'value.name', isRequired: true }), + { name: 'Match expression' }, + rowLayout( + singleLineText({ path: '$value.key', isRequired: true }), + selectValue({ path: '$value.operator', isRequired: true, options: SELECTOR_OPERATOR_OPTIONS }), + ), + // TODO Different value requirements for different operators. arrayTypeRoster( - { - name: 'Prefixes', - path: 'value.prefixes', - isRequired: true, - // FIXME Not a best way of doing this. E.g path is redundant. - itemValue: { type: PxeValueType.Object, isRequired: true, path: '$value' }, - }, - section( - { name: 'Prefix' }, - singleLineText({ path: 'value.prefix', isRequired: true }), - objectTypeRoster( - { name: 'Labels', path: 'value.labels', isRequired: false }, - rowLayout(singleLineText({ path: '$key', isRequired: true }), singleLineText({ path: '$value' })), - ), - ), + { name: 'Values', path: '$value.values' }, + singleLineText({ path: '$value', isRequired: true }), ), ), ), - section({ name: 'Bridge domains' }), - ], -}); + ]; +} diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts index 2abab7e8..458ee29d 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts @@ -25,6 +25,12 @@ import { import { sectionConfigurationEntry } from './sectionConfigurationEntry'; import { findInConfigurationEntries } from '../utils/findInConfigurationEntries'; +// FIXME Think of something different (name? form?). +type RosterItemDefinition = { + readonly type: PxeValueType; + readonly isRequired?: boolean; +}; + // TODO With the current UI every roster entry is wrapped in implicit section entry. // Also, name parameter is mandatory as it iss used for displaying section name. // Change this after UI redesign. @@ -33,12 +39,12 @@ export const objectTypeRosterConfigurationEntry = ( name, path, isRequired = false, - itemValue, + item, }: { name: string; path: string; isRequired?: boolean; - itemValue?: PxeValueDescriptor; + item?: RosterItemDefinition; }, ...itemEntries: PxeConfigurationEntry[] ): PxeSectionEntry => @@ -47,7 +53,7 @@ export const objectTypeRosterConfigurationEntry = ( { type: PxeConfigurationEntryType.Roster, valueDescriptors: [{ path, type: PxeValueType.Object, isRequired, display: { name } }], - itemValueDescriptor: resolveItemValueDescriptor(name, itemValue ?? null, itemEntries), + itemValueDescriptor: resolveItemValueDescriptor(name, item ?? null, itemEntries), itemEntries, }, ); @@ -57,12 +63,12 @@ export const arrayTypeRosterConfigurationEntry = ( name, path, isRequired = false, - itemValue, + item, }: { name: string; path: string; isRequired?: boolean; - itemValue?: PxeValueDescriptor; + item?: RosterItemDefinition; }, ...itemEntries: PxeConfigurationEntry[] ): PxeSectionEntry => @@ -71,14 +77,14 @@ export const arrayTypeRosterConfigurationEntry = ( { type: PxeConfigurationEntryType.Roster, valueDescriptors: [{ path, type: PxeValueType.Array, isRequired, display: { name } }], - itemValueDescriptor: resolveItemValueDescriptor(name, itemValue ?? null, itemEntries), + itemValueDescriptor: resolveItemValueDescriptor(name, item ?? null, itemEntries), itemEntries, }, ); const resolveItemValueDescriptor = ( rosterName: string, - explicitItemValueDescriptor: PxeValueDescriptor | null, + explicitRosterItemDef: RosterItemDefinition | null, itemEntries: PxeConfigurationEntry[], ): PxeValueDescriptor => { const itemValueEntry = findInConfigurationEntries( @@ -86,11 +92,13 @@ const resolveItemValueDescriptor = ( entry => 'valueDescriptors' in entry && entry.valueDescriptors[0]?.path === '$value', ) as PxeWidgetEntry | null; - if (explicitItemValueDescriptor && itemValueEntry) { - throw new Error(`Redundant itemValue definition in roster ${rosterName}. Descriptor inherited from $value entry.`); - } else if (!explicitItemValueDescriptor && !itemValueEntry) { - throw new Error(`No itemValue definition in roster ${rosterName}.`); + if (explicitRosterItemDef && itemValueEntry) { + throw new Error(`Redundant item definition in roster ${rosterName}. Descriptor inherited from $value entry.`); + } else if (!explicitRosterItemDef && !itemValueEntry) { + throw new Error(`No item definition in roster ${rosterName}.`); } else { - return explicitItemValueDescriptor || itemValueEntry?.valueDescriptors[0]!; + return explicitRosterItemDef + ? { path: '$value', type: explicitRosterItemDef.type, isRequired: explicitRosterItemDef.isRequired ?? false } + : itemValueEntry?.valueDescriptors[0]!; } }; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts index a3ee75c0..d6960dce 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts @@ -23,13 +23,13 @@ import { export const selectValueWidgetConfigurationEntry = ({ path, - type, + type = PxeValueType.String, isRequired = false, name, options, }: { path: string; - type: PxeValueType; + type?: PxeValueType; isRequired?: boolean; name?: string; options: readonly PxeValueOption[]; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts index 20cffc2c..80d3c3ca 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts @@ -32,7 +32,10 @@ const FALLBACK_DEFAULT_VALUE_NAME = 'Value'; export const generateSectionDescription = (sectionEntry: PxeSectionEntry, resourceChunk: PxeResourceChunk): string => sectionEntry.entries .filter(childEntry => childEntry.type !== PxeConfigurationEntryType.Section) - .flatMap(childEntry => generateValueDescriptionsForWidget(childEntry as PxeWidgetEntry, resourceChunk)) + // FIXME 1) needs type predicates (widget / layout), 2) handle layouts + .flatMap(childEntry => + 'valueDescriptors' in childEntry ? generateValueDescriptionsForWidget(childEntry, resourceChunk) : [], + ) .join(', '); export const generateValueLabel = (valueDescriptor: PxeValueDescriptor, uppercase: boolean = true): string => { From ae0f2125ab59e76864d541ebdc8602df81fe6410 Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Wed, 20 Nov 2024 13:13:21 +0100 Subject: [PATCH 09/19] Created diagnostic context for reporting of re-renders by configurable editor components. Created first performance tests which showcase the problem of unneeded re-renders. --- .../PxeDiagnosticsContext.ts | 28 ++++++ .../PxeParametricEditor.tsx | 19 ++-- .../PxeParametricEditorNode.tsx | 16 +-- .../PxeParametricEditorNodeList.tsx | 4 +- .../PxeParametricEditor/PxeResourceContext.ts | 20 ++++ .../ParametricEditorPerformance.test.tsx | 98 +++++++++++++++++++ .../configuration/rosterConfigurationEntry.ts | 6 +- .../rowLayoutConfigurationEntry.ts | 4 +- .../sectionConfigurationEntry.ts | 4 +- .../selectValueWidgetConfigurationEntry.ts | 9 +- .../singleLineTextWidgetConfigurationEntry.ts | 4 +- .../createEditorFromConfiguration.tsx | 12 ++- .../nodes/PxeSectionNode.tsx | 10 +- .../nodes/layout/PxeRowLayoutNode.tsx | 2 + .../nodes/widget/PxeRosterWidgetNode.tsx | 16 +-- .../nodes/widget/PxeSelectValueWidgetNode.tsx | 2 + .../widget/PxeSingleLineTextWidgetNode.tsx | 2 + .../types/PxeConfiguration.types.ts | 14 +-- .../types/PxeDiagnostics.types.ts | 20 ++++ .../utils/generateLabelsForWidgets.ts | 4 +- 20 files changed, 241 insertions(+), 53 deletions(-) create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeDiagnosticsContext.ts create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeResourceContext.ts create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorPerformance.test.tsx create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeDiagnostics.types.ts diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeDiagnosticsContext.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeDiagnosticsContext.ts new file mode 100644 index 00000000..94f9b10a --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeDiagnosticsContext.ts @@ -0,0 +1,28 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createContext, useContext } from 'react'; +import { PxeDiagnosticsReporter } from './types/PxeDiagnostics.types'; +import { PxeConfigurationEntry } from './types/PxeConfiguration.types'; + +export const PxeDiagnosticsContext = createContext(null); + +export const useDiagnostics = (configurationEntry: PxeConfigurationEntry) => { + const diagnosticsReporter = useContext(PxeDiagnosticsContext); + if (diagnosticsReporter) { + diagnosticsReporter.reportRender(configurationEntry); + } +}; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditor.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditor.tsx index 897efd19..8945458c 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditor.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditor.tsx @@ -20,20 +20,24 @@ import { useSetStateAndCall } from '../../../../hooks/useSetStateAndCall'; import { useEditorStyles } from '../FirstClassEditors/styles'; import { PxeConfiguration } from './types/PxeConfiguration.types'; import { PxeExpandedSectionState, PxeResourceChangeRequestHandler } from './types/PxeParametricEditor.types'; +import { PxeDiagnosticsReporter } from './types/PxeDiagnostics.types'; import { parseYaml, stringifyYaml } from './utils/yamlConversion'; import { createResourceChunkAfterChangeRequest } from './utils/createResourceChunkAfterChangeRequest'; import { PxeParametricEditorNodeList } from './PxeParametricEditorNodeList'; +import { PxeDiagnosticsContext } from './PxeDiagnosticsContext'; export type PxeParametricEditorProps = { readonly configuration: PxeConfiguration; readonly yamlText: string; readonly onResourceChange: (yaml: string) => void; + readonly __diagnosticsReporter?: PxeDiagnosticsReporter; }; export const PxeParametricEditor: React.FC = ({ configuration: { topLevelProperties, entries }, yamlText, onResourceChange, + __diagnosticsReporter, }) => { const { yamlObject: initialYamlObject } = parseYaml(yamlText); const initialResourceState = pick(initialYamlObject, topLevelProperties); @@ -53,14 +57,17 @@ export const PxeParametricEditor: React.FC = ({ ); const classes = useEditorStyles(); + return (
- + + +
); }; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNode.tsx index 8c705505..45ff5ae2 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNode.tsx @@ -20,7 +20,7 @@ import { PxeResourceChangeRequestHandler, PxeResourceChunk, } from './types/PxeParametricEditor.types'; -import { PxeConfigurationEntry, PxeConfigurationEntryType } from './types/PxeConfiguration.types'; +import { PxeConfigurationEntry, PxeNodeType } from './types/PxeConfiguration.types'; import { PxeSectionNode } from './nodes/PxeSectionNode'; import { PxeRowLayoutNode } from './nodes/layout/PxeRowLayoutNode'; import { PxeRosterWidgetNode } from './nodes/widget/PxeRosterWidgetNode'; @@ -34,19 +34,19 @@ export type PxeParametricEditorNodeProps = { readonly parentExpandedSectionState?: PxeExpandedSectionStateTuple; }; -const NODE_BY_ENTRY_TYPE_RECORD: Record> = { - [PxeConfigurationEntryType.Section]: PxeSectionNode, +const NODE_BY_TYPE_RECORD: Record> = { + [PxeNodeType.Section]: PxeSectionNode, - [PxeConfigurationEntryType.RowLayout]: PxeRowLayoutNode, + [PxeNodeType.RowLayout]: PxeRowLayoutNode, - [PxeConfigurationEntryType.Roster]: PxeRosterWidgetNode, - [PxeConfigurationEntryType.SingleLineText]: PxeSingleLineTextWidgetNode, - [PxeConfigurationEntryType.SelectValue]: PxeSelectValueWidgetNode, + [PxeNodeType.Roster]: PxeRosterWidgetNode, + [PxeNodeType.SingleLineText]: PxeSingleLineTextWidgetNode, + [PxeNodeType.SelectValue]: PxeSelectValueWidgetNode, }; export const PxeParametricEditorNode: React.FC = props => { const { configurationEntry } = props; - const ConcreteEditorNode = NODE_BY_ENTRY_TYPE_RECORD[configurationEntry.type]; + const ConcreteEditorNode = NODE_BY_TYPE_RECORD[configurationEntry.type]; return ; }; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNodeList.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNodeList.tsx index 43aece1e..c4d73637 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNodeList.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNodeList.tsx @@ -18,7 +18,7 @@ import React, { Fragment } from 'react'; import { renderGroupedArray } from './utils/rendering/renderGroupedArray'; import { PxeParametricEditorNode } from './PxeParametricEditorNode'; import { chunkByTrait } from './utils/general/chunkByTrait'; -import { PxeConfigurationEntry, PxeConfigurationEntryType } from './types/PxeConfiguration.types'; +import { PxeConfigurationEntry, PxeNodeType } from './types/PxeConfiguration.types'; import { PxeExpandedSectionStateTuple, PxeResourceChangeRequestHandler, @@ -38,7 +38,7 @@ export const PxeParametricEditorNodeList: React.FC { - const groupedEntries = chunkByTrait(entries, entry => entry.type === PxeConfigurationEntryType.Section || null); + const groupedEntries = chunkByTrait(entries, entry => entry.type === PxeNodeType.Section || null); return ( diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeResourceContext.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeResourceContext.ts new file mode 100644 index 00000000..775fb261 --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeResourceContext.ts @@ -0,0 +1,20 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createContext } from 'react'; +import { PxeResourceChunk } from './types/PxeParametricEditor.types'; + +export const PxeResourceContext = createContext({}); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorPerformance.test.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorPerformance.test.tsx new file mode 100644 index 00000000..db6b9d47 --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorPerformance.test.tsx @@ -0,0 +1,98 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { renderWithEffects } from '@backstage/test-utils'; +import { RenderResult } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; +import { noop } from 'lodash'; +import React from 'react'; +import { findTextFieldInput } from './testUtils/findEditorElement'; +import { PxeConfiguration } from '../types/PxeConfiguration.types'; +import { PxeConfigurationFactory } from '../configuration'; +import { PxeParametricEditor } from '../PxeParametricEditor'; + +const { arrayTypeRoster, rowLayout, section, selectValue, singleLineText, objectTypeRoster } = PxeConfigurationFactory; + +const CONFIGURATION: PxeConfiguration = { + topLevelProperties: ['spec'], + entries: [ + section( + { name: 'Section' }, + rowLayout( + singleLineText({ path: 'spec.singleLineText' }), + selectValue({ path: 'spec.selectValue', options: [{ value: 'foo', label: 'foo' }] }), + ), + arrayTypeRoster({ name: 'Array', path: 'spec.arrayTypeRoster' }, singleLineText({ path: '$value' })), + objectTypeRoster( + { name: 'Array', path: 'spec.objectTypeRoster' }, + rowLayout(singleLineText({ path: '$key' }), singleLineText({ path: '$value' })), + ), + ), + ], +}; + +const YAML = ` +spec: + singleLineText: "foo" + selectValue: "foo" + arrayTypeRoster: + - foo + objectTypeRoster: + foo: bar +`; + +describe('PxeParametricEditor performance', () => { + const INITIAL_NON_ITEM_NODE_COUNT = 6; + const INITIAL_ROSTER_SECTION_COUNT = 2; + const INITIAL_ITEM_NODE_COUNT = 4; + const INITIAL_NODE_COUNT = INITIAL_NON_ITEM_NODE_COUNT + INITIAL_ITEM_NODE_COUNT + INITIAL_ROSTER_SECTION_COUNT; + + let editor: RenderResult; + let renderReporter: jest.Mock; + + beforeEach(async () => { + renderReporter = jest.fn(); + + editor = await renderWithEffects( + , + ); + }); + + it('should render with minimal number of component renders', () => { + expect(renderReporter.mock.calls.length).toBe(INITIAL_NODE_COUNT); + }); + + it('should rerender text widget when single character is inputted', async () => { + const textInput = findTextFieldInput(editor, `spec.singleLineText`); + + await userEvent.type(textInput, 'a'); + + expect(renderReporter.mock.calls.length).toBe(INITIAL_NODE_COUNT + 1); + }); + + it('should rerender text widget multiple times when multiple characters are inputted', async () => { + const textInput = findTextFieldInput(editor, `spec.singleLineText`); + + await userEvent.type(textInput, 'aaa'); + + expect(renderReporter.mock.calls.length).toBe(INITIAL_NODE_COUNT + 3); + }); +}); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts index 458ee29d..03dd2aed 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts @@ -16,7 +16,7 @@ import { PxeConfigurationEntry, - PxeConfigurationEntryType, + PxeNodeType, PxeSectionEntry, PxeValueDescriptor, PxeValueType, @@ -51,7 +51,7 @@ export const objectTypeRosterConfigurationEntry = ( sectionConfigurationEntry( { name }, { - type: PxeConfigurationEntryType.Roster, + type: PxeNodeType.Roster, valueDescriptors: [{ path, type: PxeValueType.Object, isRequired, display: { name } }], itemValueDescriptor: resolveItemValueDescriptor(name, item ?? null, itemEntries), itemEntries, @@ -75,7 +75,7 @@ export const arrayTypeRosterConfigurationEntry = ( sectionConfigurationEntry( { name }, { - type: PxeConfigurationEntryType.Roster, + type: PxeNodeType.Roster, valueDescriptors: [{ path, type: PxeValueType.Array, isRequired, display: { name } }], itemValueDescriptor: resolveItemValueDescriptor(name, item ?? null, itemEntries), itemEntries, diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rowLayoutConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rowLayoutConfigurationEntry.ts index 2c992c55..5cd8c740 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rowLayoutConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rowLayoutConfigurationEntry.ts @@ -14,9 +14,9 @@ * limitations under the License. */ -import { PxeConfigurationEntry, PxeConfigurationEntryType, PxeRowLayoutEntry } from '../types/PxeConfiguration.types'; +import { PxeConfigurationEntry, PxeNodeType, PxeRowLayoutEntry } from '../types/PxeConfiguration.types'; export const rowLayoutConfigurationEntry = (...childEntries: PxeConfigurationEntry[]): PxeRowLayoutEntry => ({ - type: PxeConfigurationEntryType.RowLayout, + type: PxeNodeType.RowLayout, entries: childEntries, }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/sectionConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/sectionConfigurationEntry.ts index 518ac0c1..6cff8af5 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/sectionConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/sectionConfigurationEntry.ts @@ -14,13 +14,13 @@ * limitations under the License. */ -import { PxeConfigurationEntry, PxeConfigurationEntryType, PxeSectionEntry } from '../types/PxeConfiguration.types'; +import { PxeConfigurationEntry, PxeNodeType, PxeSectionEntry } from '../types/PxeConfiguration.types'; export const sectionConfigurationEntry = ( { name }: { name: string }, ...childEntries: PxeConfigurationEntry[] ): PxeSectionEntry => ({ - type: PxeConfigurationEntryType.Section, + type: PxeNodeType.Section, name, entries: childEntries, }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts index d6960dce..48ac517b 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts @@ -14,12 +14,7 @@ * limitations under the License. */ -import { - PxeConfigurationEntryType, - PxeSelectValueWidgetEntry, - PxeValueOption, - PxeValueType, -} from '../types/PxeConfiguration.types'; +import { PxeNodeType, PxeSelectValueWidgetEntry, PxeValueOption, PxeValueType } from '../types/PxeConfiguration.types'; export const selectValueWidgetConfigurationEntry = ({ path, @@ -34,7 +29,7 @@ export const selectValueWidgetConfigurationEntry = ({ name?: string; options: readonly PxeValueOption[]; }): PxeSelectValueWidgetEntry => ({ - type: PxeConfigurationEntryType.SelectValue, + type: PxeNodeType.SelectValue, valueDescriptors: [{ path, type, isRequired, display: { name } }], options, }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/singleLineTextWidgetConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/singleLineTextWidgetConfigurationEntry.ts index e5ecb295..79b596e7 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/singleLineTextWidgetConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/singleLineTextWidgetConfigurationEntry.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { PxeConfigurationEntryType, PxeSingleLineTextWidgetEntry, PxeValueType } from '../types/PxeConfiguration.types'; +import { PxeNodeType, PxeSingleLineTextWidgetEntry, PxeValueType } from '../types/PxeConfiguration.types'; import { identityTextFilter, naturalNumberTextFilter, TextFilter } from '../validation/textFilters'; export const singleLineTextWidgetConfigurationEntry = ({ @@ -30,7 +30,7 @@ export const singleLineTextWidgetConfigurationEntry = ({ name?: string; textFilter?: TextFilter; }): PxeSingleLineTextWidgetEntry => ({ - type: PxeConfigurationEntryType.SingleLineText, + type: PxeNodeType.SingleLineText, valueDescriptors: [{ path, type, isRequired, display: { name } }], textFilter, }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/createEditorFromConfiguration.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/createEditorFromConfiguration.tsx index 196d104d..46ee0618 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/createEditorFromConfiguration.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/createEditorFromConfiguration.tsx @@ -17,6 +17,7 @@ import React from 'react'; import { PxeConfiguration } from './types/PxeConfiguration.types'; import { PxeParametricEditor } from './PxeParametricEditor'; +import { PxeDiagnosticsReporter } from './types/PxeDiagnostics.types'; type ConfiguredEditorProps = { readonly yamlText: string; @@ -24,6 +25,13 @@ type ConfiguredEditorProps = { }; export const createEditorFromConfiguration = - (configuration: PxeConfiguration): React.FC => + (configuration: PxeConfiguration, diagnosticsReporter?: PxeDiagnosticsReporter): React.FC => ({ yamlText, onResourceChange }) => - ; + ( + + ); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx index 6bace869..c3318b3f 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx @@ -23,21 +23,23 @@ import { PxeSectionEntry } from '../types/PxeConfiguration.types'; import { generateSectionDescription } from '../utils/generateLabelsForWidgets'; import { PxeParametricEditorNodeProps } from '../PxeParametricEditorNode'; import { PxeParametricEditorNodeList } from '../PxeParametricEditorNodeList'; +import { useDiagnostics } from '../PxeDiagnosticsContext'; export const PxeSectionNode: React.FC = ({ - configurationEntry: configurationEntryUncasted, + configurationEntry, resourceChunk, onResourceChangeRequest, parentExpandedSectionState, children, }) => { - const configurationEntry = configurationEntryUncasted as PxeSectionEntry; - const { name, entries: childEntries } = configurationEntry; + useDiagnostics(configurationEntry); + const sectionEntry = configurationEntry as PxeSectionEntry; + const { name, entries: childEntries } = sectionEntry; const sectionIdRef = useRef(`section-${nanoid()}`); const [expandedSection, setExpandedSection] = useState(undefined); - const description = generateSectionDescription(configurationEntry, resourceChunk); + const description = generateSectionDescription(sectionEntry, resourceChunk); return ( ({ rowContainer: { @@ -33,6 +34,7 @@ export const PxeRowLayoutNode: React.FC = ({ onResourceChangeRequest, resourceChunk, }) => { + useDiagnostics(configurationEntry); const { entries } = configurationEntry as PxeRowLayoutEntry; const classes = useStyles(); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx index a88361d3..47364771 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx @@ -18,16 +18,17 @@ import { Button, makeStyles } from '@material-ui/core'; import AddIcon from '@material-ui/icons/Add'; import DeleteIcon from '@material-ui/icons/Delete'; import { get } from 'lodash'; -import React, { Fragment, useEffect, useState } from 'react'; +import React, { Fragment, useState } from 'react'; import { IconButton } from '../../../../../Controls'; import { useEditorStyles } from '../../../FirstClassEditors/styles'; import { PxeParametricEditorNode, PxeParametricEditorNodeProps } from '../../PxeParametricEditorNode'; -import { PxeConfigurationEntryType, PxeRosterWidgetEntry, PxeValueType } from '../../types/PxeConfiguration.types'; +import { PxeNodeType, PxeRosterWidgetEntry, PxeValueType } from '../../types/PxeConfiguration.types'; import { PxeResourceChangeRequest, PxeValue } from '../../types/PxeParametricEditor.types'; import { createResourceChunkAfterChangeRequest } from '../../utils/createResourceChunkAfterChangeRequest'; import { generateValueLabel } from '../../utils/generateLabelsForWidgets'; import { arrayWithItemRemoved, arrayWithItemReplaced } from '../../utils/general/immutableArrays'; import { defaultValueForType } from '../../utils/defaultValueForType'; +import { useDiagnostics } from '../../PxeDiagnosticsContext'; type RosterItemResourceChunk = { readonly $key: string; @@ -43,6 +44,7 @@ export const PxeRosterWidgetNode: React.FC = ({ onResourceChangeRequest, parentExpandedSectionState, }) => { + useDiagnostics(configurationEntry); const { valueDescriptors: [valueDescriptor], itemValueDescriptor: itemValueDescriptor, @@ -54,9 +56,11 @@ export const PxeRosterWidgetNode: React.FC = ({ itemChunksFromValue(get(resourceChunk, valueDescriptor.path)), ); - useEffect(() => { + const [previousResourceChunk, setPreviousResourceChunk] = useState(resourceChunk); + if (previousResourceChunk !== resourceChunk) { setItemChunks(itemChunksFromValue(get(resourceChunk, valueDescriptor.path))); - }, [resourceChunk, valueDescriptor]); + setPreviousResourceChunk(resourceChunk); + } const handleResourceChangeRequestForItem = (itemIndex: number, changeRequest: PxeResourceChangeRequest) => { const newItemChunk = createResourceChunkAfterChangeRequest( @@ -114,14 +118,14 @@ export const PxeRosterWidgetNode: React.FC = ({ onResourceChangeRequest={changeRequest => handleResourceChangeRequestForItem(itemIndex, changeRequest)} parentExpandedSectionState={parentExpandedSectionState} > - {itemEntries[0].type === PxeConfigurationEntryType.Section && ( + {itemEntries[0].type === PxeNodeType.Section && ( )}
- {itemEntries[0].type !== PxeConfigurationEntryType.Section && ( + {itemEntries[0].type !== PxeNodeType.Section && (
= onResourceChangeRequest, resourceChunk, }) => { + useDiagnostics(configurationEntry); const widgetEntry = configurationEntry as PxeSelectValueWidgetEntry; const [valueDescriptor] = widgetEntry.valueDescriptors; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx index 3a99cdbe..e6cec2f0 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx @@ -20,12 +20,14 @@ import React from 'react'; import { PxeSingleLineTextWidgetEntry } from '../../types/PxeConfiguration.types'; import { PxeParametricEditorNodeProps } from '../../PxeParametricEditorNode'; import { generateValueLabel } from '../../utils/generateLabelsForWidgets'; +import { useDiagnostics } from '../../PxeDiagnosticsContext'; export const PxeSingleLineTextWidgetNode: React.FC = ({ configurationEntry, onResourceChangeRequest, resourceChunk, }) => { + useDiagnostics(configurationEntry); const { textFilter, valueDescriptors: [valueDescriptor], diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeConfiguration.types.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeConfiguration.types.ts index 8ddbabcc..0f062362 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeConfiguration.types.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeConfiguration.types.ts @@ -26,7 +26,7 @@ export type PxeConfiguration = { export type PxeConfigurationEntry = PxeSectionEntry | PxeRowLayoutEntry | PxeWidgetEntry; -export enum PxeConfigurationEntryType { +export enum PxeNodeType { Section = 'Section', Roster = 'Roster', @@ -57,7 +57,7 @@ export type PxeValueDescriptor = { // Section export type PxeSectionEntry = { - readonly type: PxeConfigurationEntryType.Section; + readonly type: PxeNodeType.Section; readonly name: string; readonly entries: readonly PxeConfigurationEntry[]; }; @@ -67,7 +67,7 @@ export type PxeSectionEntry = { export type PxeLayoutEntry = PxeRowLayoutEntry; export interface PxeRowLayoutEntry { - readonly type: PxeConfigurationEntryType.RowLayout; + readonly type: PxeNodeType.RowLayout; readonly entries: readonly PxeConfigurationEntry[]; } @@ -76,23 +76,23 @@ export interface PxeRowLayoutEntry { export type PxeWidgetEntry = PxeRosterWidgetEntry | PxeSingleLineTextWidgetEntry | PxeSelectValueWidgetEntry; type PxeWidgetEntryBase = { - readonly type: PxeConfigurationEntryType; + readonly type: PxeNodeType; readonly valueDescriptors: readonly PxeValueDescriptor[]; }; export interface PxeRosterWidgetEntry extends PxeWidgetEntryBase { - readonly type: PxeConfigurationEntryType.Roster; + readonly type: PxeNodeType.Roster; readonly itemValueDescriptor: PxeValueDescriptor; readonly itemEntries: readonly PxeConfigurationEntry[]; } export interface PxeSingleLineTextWidgetEntry extends PxeWidgetEntryBase { - readonly type: PxeConfigurationEntryType.SingleLineText; + readonly type: PxeNodeType.SingleLineText; readonly textFilter: TextFilter; } export interface PxeSelectValueWidgetEntry extends PxeWidgetEntryBase { - readonly type: PxeConfigurationEntryType.SelectValue; + readonly type: PxeNodeType.SelectValue; readonly options: readonly PxeValueOption[]; } diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeDiagnostics.types.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeDiagnostics.types.ts new file mode 100644 index 00000000..a2ad02ac --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/types/PxeDiagnostics.types.ts @@ -0,0 +1,20 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { PxeConfigurationEntry } from './PxeConfiguration.types'; + +export type PxeDiagnosticsReporter = { + readonly reportRender: (entry: PxeConfigurationEntry) => void; +}; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts index 80d3c3ca..12c201f5 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts @@ -17,7 +17,7 @@ import * as changeCase from 'change-case'; import { get, size } from 'lodash'; import { - PxeConfigurationEntryType, + PxeNodeType, PxeSectionEntry, PxeValueDescriptor, PxeValueType, @@ -31,7 +31,7 @@ const FALLBACK_DEFAULT_VALUE_NAME = 'Value'; export const generateSectionDescription = (sectionEntry: PxeSectionEntry, resourceChunk: PxeResourceChunk): string => sectionEntry.entries - .filter(childEntry => childEntry.type !== PxeConfigurationEntryType.Section) + .filter(childEntry => childEntry.type !== PxeNodeType.Section) // FIXME 1) needs type predicates (widget / layout), 2) handle layouts .flatMap(childEntry => 'valueDescriptors' in childEntry ? generateValueDescriptionsForWidget(childEntry, resourceChunk) : [], From abebaa9cf97584b3ad541a085045015996f8f024 Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Thu, 21 Nov 2024 07:20:16 +0100 Subject: [PATCH 10/19] Basic rendering optimization for non-roster widgets done. --- .../PxeParametricEditor.tsx | 52 +++-- .../PxeParametricEditorNode.tsx | 7 +- .../PxeParametricEditorNodeList.tsx | 16 +- .../configuration/rosterConfigurationEntry.ts | 3 +- .../nodes/PxeSectionNode.tsx | 58 +++-- .../nodes/layout/PxeRowLayoutNode.tsx | 42 ++-- .../nodes/widget/PxeRosterWidgetNode.tsx | 213 +++++++++--------- .../nodes/widget/PxeSelectValueWidgetNode.tsx | 53 +++-- .../widget/PxeSingleLineTextWidgetNode.tsx | 53 +++-- .../utils/generateLabelsForWidgets.ts | 7 +- .../utils/nodePredicates.ts | 31 +++ .../utils/rendering/withCurrentValues.tsx | 35 +++ 12 files changed, 312 insertions(+), 258 deletions(-) create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/nodePredicates.ts create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/rendering/withCurrentValues.tsx diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditor.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditor.tsx index 8945458c..d35741ee 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditor.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditor.tsx @@ -15,16 +15,20 @@ */ import { pick } from 'lodash'; -import React, { useCallback, useState } from 'react'; -import { useSetStateAndCall } from '../../../../hooks/useSetStateAndCall'; +import React, { useCallback, useMemo, useRef, useState } from 'react'; import { useEditorStyles } from '../FirstClassEditors/styles'; import { PxeConfiguration } from './types/PxeConfiguration.types'; -import { PxeExpandedSectionState, PxeResourceChangeRequestHandler } from './types/PxeParametricEditor.types'; +import { + PxeExpandedSectionState, + PxeExpandedSectionStateTuple, + PxeResourceChangeRequestHandler, +} from './types/PxeParametricEditor.types'; import { PxeDiagnosticsReporter } from './types/PxeDiagnostics.types'; -import { parseYaml, stringifyYaml } from './utils/yamlConversion'; import { createResourceChunkAfterChangeRequest } from './utils/createResourceChunkAfterChangeRequest'; +import { parseYaml, stringifyYaml } from './utils/yamlConversion'; import { PxeParametricEditorNodeList } from './PxeParametricEditorNodeList'; import { PxeDiagnosticsContext } from './PxeDiagnosticsContext'; +import { PxeResourceContext } from './PxeResourceContext'; export type PxeParametricEditorProps = { readonly configuration: PxeConfiguration; @@ -39,21 +43,26 @@ export const PxeParametricEditor: React.FC = ({ onResourceChange, __diagnosticsReporter, }) => { - const { yamlObject: initialYamlObject } = parseYaml(yamlText); - const initialResourceState = pick(initialYamlObject, topLevelProperties); - - const [resource, setResource] = useState(initialResourceState); - const [expandedSection, setExpandedSection] = useState(undefined); + const initialYamlObject = useRef(parseYaml(yamlText).yamlObject); - const setResourceAndNotifyOnChange = useSetStateAndCall([resource, setResource], newState => { - const newYamlText = stringifyYaml({ ...initialYamlObject, ...newState }); - onResourceChange(newYamlText); - }); + const [resource, setResource] = useState(() => pick(initialYamlObject.current, topLevelProperties)); + const previousResource = useRef(resource); + if (previousResource.current !== resource) { + previousResource.current = resource; + onResourceChange(stringifyYaml({ ...initialYamlObject.current, ...resource })); + } const handleResourceChangeRequest: PxeResourceChangeRequestHandler = useCallback( (changeRequest): void => - setResourceAndNotifyOnChange(createResourceChunkAfterChangeRequest(resource, changeRequest)), - [resource, setResourceAndNotifyOnChange], + setResource(prevResource => createResourceChunkAfterChangeRequest(prevResource, changeRequest)), + [], + ); + + // TODO Move to context. + const [expandedSection, setExpandedSection] = useState(undefined); + const expandedSectionState = useMemo( + () => [expandedSection, setExpandedSection], + [expandedSection], ); const classes = useEditorStyles(); @@ -61,12 +70,13 @@ export const PxeParametricEditor: React.FC = ({ return (
- + + +
); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNode.tsx index 45ff5ae2..cb1b3118 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNode.tsx @@ -15,11 +15,7 @@ */ import React from 'react'; -import { - PxeExpandedSectionStateTuple, - PxeResourceChangeRequestHandler, - PxeResourceChunk, -} from './types/PxeParametricEditor.types'; +import { PxeExpandedSectionStateTuple, PxeResourceChangeRequestHandler } from './types/PxeParametricEditor.types'; import { PxeConfigurationEntry, PxeNodeType } from './types/PxeConfiguration.types'; import { PxeSectionNode } from './nodes/PxeSectionNode'; import { PxeRowLayoutNode } from './nodes/layout/PxeRowLayoutNode'; @@ -29,7 +25,6 @@ import { PxeSelectValueWidgetNode } from './nodes/widget/PxeSelectValueWidgetNod export type PxeParametricEditorNodeProps = { readonly configurationEntry: PxeConfigurationEntry; - readonly resourceChunk: PxeResourceChunk; readonly onResourceChangeRequest: PxeResourceChangeRequestHandler; readonly parentExpandedSectionState?: PxeExpandedSectionStateTuple; }; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNodeList.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNodeList.tsx index c4d73637..e74dccdd 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNodeList.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/PxeParametricEditorNodeList.tsx @@ -15,30 +15,25 @@ */ import React, { Fragment } from 'react'; -import { renderGroupedArray } from './utils/rendering/renderGroupedArray'; import { PxeParametricEditorNode } from './PxeParametricEditorNode'; +import { PxeConfigurationEntry } from './types/PxeConfiguration.types'; +import { PxeExpandedSectionStateTuple, PxeResourceChangeRequestHandler } from './types/PxeParametricEditor.types'; import { chunkByTrait } from './utils/general/chunkByTrait'; -import { PxeConfigurationEntry, PxeNodeType } from './types/PxeConfiguration.types'; -import { - PxeExpandedSectionStateTuple, - PxeResourceChangeRequestHandler, - PxeResourceChunk, -} from './types/PxeParametricEditor.types'; +import { isSectionNode } from './utils/nodePredicates'; +import { renderGroupedArray } from './utils/rendering/renderGroupedArray'; type PxeParametricEditorNodeListProps = { readonly entries: readonly PxeConfigurationEntry[]; - readonly resourceChunk: PxeResourceChunk; readonly onResourceChangeRequest: PxeResourceChangeRequestHandler; readonly parentExpandedSectionState?: PxeExpandedSectionStateTuple; }; export const PxeParametricEditorNodeList: React.FC = ({ entries, - resourceChunk, onResourceChangeRequest, parentExpandedSectionState, }) => { - const groupedEntries = chunkByTrait(entries, entry => entry.type === PxeNodeType.Section || null); + const groupedEntries = chunkByTrait(entries, entry => isSectionNode(entry) || null); return ( @@ -46,7 +41,6 @@ export const PxeParametricEditorNodeList: React.FC diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts index 03dd2aed..9adddf03 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/rosterConfigurationEntry.ts @@ -24,6 +24,7 @@ import { } from '../types/PxeConfiguration.types'; import { sectionConfigurationEntry } from './sectionConfigurationEntry'; import { findInConfigurationEntries } from '../utils/findInConfigurationEntries'; +import { isWidgetNode } from '../utils/nodePredicates'; // FIXME Think of something different (name? form?). type RosterItemDefinition = { @@ -89,7 +90,7 @@ const resolveItemValueDescriptor = ( ): PxeValueDescriptor => { const itemValueEntry = findInConfigurationEntries( itemEntries, - entry => 'valueDescriptors' in entry && entry.valueDescriptors[0]?.path === '$value', + entry => isWidgetNode(entry) && entry.valueDescriptors[0]?.path === '$value', ) as PxeWidgetEntry | null; if (explicitRosterItemDef && itemValueEntry) { diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx index c3318b3f..f4d04452 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/PxeSectionNode.tsx @@ -20,41 +20,37 @@ import React, { useRef, useState } from 'react'; import { EditorAccordion } from '../../FirstClassEditors/Controls'; import { PxeExpandedSectionState } from '../types/PxeParametricEditor.types'; import { PxeSectionEntry } from '../types/PxeConfiguration.types'; -import { generateSectionDescription } from '../utils/generateLabelsForWidgets'; import { PxeParametricEditorNodeProps } from '../PxeParametricEditorNode'; import { PxeParametricEditorNodeList } from '../PxeParametricEditorNodeList'; import { useDiagnostics } from '../PxeDiagnosticsContext'; -export const PxeSectionNode: React.FC = ({ - configurationEntry, - resourceChunk, - onResourceChangeRequest, - parentExpandedSectionState, - children, -}) => { - useDiagnostics(configurationEntry); - const sectionEntry = configurationEntry as PxeSectionEntry; - const { name, entries: childEntries } = sectionEntry; +export const PxeSectionNode: React.FC = React.memo( + ({ configurationEntry, onResourceChangeRequest, parentExpandedSectionState, children }) => { + useDiagnostics(configurationEntry); - const sectionIdRef = useRef(`section-${nanoid()}`); - const [expandedSection, setExpandedSection] = useState(undefined); + const sectionEntry = configurationEntry as PxeSectionEntry; + const { name, entries: childEntries } = sectionEntry; - const description = generateSectionDescription(sectionEntry, resourceChunk); + const sectionIdRef = useRef(`section-${nanoid()}`); + const [expandedSection, setExpandedSection] = useState(undefined); - return ( - - - {children} - - ); -}; + // FIXME Will cause rerenders, repair later.. + const description = 'TODO'; // generateSectionDescription(sectionEntry, resourceChunk); + + return ( + + + {children} + + ); + }, +); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/layout/PxeRowLayoutNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/layout/PxeRowLayoutNode.tsx index 629be8ee..c463f4d7 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/layout/PxeRowLayoutNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/layout/PxeRowLayoutNode.tsx @@ -28,27 +28,23 @@ const useStyles = makeStyles(() => ({ }, })); -export const PxeRowLayoutNode: React.FC = ({ - configurationEntry, - parentExpandedSectionState, - onResourceChangeRequest, - resourceChunk, -}) => { - useDiagnostics(configurationEntry); - const { entries } = configurationEntry as PxeRowLayoutEntry; +export const PxeRowLayoutNode: React.FC = React.memo( + ({ configurationEntry, parentExpandedSectionState, onResourceChangeRequest }) => { + useDiagnostics(configurationEntry); + const { entries } = configurationEntry as PxeRowLayoutEntry; - const classes = useStyles(); - return ( -
- {entries.map((entry, index) => ( - - ))} -
- ); -}; + const classes = useStyles(); + return ( +
+ {entries.map((entry, index) => ( + + ))} +
+ ); + }, +); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx index 47364771..403be5b2 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeRosterWidgetNode.tsx @@ -17,7 +17,6 @@ import { Button, makeStyles } from '@material-ui/core'; import AddIcon from '@material-ui/icons/Add'; import DeleteIcon from '@material-ui/icons/Delete'; -import { get } from 'lodash'; import React, { Fragment, useState } from 'react'; import { IconButton } from '../../../../../Controls'; import { useEditorStyles } from '../../../FirstClassEditors/styles'; @@ -29,6 +28,9 @@ import { generateValueLabel } from '../../utils/generateLabelsForWidgets'; import { arrayWithItemRemoved, arrayWithItemReplaced } from '../../utils/general/immutableArrays'; import { defaultValueForType } from '../../utils/defaultValueForType'; import { useDiagnostics } from '../../PxeDiagnosticsContext'; +import { PxeResourceContext } from '../../PxeResourceContext'; +import { withCurrentValues } from '../../utils/rendering/withCurrentValues'; +import { isSectionNode } from '../../utils/nodePredicates'; type RosterItemResourceChunk = { readonly $key: string; @@ -38,119 +40,114 @@ type RosterItemResourceChunk = { type RosterValueType = PxeValueType.Object | PxeValueType.Array; // TODO Consider refactoring this component after Prettier settings update. -export const PxeRosterWidgetNode: React.FC = ({ - configurationEntry, - resourceChunk, - onResourceChangeRequest, - parentExpandedSectionState, -}) => { - useDiagnostics(configurationEntry); - const { - valueDescriptors: [valueDescriptor], - itemValueDescriptor: itemValueDescriptor, - itemEntries, - } = configurationEntry as PxeRosterWidgetEntry; - const rosterValueType = valueDescriptor.type as RosterValueType; - - const [itemChunks, setItemChunks] = useState( - itemChunksFromValue(get(resourceChunk, valueDescriptor.path)), - ); - - const [previousResourceChunk, setPreviousResourceChunk] = useState(resourceChunk); - if (previousResourceChunk !== resourceChunk) { - setItemChunks(itemChunksFromValue(get(resourceChunk, valueDescriptor.path))); - setPreviousResourceChunk(resourceChunk); - } - - const handleResourceChangeRequestForItem = (itemIndex: number, changeRequest: PxeResourceChangeRequest) => { - const newItemChunk = createResourceChunkAfterChangeRequest( - itemChunks[itemIndex], - changeRequest, - ) as RosterItemResourceChunk; - - const newItemChunks = arrayWithItemReplaced(itemChunks, itemIndex, newItemChunk); - const newValue = valueFromItemChunks(newItemChunks, rosterValueType); - if (Object.values(newValue).length === itemChunks.length) { - onResourceChangeRequest({ valueDescriptor, newValue }); - } else { - setItemChunks(newItemChunks); +export const PxeRosterWidgetNode: React.FC = withCurrentValues( + ({ configurationEntry, onResourceChangeRequest, parentExpandedSectionState, currentValues: [currentValue] }) => { + useDiagnostics(configurationEntry); + + const { + valueDescriptors: [valueDescriptor], + itemValueDescriptor: itemValueDescriptor, + itemEntries, + } = configurationEntry as PxeRosterWidgetEntry; + const rosterValueType = valueDescriptor.type as RosterValueType; + + const [itemChunks, setItemChunks] = useState(itemChunksFromValue(currentValue)); + + const [previousResourceChunk, setPreviousResourceChunk] = useState(currentValue); + if (previousResourceChunk !== currentValue) { + setItemChunks(itemChunksFromValue(currentValue)); + setPreviousResourceChunk(currentValue); } - }; - const handleItemAddition = () => { - const newItemChunk: RosterItemResourceChunk = { - $key: rosterValueType === PxeValueType.Array ? String(itemChunks.length) : '', - $value: itemValueDescriptor.isRequired ? defaultValueForType(itemValueDescriptor.type) : null, + const handleResourceChangeRequestForItem = (itemIndex: number, changeRequest: PxeResourceChangeRequest) => { + const newItemChunk = createResourceChunkAfterChangeRequest( + itemChunks[itemIndex], + changeRequest, + ) as RosterItemResourceChunk; + + const newItemChunks = arrayWithItemReplaced(itemChunks, itemIndex, newItemChunk); + const newValue = valueFromItemChunks(newItemChunks, rosterValueType); + if (Object.values(newValue).length === itemChunks.length) { + onResourceChangeRequest({ valueDescriptor, newValue }); + } else { + setItemChunks(newItemChunks); + } }; - const newValue = valueFromItemChunks([...itemChunks, newItemChunk], rosterValueType); - onResourceChangeRequest({ valueDescriptor, newValue }); - }; - - const handleItemDeletion = (itemIndex: number) => { - const newValue = valueFromItemChunks(arrayWithItemRemoved(itemChunks, itemIndex), rosterValueType); - onResourceChangeRequest({ valueDescriptor, newValue }); - }; - - const isAddButtonEnabled = rosterValueType === PxeValueType.Array || itemChunks.every(({ $key }) => $key !== ''); - const editorClasses = useEditorStyles(); - const rosterClasses = useStyles(); - - if (itemEntries.length === 0) { - return ; - } else if (itemEntries.length > 1) { - // TODO With proper styling this should be actually possible. Reconsider handling this. - throw new Error('Roster Widget does not support multiple configuration entries per item.'); - } else { - // FIXME Refactor by extraction? - return ( - - {itemChunks.map((itemChunk, itemIndex) => ( -
-
- handleResourceChangeRequestForItem(itemIndex, changeRequest)} - parentExpandedSectionState={parentExpandedSectionState} - > - {itemEntries[0].type === PxeNodeType.Section && ( - + const handleItemAddition = () => { + const newItemChunk: RosterItemResourceChunk = { + $key: rosterValueType === PxeValueType.Array ? String(itemChunks.length) : '', + $value: itemValueDescriptor.isRequired ? defaultValueForType(itemValueDescriptor.type) : null, + }; + + const newValue = valueFromItemChunks([...itemChunks, newItemChunk], rosterValueType); + onResourceChangeRequest({ valueDescriptor, newValue }); + }; + + const handleItemDeletion = (itemIndex: number) => { + const newValue = valueFromItemChunks(arrayWithItemRemoved(itemChunks, itemIndex), rosterValueType); + onResourceChangeRequest({ valueDescriptor, newValue }); + }; + + const isAddButtonEnabled = rosterValueType === PxeValueType.Array || itemChunks.every(({ $key }) => $key !== ''); + const editorClasses = useEditorStyles(); + const rosterClasses = useStyles(); + + if (itemEntries.length === 0) { + return ; + } else if (itemEntries.length > 1) { + // TODO With proper styling this should be actually possible. Reconsider handling this. + throw new Error('Roster Widget does not support multiple configuration entries per item.'); + } else { + // FIXME Refactor by extraction? + return ( + + {itemChunks.map((itemChunk, itemIndex) => ( + +
+
+ + handleResourceChangeRequestForItem(itemIndex, changeRequest) + } + parentExpandedSectionState={parentExpandedSectionState} + > + {isSectionNode(itemEntries[0]) && ( + + )} + +
+ {itemEntries[0].type !== PxeNodeType.Section && ( +
+ handleItemDeletion(itemIndex)} + > + + +
)} - -
- {itemEntries[0].type !== PxeNodeType.Section && ( -
- handleItemDeletion(itemIndex)} - > - -
- )} -
- ))} - - - ); - } -}; + + ))} + + + ); + } + }, +); const itemChunksFromValue = (value: PxeValue): readonly RosterItemResourceChunk[] => Object.entries(value ?? {}).map(([itemKey, itemValue]) => ({ diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx index ae14e62e..dc31546f 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx @@ -14,41 +14,40 @@ * limitations under the License. */ -import { get } from 'lodash'; import React from 'react'; import { Select } from '../../../../../Controls'; import { PxeSelectValueWidgetEntry } from '../../types/PxeConfiguration.types'; +import { withCurrentValues } from '../../utils/rendering/withCurrentValues'; import { generateValueLabel } from '../../utils/generateLabelsForWidgets'; import { PxeParametricEditorNodeProps } from '../../PxeParametricEditorNode'; import { useDiagnostics } from '../../PxeDiagnosticsContext'; const DEFAULT_VALUE = '__DEFAULT_VALUE__'; -export const PxeSelectValueWidgetNode: React.FC = ({ - configurationEntry, - onResourceChangeRequest, - resourceChunk, -}) => { - useDiagnostics(configurationEntry); - const widgetEntry = configurationEntry as PxeSelectValueWidgetEntry; - const [valueDescriptor] = widgetEntry.valueDescriptors; +export const PxeSelectValueWidgetNode: React.FC = withCurrentValues( + ({ configurationEntry, onResourceChangeRequest, currentValues: [currentValue] }) => { + useDiagnostics(configurationEntry); - const selectItems = widgetEntry.options.map(({ value, label }) => ({ - value: value !== undefined ? String(value) : DEFAULT_VALUE, - label, - })); + const widgetEntry = configurationEntry as PxeSelectValueWidgetEntry; + const [valueDescriptor] = widgetEntry.valueDescriptors; - return ( - { + onResourceChangeRequest({ + valueDescriptor, + newValue: value !== DEFAULT_VALUE ? value : undefined, + }); + }} + /> + ); + }, +); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx index e6cec2f0..de70a668 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSingleLineTextWidgetNode.tsx @@ -15,37 +15,36 @@ */ import { TextField } from '@material-ui/core'; -import { get } from 'lodash'; import React from 'react'; import { PxeSingleLineTextWidgetEntry } from '../../types/PxeConfiguration.types'; import { PxeParametricEditorNodeProps } from '../../PxeParametricEditorNode'; +import { withCurrentValues } from '../../utils/rendering/withCurrentValues'; import { generateValueLabel } from '../../utils/generateLabelsForWidgets'; import { useDiagnostics } from '../../PxeDiagnosticsContext'; -export const PxeSingleLineTextWidgetNode: React.FC = ({ - configurationEntry, - onResourceChangeRequest, - resourceChunk, -}) => { - useDiagnostics(configurationEntry); - const { - textFilter, - valueDescriptors: [valueDescriptor], - } = configurationEntry as PxeSingleLineTextWidgetEntry; +export const PxeSingleLineTextWidgetNode: React.FC = withCurrentValues( + ({ configurationEntry, onResourceChangeRequest, currentValues: [currentValue] }) => { + useDiagnostics(configurationEntry); - return ( - { - onResourceChangeRequest({ - valueDescriptor, - newValue: textFilter(e.target.value), - }); - }} - fullWidth - /> - ); -}; + const { + textFilter, + valueDescriptors: [valueDescriptor], + } = configurationEntry as PxeSingleLineTextWidgetEntry; + + return ( + { + onResourceChangeRequest({ + valueDescriptor, + newValue: textFilter(e.target.value), + }); + }} + fullWidth + /> + ); + }, +); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts index 12c201f5..56eac33c 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/generateLabelsForWidgets.ts @@ -24,17 +24,18 @@ import { PxeWidgetEntry, } from '../types/PxeConfiguration.types'; import { PxeResourceChunk } from '../types/PxeParametricEditor.types'; -import { isEmptyPxeValue } from './isEmptyPxeValue'; import { upperCaseFirstLetter } from './general/stringCasing'; +import { isEmptyPxeValue } from './isEmptyPxeValue'; +import { isWidgetNode } from './nodePredicates'; const FALLBACK_DEFAULT_VALUE_NAME = 'Value'; export const generateSectionDescription = (sectionEntry: PxeSectionEntry, resourceChunk: PxeResourceChunk): string => sectionEntry.entries .filter(childEntry => childEntry.type !== PxeNodeType.Section) - // FIXME 1) needs type predicates (widget / layout), 2) handle layouts + // FIXME handle layouts .flatMap(childEntry => - 'valueDescriptors' in childEntry ? generateValueDescriptionsForWidget(childEntry, resourceChunk) : [], + isWidgetNode(childEntry) ? generateValueDescriptionsForWidget(childEntry, resourceChunk) : [], ) .join(', '); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/nodePredicates.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/nodePredicates.ts new file mode 100644 index 00000000..d55655ae --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/nodePredicates.ts @@ -0,0 +1,31 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + PxeConfigurationEntry, + PxeLayoutEntry, + PxeNodeType, + PxeSectionEntry, + PxeWidgetEntry, +} from '../types/PxeConfiguration.types'; + +export const isSectionNode = (entry: PxeConfigurationEntry): entry is PxeSectionEntry => + entry.type === PxeNodeType.Section; + +export const isLayoutNode = (entry: PxeConfigurationEntry): entry is PxeLayoutEntry => + [PxeNodeType.RowLayout].includes(entry.type); + +export const isWidgetNode = (entry: PxeConfigurationEntry): entry is PxeWidgetEntry => 'valueDescriptors' in entry; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/rendering/withCurrentValues.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/rendering/withCurrentValues.tsx new file mode 100644 index 00000000..222d3930 --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/utils/rendering/withCurrentValues.tsx @@ -0,0 +1,35 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { get, isEqual } from 'lodash'; +import React, { useContext } from 'react'; +import { PxeValue } from '../../types/PxeParametricEditor.types'; +import { PxeParametricEditorNodeProps } from '../../PxeParametricEditorNode'; +import { PxeResourceContext } from '../../PxeResourceContext'; +import { PxeWidgetEntry } from '../../types/PxeConfiguration.types'; + +export const withCurrentValues = ( + Component: React.FC, +) => { + const MemoizedComponent = React.memo(Component, isEqual); + return (props: PxeParametricEditorNodeProps) => { + const { valueDescriptors } = props.configurationEntry as PxeWidgetEntry; + const resource = useContext(PxeResourceContext); + + const currentValues = valueDescriptors.map(({ path }) => get(resource, path)); + return ; + }; +}; From 68f0269d32ba253131097b5ba3fff9b092c223e7 Mon Sep 17 00:00:00 2001 From: Kamil Madejek Date: Fri, 22 Nov 2024 17:43:16 +0100 Subject: [PATCH 11/19] Fixes, refactoring and more tests. --- .../cad/src/components/Controls/Select.tsx | 4 +- .../ParametricEditorPerformance.test.tsx | 38 ++++- ...ParametricEditorSelectValueWidget.test.tsx | 146 ++++++++++++++++++ .../__tests__/testUtils/findEditorElement.ts | 38 +++-- .../selectValueWidgetConfigurationEntry.ts | 16 +- .../nodes/widget/PxeSelectValueWidgetNode.tsx | 3 +- .../types/PxeConfiguration.types.ts | 3 +- .../createResourceChunkAfterChangeRequest.ts | 7 +- 8 files changed, 218 insertions(+), 37 deletions(-) create mode 100644 plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorSelectValueWidget.test.tsx diff --git a/plugins/cad/src/components/Controls/Select.tsx b/plugins/cad/src/components/Controls/Select.tsx index eebd0fac..4a6d69c7 100644 --- a/plugins/cad/src/components/Controls/Select.tsx +++ b/plugins/cad/src/components/Controls/Select.tsx @@ -27,13 +27,13 @@ type SelectProps = { helperText?: string; }; -export const Select = ({ label, selected, items, onChange, className, helperText }: SelectProps) => { +export const Select = ({ label, selected, items, onChange, className, helperText, ...otherProps }: SelectProps) => { const handleChange = (event: ChangeEvent<{ name?: string | undefined; value: unknown }>): void => { onChange(event.target.value as string); }; return ( - + {label} {items.map(item => ( diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorPerformance.test.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorPerformance.test.tsx index db6b9d47..11c0a9f0 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorPerformance.test.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorPerformance.test.tsx @@ -19,7 +19,7 @@ import { RenderResult } from '@testing-library/react'; import { userEvent } from '@testing-library/user-event'; import { noop } from 'lodash'; import React from 'react'; -import { findTextFieldInput } from './testUtils/findEditorElement'; +import { findSelectInput, findSelectOption, findTextFieldInput } from './testUtils/findEditorElement'; import { PxeConfiguration } from '../types/PxeConfiguration.types'; import { PxeConfigurationFactory } from '../configuration'; import { PxeParametricEditor } from '../PxeParametricEditor'; @@ -33,7 +33,13 @@ const CONFIGURATION: PxeConfiguration = { { name: 'Section' }, rowLayout( singleLineText({ path: 'spec.singleLineText' }), - selectValue({ path: 'spec.selectValue', options: [{ value: 'foo', label: 'foo' }] }), + selectValue({ + path: 'spec.selectValue', + options: [ + { value: 'foo', label: 'Foo' }, + { value: 'bar', label: 'Bar' }, + ], + }), ), arrayTypeRoster({ name: 'Array', path: 'spec.arrayTypeRoster' }, singleLineText({ path: '$value' })), objectTypeRoster( @@ -55,10 +61,9 @@ spec: `; describe('PxeParametricEditor performance', () => { - const INITIAL_NON_ITEM_NODE_COUNT = 6; - const INITIAL_ROSTER_SECTION_COUNT = 2; + const INITIAL_NON_ITEM_NODE_COUNT = 6 + 2; // 2 implicit roster sections const INITIAL_ITEM_NODE_COUNT = 4; - const INITIAL_NODE_COUNT = INITIAL_NON_ITEM_NODE_COUNT + INITIAL_ITEM_NODE_COUNT + INITIAL_ROSTER_SECTION_COUNT; + const INITIAL_NODE_COUNT = INITIAL_NON_ITEM_NODE_COUNT + INITIAL_ITEM_NODE_COUNT; let editor: RenderResult; let renderReporter: jest.Mock; @@ -77,7 +82,7 @@ describe('PxeParametricEditor performance', () => { }); it('should render with minimal number of component renders', () => { - expect(renderReporter.mock.calls.length).toBe(INITIAL_NODE_COUNT); + expect(renderReporter).toHaveBeenCalledTimes(INITIAL_NODE_COUNT); }); it('should rerender text widget when single character is inputted', async () => { @@ -85,7 +90,7 @@ describe('PxeParametricEditor performance', () => { await userEvent.type(textInput, 'a'); - expect(renderReporter.mock.calls.length).toBe(INITIAL_NODE_COUNT + 1); + expect(renderReporter).toHaveBeenCalledTimes(INITIAL_NODE_COUNT + 1); }); it('should rerender text widget multiple times when multiple characters are inputted', async () => { @@ -93,6 +98,23 @@ describe('PxeParametricEditor performance', () => { await userEvent.type(textInput, 'aaa'); - expect(renderReporter.mock.calls.length).toBe(INITIAL_NODE_COUNT + 3); + expect(renderReporter).toHaveBeenCalledTimes(INITIAL_NODE_COUNT + 3); + }); + + it('should rerender text widget when input is cleared', async () => { + const textInput = findTextFieldInput(editor, `spec.singleLineText`); + + await userEvent.clear(textInput); + + expect(renderReporter).toHaveBeenCalledTimes(INITIAL_NODE_COUNT + 1); + }); + + it('should rerender select widget when value is changed', async () => { + const selectInput = findSelectInput(editor, `spec.selectValue`); + + await userEvent.click(selectInput); + await userEvent.click(findSelectOption(editor, 2)); + + expect(renderReporter).toHaveBeenCalledTimes(INITIAL_NODE_COUNT + 1); }); }); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorSelectValueWidget.test.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorSelectValueWidget.test.tsx new file mode 100644 index 00000000..938ce467 --- /dev/null +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/ParametricEditorSelectValueWidget.test.tsx @@ -0,0 +1,146 @@ +/** + * Copyright 2024 The Nephio Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { renderWithEffects } from '@backstage/test-utils'; +import { RenderResult } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; +import React from 'react'; +import { PxeParametricEditor } from '../PxeParametricEditor'; +import { PxeConfiguration, PxeValueOption, PxeValueType } from '../types/PxeConfiguration.types'; +import { PxeConfigurationFactory } from '../configuration'; +import { findSelectInput, findSelectLabel, findSelectOption } from './testUtils/findEditorElement'; +import { getLastResourceState } from './testUtils/getLastResourceState'; + +const { selectValue } = PxeConfigurationFactory; + +const OPTIONS_S: readonly PxeValueOption[] = [ + { value: 'foo', label: 'Foo' }, + { value: 'bar', label: 'Bar' }, +]; +const OPTIONS_S_OPT: readonly PxeValueOption[] = [...OPTIONS_S, { value: undefined, label: 'Empty' }]; + +const OPTIONS_N: readonly PxeValueOption[] = [ + { value: 1, label: 'Number 1' }, + { value: 2, label: 'Number 2' }, +]; +const OPTIONS_N_OPT: readonly PxeValueOption[] = [...OPTIONS_N, { value: undefined, label: 'Empty' }]; + +const CONFIGURATION: PxeConfiguration = { + topLevelProperties: ['spec'], + entries: [ + selectValue({ path: 'spec.selectOptional', options: OPTIONS_S_OPT }), + selectValue({ path: 'spec.selectRequired', options: OPTIONS_S, isRequired: true }), + selectValue({ path: 'spec.selectNumberOptional', options: OPTIONS_N_OPT, type: PxeValueType.Number }), + selectValue({ path: 'spec.selectNumberRequired', options: OPTIONS_N, type: PxeValueType.Number, isRequired: true }), + selectValue({ path: 'spec.selectCustomName', options: OPTIONS_S, name: 'fizz buzz' }), + ], +}; + +const YAML = ` +spec: + selectOptional: "foo" + selectRequired: "foo" + selectNumberOptional: 1 + selectNumberRequired: 1 + selectCustomName: "foo" +`; + +describe('ParametricEditorSelectValueWidget', () => { + let editor: RenderResult; + let resourceChangeHandler: jest.Mock; + + beforeEach(async () => { + resourceChangeHandler = jest.fn(); + + editor = await renderWithEffects( + , + ); + }); + + describe('(type: string)', () => { + ['selectOptional', 'selectRequired'].forEach(property => { + it('should have current value from resource', () => { + const selectInput = findSelectInput(editor, `spec.${property}`); + + expect(selectInput.textContent).toBe('Foo'); + }); + + it(`should change value on new value selection (${property})`, async () => { + const selectInput = findSelectInput(editor, `spec.${property}`); + + await userEvent.click(selectInput); + await userEvent.click(findSelectOption(editor, 2)); + + const resource = getLastResourceState(resourceChangeHandler); + expect(resource.spec[property]).toBe('bar'); + expect(selectInput.textContent).toBe('Bar'); + }); + }); + + it('should change value on empty value selection', async () => { + const selectInput = findSelectInput(editor, `spec.selectOptional`); + + await userEvent.click(selectInput); + await userEvent.click(findSelectOption(editor, 3)); + + const resource = getLastResourceState(resourceChangeHandler); + expect(resource.spec.selectOptional).toBeUndefined(); + expect(selectInput.textContent).toBe('Empty'); + }); + }); + + describe('(type: number)', () => { + ['selectNumberOptional', 'selectNumberRequired'].forEach(property => { + it('should have current value from resource', () => { + const selectInput = findSelectInput(editor, `spec.${property}`); + + expect(selectInput.textContent).toBe('Number 1'); + }); + + it(`should change value on new value selection (${property})`, async () => { + const selectInput = findSelectInput(editor, `spec.${property}`); + + await userEvent.click(selectInput); + await userEvent.click(findSelectOption(editor, 2)); + + const resource = getLastResourceState(resourceChangeHandler); + expect(resource.spec[property]).toBe(2); + expect(selectInput.textContent).toBe('Number 2'); + }); + }); + + it('should change value on empty value selection', async () => { + const selectInput = findSelectInput(editor, `spec.selectNumberOptional`); + + await userEvent.click(selectInput); + await userEvent.click(findSelectOption(editor, 3)); + + const resource = getLastResourceState(resourceChangeHandler); + expect(resource.spec.selectNumberOptional).toBeUndefined(); + expect(selectInput.textContent).toBe('Empty'); + }); + }); + + it('should have sentence-cased auto-generated names', () => { + expect(findSelectLabel(editor, `spec.selectOptional`).textContent).toBe('Select optional'); + expect(findSelectLabel(editor, `spec.selectNumberRequired`).textContent).toBe('Select number required'); + expect(findSelectLabel(editor, `spec.selectRequired`).textContent).toBe('Select required'); + }); + + it('should have custom name if provided', () => { + expect(findSelectLabel(editor, `spec.selectCustomName`).textContent).toBe('Fizz buzz'); + }); +}); diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/findEditorElement.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/findEditorElement.ts index 21964e13..d2c15bfa 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/findEditorElement.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/__tests__/testUtils/findEditorElement.ts @@ -17,26 +17,32 @@ /* eslint @backstage/no-undeclared-imports: off */ import { RenderResult } from '@testing-library/react'; -export const findTextFieldInput = (searchRoot: RenderResult | HTMLElement, path: string): HTMLInputElement => { - const searchRootElement = 'baseElement' in searchRoot ? searchRoot.baseElement : searchRoot; - const inputElement = searchRootElement?.querySelector(`[data-testid="TextField_${path}"]`)?.querySelector('input'); +export const findTextFieldInput = (searchRoot: RenderResult | HTMLElement, path: string): HTMLInputElement => + findElement(searchRoot, `[data-testid="TextField_${path}"] input`, `Text field ${path}`); - if (inputElement) return inputElement; - else throw new Error(`Text field ${path} not found.`); -}; +export const findSelectInput = (searchRoot: RenderResult | HTMLElement, path: string): HTMLDivElement => + findElement(searchRoot, `[data-testid="Select_${path}"] [role="button"]`, `Select ${path}`); -export const findRosterItem = (searchRoot: RenderResult | HTMLElement, path: string, index: number): HTMLElement => { - const searchRootElement = 'baseElement' in searchRoot ? searchRoot.baseElement : searchRoot; - const rosterItemElement = searchRootElement?.querySelector(`[data-testid="RosterItem_${path}_${index}"]`); +export const findSelectLabel = (searchRoot: RenderResult | HTMLElement, path: string): HTMLDivElement => + findElement(searchRoot, `[data-testid="Select_${path}"] label`, `Select label ${path}`); - if (rosterItemElement) return rosterItemElement as HTMLElement; - else throw new Error(`Roster item ${path} [${index}] not found.`); -}; +export const findSelectOption = (searchRoot: RenderResult | HTMLElement, optionNumber: number): HTMLDivElement => + findElement(searchRoot, `[role="listbox"] > li:nth-child(${optionNumber})`, `Select option ${optionNumber}`); + +export const findRosterItem = (searchRoot: RenderResult | HTMLElement, path: string, index: number): HTMLElement => + findElement(searchRoot, `[data-testid="RosterItem_${path}_${index}"]`, `Roster item ${path} [${index}]`); + +export const findRosterAddButton = (searchRoot: RenderResult | HTMLElement, path: string): HTMLButtonElement => + findElement(searchRoot, `[data-testid="RosterAddButton_${path}"]`, `Roster add button ${path}`); -export const findRosterAddButton = (searchRoot: RenderResult | HTMLElement, path: string): HTMLButtonElement => { +const findElement = ( + searchRoot: RenderResult | HTMLElement, + selector: string, + description: string, +): T => { const searchRootElement = 'baseElement' in searchRoot ? searchRoot.baseElement : searchRoot; - const rosterAddButtonElement = searchRootElement?.querySelector(`[data-testid="RosterAddButton_${path}"]`); + const inputElement = searchRootElement?.querySelector(selector); - if (rosterAddButtonElement) return rosterAddButtonElement as HTMLButtonElement; - else throw new Error(`Roster add button ${path} not found.`); + if (inputElement) return inputElement as T; + else throw new Error(`${description} not found.`); }; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts index 48ac517b..ecb24312 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/configuration/selectValueWidgetConfigurationEntry.ts @@ -28,8 +28,14 @@ export const selectValueWidgetConfigurationEntry = ({ isRequired?: boolean; name?: string; options: readonly PxeValueOption[]; -}): PxeSelectValueWidgetEntry => ({ - type: PxeNodeType.SelectValue, - valueDescriptors: [{ path, type, isRequired, display: { name } }], - options, -}); +}): PxeSelectValueWidgetEntry => { + if (![PxeValueType.String, PxeValueType.Number].includes(type)) { + throw new Error('Select value widget only supports String and Number types.'); + } + + return { + type: PxeNodeType.SelectValue, + valueDescriptors: [{ path, type, isRequired, display: { name } }], + options, + }; +}; diff --git a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx index dc31546f..068c40f8 100644 --- a/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx +++ b/plugins/cad/src/components/ResourceEditorDialog/components/PxeParametricEditor/nodes/widget/PxeSelectValueWidgetNode.tsx @@ -32,12 +32,13 @@ export const PxeSelectValueWidgetNode: React.FC = const [valueDescriptor] = widgetEntry.valueDescriptors; const selectItems = widgetEntry.options.map(({ value, label }) => ({ - value: value !== undefined ? String(value) : DEFAULT_VALUE, + value: value !== undefined ? value : DEFAULT_VALUE, label, })); return (