fix: Resource Validation refactor (kyma-project#4506)

* fix: Resource Validation refactor

* test: Adjust tests to new layout

* fix: Remove unused code

* fix: Initial default value

* fix

* fix: Move the logic to atom

* test: Clear localStorage

* fix: cleanup

* test: move clearLocalStorage

* fix: Adjust background

* test: adjust test

Author: OliwiaGowor

public/i18n/en.yaml

@@ -1449,12 +1449,12 @@ settings:
       title: Cluster Interaction
     resourcesValidation:
       customize: Customize
-      enabled-policies: Enabled Policies
       no-policies-found: No policies found
       reset: Reset
       title: Resource Validation
       validateResources: Validate Resources
       validation-disabled: Resource validation is disabled
+      validation-policies: Validation Policies
     showHiddenNamespaces: Show hidden Namespaces
     title: Advanced Options
   language:

src/components/Settings/ResourceValidation/ResourceValidationSettings.scss

@@ -1,6 +1,14 @@
+.resource-validation-table {
+  ui5-table-cell {
+    overflow-clip-margin: unset !important; // Otherwise switch focus is being cut off
+  }
+
+  ui5-table-header-row {
+    display: none;
+  }
+}
+
 .policy-row {
-  display: grid;
-  grid-template-columns: 1fr fit-content(50%);
   align-items: center;
   font-size: var(--sapFontSize);
 }

src/components/Settings/ResourceValidation/ResourceValidationSettings.tsx

@@ -1,38 +1,20 @@
 import { useTranslation } from 'react-i18next';
-import { useAtom } from 'jotai';
-import { Button, Switch } from '@ui5/webcomponents-react';
-import {
-  getExtendedValidateResourceState,
-  validateResourcesAtom,
-} from 'state/settings/validateResourcesAtom';
-import { validationSchemasAtom } from 'state/validationSchemasAtom';
+import { useAtomValue } from 'jotai';
+import { FlexBox, Label, Switch } from '@ui5/webcomponents-react';
 import { useMemo } from 'react';
 import { GenericList } from 'shared/components/GenericList/GenericList';
 
-import { useFeature } from 'hooks/useFeature';
-import { ValidationFeatureConfig } from 'state/validationEnabledSchemasAtom';
-import { UI5Panel } from 'shared/components/UI5Panel/UI5Panel';
-
+import { useSyncedValidateResources } from 'state/settings/validateResourcesAtom';
+import { validationSchemasAtom } from 'state/validationSchemasAtom';
 import './ResourceValidationSettings.scss';
-import { configFeaturesNames } from 'state/types';
-import { useAtomValue } from 'jotai';
 
 export default function ResourceValidationSettings() {
   const { t } = useTranslation();
-  const [validateResources, setValidateResources] = useAtom(
-    validateResourcesAtom,
-  );
-  const validationFeature = useFeature(
-    configFeaturesNames.RESOURCE_VALIDATION,
-  ) as ValidationFeatureConfig;
 
-  const {
-    isEnabled,
-    choosePolicies,
-    policies: selectedPolicies = (validationFeature?.isEnabled &&
-      validationFeature?.config?.policies) ||
-      [],
-  } = getExtendedValidateResourceState(validateResources);
+  const [{ isEnabled, policies }, setValidateResources] =
+    useSyncedValidateResources();
+
+  const selectedPolicies = policies;
 
   const validationSchemas = useAtomValue(validationSchemasAtom);
   const allOptions = useMemo(
@@ -42,12 +24,8 @@ export default function ResourceValidationSettings() {
         .sort((a, b) => (a.key < b.key ? -1 : 1)) ?? [],
     [validationSchemas],
   );
-
   const policyList = useMemo(() => {
-    const selectedPolicySet = selectedPolicies.reduce(
-      (agg, name) => agg.add(name),
-      new Set(),
-    );
+    const selectedPolicySet = new Set(selectedPolicies);
     return allOptions.map((option) => ({
       ...option,
       selected: selectedPolicySet.has(option.key),
@@ -57,133 +35,92 @@ export default function ResourceValidationSettings() {
   const toggleVisibility = () => {
     setValidateResources({
       isEnabled: !isEnabled,
-      choosePolicies,
       policies: selectedPolicies,
     });
   };
 
-  const enablePolicyCustomization = () => {
-    setValidateResources({
-      isEnabled,
-      choosePolicies: true,
-      policies:
-        (validationFeature?.isEnabled && validationFeature?.config?.policies) ||
-        [],
-    });
-  };
-
-  const disablePolicyCustomization = () => {
-    setValidateResources({
-      isEnabled,
-      choosePolicies: false,
-    });
-  };
-
   const deleteSelectedPolicy = (policyToDelete: string) => {
     setValidateResources({
       isEnabled,
-      choosePolicies,
-      policies: selectedPolicies.filter((policy) => policy !== policyToDelete),
+      policies: selectedPolicies.filter(
+        (policy: string) => policy !== policyToDelete,
+      ),
     });
   };
 
   const addSelectedPolicy = (policyToAdd: string) => {
     setValidateResources({
       isEnabled,
-      choosePolicies,
       policies: [...selectedPolicies, policyToAdd].sort(),
     });
   };
 
   return (
-    <UI5Panel
-      title={t('settings.general.resourcesValidation.validateResources')}
-      accessibleName={t('settings.general.accessible-name.validateResources')}
-      headerActions={
+    <>
+      <FlexBox
+        alignItems="Center"
+        gap={'0.5rem'}
+        className="resource-validation-container sap-margin-small"
+      >
+        <Label showColon>
+          {t('settings.general.resourcesValidation.validateResources')}
+        </Label>
         <Switch
           accessibleName={t(
             'settings.general.resourcesValidation.validateResources',
           )}
           checked={isEnabled}
           onChange={toggleVisibility}
         />
-      }
-    >
+      </FlexBox>
       {!isEnabled && (
         <div className="no-validation-info">
           <span className="bsl-has-color-status-4">
             {t('settings.general.resourcesValidation.validation-disabled')}
           </span>
         </div>
       )}
-      {isEnabled &&
-        (choosePolicies ||
-          policyList.filter((policy) => policy.selected).length > 0) && (
-          <>
-            <GenericList
-              title={t('settings.general.resourcesValidation.enabled-policies')}
-              //@ts-expect-error Type mismatch between js and ts
-              entries={
-                choosePolicies
-                  ? policyList
-                  : policyList.filter((policy) => policy.selected)
-              }
-              headerRenderer={() => ['policies']}
-              rowRenderer={(entry) => [
-                <div key={entry.text} className="policy-row">
-                  <span>{entry.text}</span>
-                  {choosePolicies && (
-                    <Switch
-                      accessibleName={t(
-                        'settings.general.resourcesValidation.select-policy',
-                        {
-                          name: entry.text,
-                        },
-                      )}
-                      checked={entry.selected}
-                      onChange={() => {
-                        if (entry.selected) deleteSelectedPolicy(entry.key);
-                        else addSelectedPolicy(entry.key);
-                      }}
-                    />
-                  )}
-                </div>,
-              ]}
-              extraHeaderContent={
-                <>
-                  {!choosePolicies && (
-                    <Button
-                      design="Transparent"
-                      endIcon="customize"
-                      onClick={enablePolicyCustomization}
-                    >
-                      {t('settings.general.resourcesValidation.customize')}
-                    </Button>
+      {isEnabled && policyList.length > 0 && (
+        <>
+          <GenericList
+            className="resource-validation-table"
+            title={t(
+              'settings.general.resourcesValidation.validation-policies',
+            )}
+            //@ts-expect-error Type mismatch between js and ts
+            entries={policyList}
+            headerRenderer={() => ['']} //Throws an error if it's empty - header column is hidden with CSS
+            rowRenderer={(entry) => [
+              <FlexBox gap="0.5rem" key={entry.text} className="policy-row">
+                <Switch
+                  accessibleName={t(
+                    'settings.general.resourcesValidation.select-policy',
+                    {
+                      name: entry.text,
+                    },
                   )}
-                  {choosePolicies && (
-                    <Button
-                      design="Transparent"
-                      endIcon="reset"
-                      onClick={disablePolicyCustomization}
-                    >
-                      {t('settings.general.resourcesValidation.reset')}
-                    </Button>
-                  )}
-                </>
-              }
-              searchSettings={{
-                showSearchSuggestion: false,
-                noSearchResultTitle: t(
-                  'settings.general.resourcesValidation.no-policies-found',
-                ),
-                noSearchResultSubtitle: '',
-                textSearchProperties: ['key', 'text'],
-                showSearchField: true,
-                allowSlashShortcut: true,
-              }}
-            />
-          </>
-        )}
-    </UI5Panel>
+                  checked={entry.selected}
+                  onChange={() => {
+                    if (entry.selected) deleteSelectedPolicy(entry.key);
+                    else addSelectedPolicy(entry.key);
+                  }}
+                />
+                <span>{entry.text}</span>
+              </FlexBox>,
+            ]}
+            searchSettings={{
+              showSearchSuggestion: false,
+              noSearchResultTitle: t(
+                'settings.general.resourcesValidation.no-policies-found',
+              ),
+              noSearchResultSubtitle: '',
+              textSearchProperties: ['key', 'text'],
+              showSearchField: true,
+              allowSlashShortcut: true,
+            }}
+          />
+        </>
+      )}
+    </>
   );
 }

src/components/Settings/Settings.scss

@@ -1,14 +1,9 @@
 .settings-dialog::part(content) {
-  background: var(--sapBackgroundColor);
   overflow-y: auto;
   width: 100%;
   height: 75vh;
-  padding: 0;
+  padding: 0 !important;
   list-style-type: none;
-
-  .vertical-tabs-wrapper {
-    height: 100% !important;
-  }
 }
 
 .settings-row {

src/resources/Namespaces/YamlUpload/ValidationSwitch.tsx

@@ -12,11 +12,8 @@ export const ValidationSwitch = () => {
     validateResourcesAtom,
   );
 
-  const {
-    isEnabled,
-    choosePolicies,
-    policies: selectedPolicies = [],
-  } = getExtendedValidateResourceState(validateResources);
+  const { isEnabled, policies: selectedPolicies = [] } =
+    getExtendedValidateResourceState(validateResources);
 
   return (
     <div className="validate-resources">
@@ -29,7 +26,6 @@ export const ValidationSwitch = () => {
         onChange={() =>
           setValidateResources({
             isEnabled: !isEnabled,
-            choosePolicies,
             policies: selectedPolicies,
           })
         }

src/shared/components/VerticalTabs/VerticalTabs.scss

@@ -1,10 +1,9 @@
 .vertical-tabs-wrapper {
   display: grid;
   grid-template-columns: 1fr 2fr;
+  column-gap: 1px;
   height: 100%;
 
-  background: var(--sapBackgroundColor);
-
   ul {
     padding: 0;
     margin: 0;

src/state/settings/validateResourcesAtom.ts

@@ -1,8 +1,11 @@
 import { atomWithStorage } from 'jotai/utils';
+import { useAtom } from 'jotai';
+import { useEffect, useMemo } from 'react';
+import { useFeature } from 'hooks/useFeature';
+import { configFeaturesNames } from 'state/types';
 
 export type ExtendedValidateResources = {
   isEnabled: boolean;
-  choosePolicies: boolean;
   policies?: string[];
 };
 
@@ -23,9 +26,43 @@ export const getExtendedValidateResourceState = (
   if (typeof validateResources === 'boolean') {
     return {
       isEnabled: validateResources,
-      choosePolicies: false,
     };
   } else {
     return validateResources;
   }
 };
+
+export const useSyncedValidateResources = () => {
+  const [validateResources, setValidateResources] = useAtom(
+    validateResourcesAtom,
+  );
+
+  const validationFeature = useFeature(configFeaturesNames.RESOURCE_VALIDATION);
+
+  const configPolicies = useMemo(() => {
+    return validationFeature?.isEnabled
+      ? validationFeature.config?.policies || []
+      : [];
+  }, [validationFeature]);
+
+  useEffect(() => {
+    if (typeof validateResources === 'boolean' && configPolicies.length > 0) {
+      setValidateResources({
+        isEnabled: validateResources,
+        policies: configPolicies,
+      });
+    }
+  }, [validateResources, configPolicies, setValidateResources]);
+
+  return useMemo(() => {
+    const extendedState = getExtendedValidateResourceState(validateResources);
+
+    return [
+      {
+        isEnabled: extendedState.isEnabled,
+        policies: extendedState.policies ?? configPolicies,
+      },
+      setValidateResources,
+    ] as const;
+  }, [validateResources, configPolicies, setValidateResources]);
+};