[CLDN-1968] Added more UI/UX changes based on QA feedback

CybercentreCanada · Mar 6, 2023 · a3e8eb7 · a3e8eb7
1 parent bc41022
commit a3e8eb7
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 20 deletions.
diff --git a/superset-frontend/src/cccs-viz/plugins/plugin-chart-cccs-grid/src/ExpandAllValueRenderer.tsx b/superset-frontend/src/cccs-viz/plugins/plugin-chart-cccs-grid/src/ExpandAllValueRenderer.tsx
@@ -1,3 +1,4 @@
+import { GroupCellRenderer } from '@ag-grid-enterprise/all-modules';
 import React, { Component } from 'react';
 import './Buttons.css';
 
@@ -41,13 +42,21 @@ export default class ExpandAllValueRenderer extends Component<
   getJSONCells = () => {
     const instances = this.state.api.getCellRendererInstances();
 
-    const newInstances = instances.filter(
-      (instance: any) =>
-        instance.params.rowIndex === this.state.rowIndex &&
-        instance.params.column.colDef.cellRenderer === 'jsonValueRenderer',
-    );
-
-    return newInstances;
+    // Make sure row grouping is not enabled, but if it is, don't
+    // try to find all of the JSON blobs in the row
+    if (
+      instances.filter((instance: any) => instance instanceof GroupCellRenderer)
+        .length === 0
+    ) {
+      const newInstances = instances.filter(
+        (instance: any) =>
+          instance.params.rowIndex === this.state.rowIndex &&
+          instance.params.column.colDef.cellRenderer === 'jsonValueRenderer',
+      );
+
+      return newInstances;
+    }
+    return [];
   };
 
   // Set the current `expanded` field to the opposite of what it currently is

diff --git a/superset-frontend/src/cccs-viz/plugins/plugin-chart-cccs-grid/src/JsonValueRenderer.tsx b/superset-frontend/src/cccs-viz/plugins/plugin-chart-cccs-grid/src/JsonValueRenderer.tsx
@@ -102,15 +102,22 @@ export default class JsonValueRenderer extends Component<
       () => {
         const instances = this.state.api.getCellRendererInstances();
 
-        instances
-          .filter((instance: any) => !(instance instanceof GroupCellRenderer))
-          .filter(
-            (instance: any) =>
-              instance.params.rowIndex === this.state.rowIndex &&
-              instance.params.column.colDef.cellRenderer ===
-                'expandAllValueRenderer',
-          )
-          .map((instance: any) => instance.componentInstance.checkState());
+        // Make sure row grouping is not enabled, but if it is, don't
+        // trigger the 'checkState` function in the expand all button for the row
+        if (
+          instances.filter(
+            (instance: any) => instance instanceof GroupCellRenderer,
+          ).length === 0
+        ) {
+          instances
+            .filter(
+              (instance: any) =>
+                instance.params.rowIndex === this.state.rowIndex &&
+                instance.params.column.colDef.cellRenderer ===
+                  'expandAllValueRenderer',
+            )
+            .map((instance: any) => instance.componentInstance.checkState());
+        }
       },
     );
   };
@@ -138,7 +145,8 @@ export default class JsonValueRenderer extends Component<
       }
       return collapseJSON(this.reverseState, jsonObject);
     }
-    return cellData ?? null;
+    // If the cellData is set to 'null' or undefined, return null
+    return cellData !== 'null' && cellData !== undefined ? cellData : null;
   }
 
   static getValueToDisplay(params: { valueFormatted: any; value: any }) {

diff --git a/superset-frontend/src/cccs-viz/plugins/plugin-chart-cccs-grid/src/plugin/controlPanel.tsx b/superset-frontend/src/cccs-viz/plugins/plugin-chart-cccs-grid/src/plugin/controlPanel.tsx
@@ -452,7 +452,7 @@ const config: ControlPanelConfig = {
                     return newState;
                   },
                   visibility: ({ controls }) =>
-                    // TODO properly emsure is Bool
+                    // TODO properly ensure is Bool
                     Boolean(controls?.emitFilter?.value),
                   canCopy: true,
                 },
@@ -588,8 +588,11 @@ config.controlPanelSections.push({
           renderTrigger: true,
           default: false,
           description: t(
-            'Whether to enable row grouping (this will only take affect after a save)',
+            'Whether to enable row grouping (this will only take affect after a save). NOTE: "JSON Row Expand" and "Row Grouping" are mutually exclusive. If "Row Grouping" is selected, "JSON Row Expand" will not be visible.',
           ),
+          visibility: ({ controls }) =>
+            // TODO properly ensure is Bool
+            Boolean(!controls?.enable_json_expand?.value),
         },
       },
     ],
@@ -613,7 +616,12 @@ config.controlPanelSections.push({
           label: t('JSON Row Expand'),
           renderTrigger: true,
           default: false,
-          description: t('Whether to enable row level JSON expand buttons'),
+          description: t(
+            'Whether to enable row level JSON expand buttons. NOTE: "JSON Row Expand" and "Row Grouping" are mutually exclusive. If "JSON Row Expand" is selected, "Row Grouping" will not be visible.',
+          ),
+          visibility: ({ controls }) =>
+            // TODO properly ensure is Bool
+            Boolean(!controls?.enable_grouping?.value),
         },
       },
     ],