From 49016f8b6836774ce265d29d7d57b8b3ffd1021b Mon Sep 17 00:00:00 2001 From: Christian Karidas <105549337+chriskari@users.noreply.github.com> Date: Mon, 30 Sep 2024 13:05:22 +0200 Subject: [PATCH] fix: section editor performance (#3361) * fix: reduce component hierarchy for form components & small fixes * fix: updated ui-schema package and disabled form view for form & details * fix: flaky tests * fix: translation * test: adjust tests to translation --- package-lock.json | 14 +- package.json | 2 +- public/i18n/en.yaml | 3 +- .../BusolaExtensionDetails.js | 7 +- .../views/ClusterOverview/ClusterDetails.js | 6 +- .../views/ClusterOverview/ClusterNodes.js | 2 +- .../EditCluster/AuthenticationDropdown.js | 1 - .../Extensibility/components-form/Jsonata.js | 26 ++-- .../components-form/MonacoRenderer.js | 1 - .../components-form/SimpleList.js | 129 ++++++++---------- src/resources/RoleBindings/RoleForm.js | 2 - src/resources/Secrets/SecretCreate.js | 28 ++-- .../ResourceForm/components/FormField.js | 48 +++---- .../ResourceForm/components/FormField.scss | 8 -- src/shared/ResourceForm/fields/MultiInput.js | 111 ++++++++------- src/shared/ResourceForm/inputs/Checkboxes.js | 20 ++- .../ResourceForm/inputs/ComboboxInput.js | 42 +++--- src/shared/ResourceForm/inputs/Dropdown.js | 14 +- src/shared/ResourceForm/inputs/Number.js | 14 +- src/shared/ResourceForm/inputs/Switch.js | 4 +- src/shared/ResourceForm/inputs/Text.js | 15 +- src/shared/components/K8sResourceSelect.js | 42 +++--- .../ResourceDetails/ResourceDetails.scss | 2 +- .../ResourceRef/ExternalResourceRef.js | 54 ++++---- .../namespace/test-dns-providers.spec.js | 2 +- .../kyma/tests/namespace/test-issuers.spec.js | 2 +- 26 files changed, 281 insertions(+), 318 deletions(-) diff --git a/package-lock.json b/package-lock.json index 982161767a..3e4b03145d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,7 +19,7 @@ "@stoplight/json-ref-resolver": "^3.1.3", "@types/jsonpath": "^0.2.0", "@types/pluralize": "^0.0.29", - "@ui-schema/ui-schema": "^0.4.0-beta.1", + "@ui-schema/ui-schema": "^0.4.5", "@ui5/webcomponents": "^1.24.0", "@ui5/webcomponents-base": "^1.24.0", "@ui5/webcomponents-fiori": "^1.24.0", @@ -5602,9 +5602,9 @@ } }, "node_modules/@ui-schema/ui-schema": { - "version": "0.4.4", - "resolved": "https://registry.npmjs.org/@ui-schema/ui-schema/-/ui-schema-0.4.4.tgz", - "integrity": "sha512-/qSPiBUqB04IO7nJZPHBmimJ4AtIkvWhcVwtoz/bUCf4LPYTNu7pGf+WCt4W1WWxPw/6oFQfQnm3RiyBa+lMxw==", + "version": "0.4.5", + "resolved": "https://registry.npmjs.org/@ui-schema/ui-schema/-/ui-schema-0.4.5.tgz", + "integrity": "sha512-PNEJfK4f9AR8X4JTjjNzJmz2X6bOAPnBI+pwB+qNKOAPlDBBfZZdiJ3GMNfG73Y1xcsfvc1HGjChvkNaXaCZHA==", "peerDependencies": { "@types/react": "^16.8 || ^17.0 || ^18.0", "immutable": "^4.0.0", @@ -31989,9 +31989,9 @@ } }, "@ui-schema/ui-schema": { - "version": "0.4.4", - "resolved": "https://registry.npmjs.org/@ui-schema/ui-schema/-/ui-schema-0.4.4.tgz", - "integrity": "sha512-/qSPiBUqB04IO7nJZPHBmimJ4AtIkvWhcVwtoz/bUCf4LPYTNu7pGf+WCt4W1WWxPw/6oFQfQnm3RiyBa+lMxw==", + "version": "0.4.5", + "resolved": "https://registry.npmjs.org/@ui-schema/ui-schema/-/ui-schema-0.4.5.tgz", + "integrity": "sha512-PNEJfK4f9AR8X4JTjjNzJmz2X6bOAPnBI+pwB+qNKOAPlDBBfZZdiJ3GMNfG73Y1xcsfvc1HGjChvkNaXaCZHA==", "requires": {} }, "@ui5/webcomponents": { diff --git a/package.json b/package.json index a3a8a9c143..6da13d19aa 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,7 @@ "@stoplight/json-ref-resolver": "^3.1.3", "@types/jsonpath": "^0.2.0", "@types/pluralize": "^0.0.29", - "@ui-schema/ui-schema": "^0.4.0-beta.1", + "@ui-schema/ui-schema": "^0.4.5", "@ui5/webcomponents": "^1.24.0", "@ui5/webcomponents-base": "^1.24.0", "@ui5/webcomponents-fiori": "^1.24.0", diff --git a/public/i18n/en.yaml b/public/i18n/en.yaml index 9a5aa51aff..543d5ccc8e 100644 --- a/public/i18n/en.yaml +++ b/public/i18n/en.yaml @@ -340,8 +340,9 @@ common: type-to-select: Start typing to select {{value}} from the list validation-error: Validation error placeholders: + enter-jsonata: Enter JSONata expression secret-ref-name: Select name - secret-ref-namespace: Select Namespace + secret-ref-namespace: Select namespace product-title: Kyma protected-resource-description: Resource protected. Go to Preferences to turn on the modification of protected resources. protected-resource: Protected resource diff --git a/src/components/BusolaExtensions/BusolaExtensionDetails.js b/src/components/BusolaExtensions/BusolaExtensionDetails.js index 320772357a..4a84385a6c 100644 --- a/src/components/BusolaExtensions/BusolaExtensionDetails.js +++ b/src/components/BusolaExtensions/BusolaExtensionDetails.js @@ -96,7 +96,12 @@ export function BusolaExtensionDetails({ name, namespace }) { { const { t } = useTranslation(); @@ -73,7 +74,10 @@ export default function ClusterDetails({ currentCluster }) { } /> {!error && !loadingModules && modules && ( -
+
{entry.metadata?.name} , diff --git a/src/components/Clusters/views/EditCluster/AuthenticationDropdown.js b/src/components/Clusters/views/EditCluster/AuthenticationDropdown.js index 1f0fb9ae6c..db6ee430fd 100644 --- a/src/components/Clusters/views/EditCluster/AuthenticationDropdown.js +++ b/src/components/Clusters/views/EditCluster/AuthenticationDropdown.js @@ -10,7 +10,6 @@ export function AuthenticationTypeDropdown({ type, setType }) { ]; return ( setType(selected.key)} diff --git a/src/components/Extensibility/components-form/Jsonata.js b/src/components/Extensibility/components-form/Jsonata.js index ef43276149..c654b60687 100644 --- a/src/components/Extensibility/components-form/Jsonata.js +++ b/src/components/Extensibility/components-form/Jsonata.js @@ -4,8 +4,8 @@ import { useGetTranslation, getPropsFromSchema, } from 'components/Extensibility/helpers'; -import { Tooltip } from 'shared/components/Tooltip/Tooltip'; import { Icon, Input } from '@ui5/webcomponents-react'; +import { t } from 'i18next'; export function JsonataInput({ value, @@ -27,17 +27,13 @@ export function JsonataInput({ if (!props.readOnly) delete props.readOnly; return ( -
- - setValue && setValue(e.target.value))} - icon={} - /> - -
+ setValue && setValue(e.target.value))} + icon={} + /> ); } @@ -72,7 +68,11 @@ export function Jsonata({ label={tFromStoreKeys(storeKeys, schema)} compact={compact} data-testid={storeKeys.join('.') || tFromStoreKeys(storeKeys, schema)} - placeholder={tExt(schemaPlaceholder) || tExt(placeholder)} + placeholder={ + tExt(schemaPlaceholder) || + tExt(placeholder) || + t('common.placeholders.enter-jsonata') + } input={JsonataInput} {...getPropsFromSchema(schema, required, tExt)} /> diff --git a/src/components/Extensibility/components-form/MonacoRenderer.js b/src/components/Extensibility/components-form/MonacoRenderer.js index 45510ad50b..694ab2f55a 100644 --- a/src/components/Extensibility/components-form/MonacoRenderer.js +++ b/src/components/Extensibility/components-form/MonacoRenderer.js @@ -100,7 +100,6 @@ export function MonacoRenderer({
-
-
-
    - {isObject && ( -
  • - -
    - titleRenderer), - custom: { - ...mapValues(widgets.custom, () => titleRenderer), - Null: () => '', - }, - }} - parentSchema={schema} - storeKeys={storeKeys.push(0)} - level={level + 1} - nestingLevel={nestingLevel + 1} - schemaKeys={schemaKeys?.push('items')} - /> -
    -     -
    • - )} - {Array(listSize + 1) - .fill(null) - .map((_val, index) => { - const ownKeys = storeKeys.push(index); - - return ( - <> -
  • - -
    - -
    - {!isLast(index) && ( -
    • - {isLast(index) && inputInfo && ( - +
      + {isObject && ( +
    • + titleRenderer), + custom: { + ...mapValues(widgets.custom, () => titleRenderer), + Null: () => '', + }, + }} + parentSchema={schema} + storeKeys={storeKeys.push(0)} + level={level + 1} + nestingLevel={nestingLevel + 1} + schemaKeys={schemaKeys?.push('items')} + /> +
      • + )} + {Array(listSize + 1) + .fill(null) + .map((_val, index) => { + const ownKeys = storeKeys.push(index); + return ( +
    • + +
      + +
      + {!isLast(index) && ( +
    -
+ + {isLast(index) && inputInfo && ( + + )} + + ); + })} + ); } diff --git a/src/resources/RoleBindings/RoleForm.js b/src/resources/RoleBindings/RoleForm.js index 0aa5c0527a..654835ddc4 100644 --- a/src/resources/RoleBindings/RoleForm.js +++ b/src/resources/RoleBindings/RoleForm.js @@ -26,7 +26,6 @@ export const RoleForm = ({ propertyPath="$.roleRef.kind" input={props => ( ({ key: v, text: v }))} {...props} @@ -62,7 +61,6 @@ export const RoleForm = ({ propertyPath="$.roleRef.name" input={props => ( ( -
- o.key === value)?.text ?? value} - disabled={!!initialUnchangedResource || !options?.length} - onChange={event => onChangeInput(event, setValue)} - onInput={event => onChangeInput(event, setValue)} - > - {options.map(option => ( - - ))} - -
+ o.key === value)?.text ?? value} + disabled={!!initialUnchangedResource || !options?.length} + onChange={event => onChangeInput(event, setValue)} + onInput={event => onChangeInput(event, setValue)} + > + {options.map(option => ( + + ))} + )} /> {!isListItem && } -
- {tooltipContent && ( - - )} -
+ {tooltipContent && ( + + )} - - {messageStrip - ? messageStrip - : input({ - updatesOnInput, - required, - disabled, - className: 'full-width', - ...inputProps, - })} - {inputInfo && ( - - )} - + {messageStrip + ? messageStrip + : input({ + updatesOnInput, + required, + disabled, + className: 'full-width', + ...inputProps, + })} + {inputInfo && ( + + )} ); diff --git a/src/shared/ResourceForm/components/FormField.scss b/src/shared/ResourceForm/components/FormField.scss index 8aecdfce58..3217f17666 100644 --- a/src/shared/ResourceForm/components/FormField.scss +++ b/src/shared/ResourceForm/components/FormField.scss @@ -1,12 +1,4 @@ .resource-form { - .tooltip-column { - text-align: center; - - [class*='sap-icon'] { - color: var(--sapButton_TextColor); - } - } - .form-field__label { padding: 12px 0 !important; } diff --git a/src/shared/ResourceForm/fields/MultiInput.js b/src/shared/ResourceForm/fields/MultiInput.js index ebbc11a630..21018e0ddc 100644 --- a/src/shared/ResourceForm/fields/MultiInput.js +++ b/src/shared/ResourceForm/fields/MultiInput.js @@ -159,63 +159,60 @@ export function MultiInput({ tooltipContent={sectionTooltipContent || tooltipContent} {...props} > -
-
    - {internalValue.map((entry, index) => { - const fieldWidth = - isLast(index) && newItemAction - ? `bsl-col-md--${12 - newItemActionWidth}` - : 'bsl-col-md--11'; - - return ( -
  • - - {noEdit && !isLast(index) && ( - {entry} - )} - - {(!noEdit || isLast(index)) && ( -
    - - {inputs.map( - (input, inputIndex) => - inputComponents[index][inputIndex], - )} - -
    - )} - - {!isLast(index) && ( -
    -
    - )} - - {isLast(index) && newItemAction && ( -
    - {newItemAction} -
    - )} -     -
    • - ); - })} - {inputInfo && ( - - )} -
-
+
    + {internalValue.map((entry, index) => { + const fieldWidth = + isLast(index) && newItemAction + ? `bsl-col-md--${12 - newItemActionWidth}` + : 'bsl-col-md--11'; + + return ( +
  • + + {noEdit && !isLast(index) && ( + {entry} + )} + + {(!noEdit || isLast(index)) && ( + + {inputs.map( + (input, inputIndex) => inputComponents[index][inputIndex], + )} + + )} + + {!isLast(index) && ( +
    • + ); + })} + {inputInfo && ( + + )} +
); } diff --git a/src/shared/ResourceForm/inputs/Checkboxes.js b/src/shared/ResourceForm/inputs/Checkboxes.js index 07cb4d7a0f..404e1a3484 100644 --- a/src/shared/ResourceForm/inputs/Checkboxes.js +++ b/src/shared/ResourceForm/inputs/Checkboxes.js @@ -27,17 +27,15 @@ export function Checkboxes({ onChange={e => updateValue(key, e.target.checked)} text={text} /> -
- {description && ( - - - - )} -
+ {description && ( + + + + )} ))} diff --git a/src/shared/ResourceForm/inputs/ComboboxInput.js b/src/shared/ResourceForm/inputs/ComboboxInput.js index 5a4aa19999..3cbfe9a6b7 100644 --- a/src/shared/ResourceForm/inputs/ComboboxInput.js +++ b/src/shared/ResourceForm/inputs/ComboboxInput.js @@ -1,4 +1,3 @@ -import classnames from 'classnames'; import { ComboBox, ComboBoxItem } from '@ui5/webcomponents-react'; export function ComboboxInput({ @@ -28,26 +27,25 @@ export function ComboboxInput({ }; return ( -
- {}} - value={ - options.find(o => o.key === value || o.key === selectedKey)?.text ?? - value - } - placeholder={placeholder} - {...props} - > - {options.map((option, index) => ( - - ))} - -
+ {}} + value={ + options.find(o => o.key === value || o.key === selectedKey)?.text ?? + value + } + placeholder={placeholder} + {...props} + > + {options.map((option, index) => ( + + ))} + ); } diff --git a/src/shared/ResourceForm/inputs/Dropdown.js b/src/shared/ResourceForm/inputs/Dropdown.js index 7501b2b1cb..4c14fbfd83 100644 --- a/src/shared/ResourceForm/inputs/Dropdown.js +++ b/src/shared/ResourceForm/inputs/Dropdown.js @@ -28,13 +28,11 @@ export function Dropdown({ value, setValue, error, loading, ...props }) { ...dropdownProps } = props; return ( -
- setValue(selected.key, selected)} - validationState={getValidationState()} - {...dropdownProps} - /> -
+ setValue(selected.key, selected)} + validationState={getValidationState()} + {...dropdownProps} + /> ); } diff --git a/src/shared/ResourceForm/inputs/Number.js b/src/shared/ResourceForm/inputs/Number.js index c0c529197d..65747e83ed 100644 --- a/src/shared/ResourceForm/inputs/Number.js +++ b/src/shared/ResourceForm/inputs/Number.js @@ -3,14 +3,12 @@ import { Input } from '@ui5/webcomponents-react'; export function Number({ value = '', setValue, ...props }) { if (!props.readOnly) delete props.readOnly; return ( -
- setValue(parseInt(e.target.value) ?? null)} - {...props} - /> -
+ setValue(parseInt(e.target.value) ?? null)} + {...props} + /> ); } diff --git a/src/shared/ResourceForm/inputs/Switch.js b/src/shared/ResourceForm/inputs/Switch.js index cf0e46f671..4b75266624 100644 --- a/src/shared/ResourceForm/inputs/Switch.js +++ b/src/shared/ResourceForm/inputs/Switch.js @@ -2,8 +2,6 @@ import { Switch as UI5Switch } from '@ui5/webcomponents-react'; export function Switch({ value, setValue, ...props }) { return ( -
- setValue(!value)} checked={value} {...props} /> -
+ setValue(!value)} checked={value} {...props} /> ); } diff --git a/src/shared/ResourceForm/inputs/Text.js b/src/shared/ResourceForm/inputs/Text.js index 253100ba99..19614e062c 100644 --- a/src/shared/ResourceForm/inputs/Text.js +++ b/src/shared/ResourceForm/inputs/Text.js @@ -14,7 +14,6 @@ export function WrappedText({ value, setValue, onChange, inputRef, ...props }) { setMultiValue, setResource, validateMessage, - fullWidth = false, ...inputProps } = props; @@ -24,13 +23,11 @@ export function WrappedText({ value, setValue, onChange, inputRef, ...props }) { }); return ( -
- setValue && setValue(e.target.value))} - {...inputProps} - {...validationProps} - /> -
+ setValue && setValue(e.target.value))} + {...inputProps} + {...validationProps} + /> ); } diff --git a/src/shared/components/K8sResourceSelect.js b/src/shared/components/K8sResourceSelect.js index ce68493b10..444a85a96c 100644 --- a/src/shared/components/K8sResourceSelect.js +++ b/src/shared/components/K8sResourceSelect.js @@ -93,27 +93,25 @@ export function K8sResourceSelect({ }; return ( -
- {getValidationState()?.text}} - pattern={k8sNamePattern} - > - {options.map(option => ( - - ))} - -
+ {getValidationState()?.text}} + pattern={k8sNamePattern} + > + {options.map(option => ( + + ))} + ); } diff --git a/src/shared/components/ResourceDetails/ResourceDetails.scss b/src/shared/components/ResourceDetails/ResourceDetails.scss index 96b9498362..244d4f803f 100644 --- a/src/shared/components/ResourceDetails/ResourceDetails.scss +++ b/src/shared/components/ResourceDetails/ResourceDetails.scss @@ -1,10 +1,10 @@ -.cluster-overview__details-wrapper, .cluster-overview__details-grid { display: grid; gap: 1rem; } .cluster-overview__details-wrapper { + display: grid; grid-template-columns: 1fr; min-width: 200px; width: fit-content; diff --git a/src/shared/components/ResourceRef/ExternalResourceRef.js b/src/shared/components/ResourceRef/ExternalResourceRef.js index f5844041e0..439e184020 100644 --- a/src/shared/components/ResourceRef/ExternalResourceRef.js +++ b/src/shared/components/ResourceRef/ExternalResourceRef.js @@ -108,34 +108,32 @@ export function ExternalResourceRef({ resource: labelPrefix, })} input={() => ( -
- { - const selectedOption = namespacesOptions.find( - o => o.text === event.target.value, - ); - if (selectedOption) - setValue({ name: '', namespace: selectedOption.text }); - }} - required={required} - value={value?.namespace || ''} - valueState={namespaceValid ? null : 'Error'} - valueStateMessage={ - - {namespaceValid - ? '' - : t('common.messages.resource-namespace-error')} - - } - > - {namespacesOptions.map(namespace => ( - - ))} - -
+ { + const selectedOption = namespacesOptions.find( + o => o.text === event.target.value, + ); + if (selectedOption) + setValue({ name: '', namespace: selectedOption.text }); + }} + required={required} + value={value?.namespace || ''} + valueState={namespaceValid ? null : 'Error'} + valueStateMessage={ + + {namespaceValid + ? '' + : t('common.messages.resource-namespace-error')} + + } + > + {namespacesOptions.map(namespace => ( + + ))} + )} />, { // secret chooseComboboxOption( - '[placeholder="Select Namespace"]', + '[placeholder="Select namespace"]', Cypress.env('NAMESPACE_NAME'), ); diff --git a/tests/kyma/tests/namespace/test-issuers.spec.js b/tests/kyma/tests/namespace/test-issuers.spec.js index f05a667ba2..ae265baec2 100644 --- a/tests/kyma/tests/namespace/test-issuers.spec.js +++ b/tests/kyma/tests/namespace/test-issuers.spec.js @@ -39,7 +39,7 @@ context('Test Issuers', () => { chooseComboboxOption('[placeholder="Select Issuer type"]', 'CA'); chooseComboboxOption( - '[placeholder="Select Namespace"]', + '[placeholder="Select namespace"]', Cypress.env('NAMESPACE_NAME'), );