fix: don't color empty cells in table chart with color formatters

apache · Sep 17, 2022 · c24bb63 · c24bb63
1 parent 6e8cad3
commit c24bb63
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 4 deletions.
diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
@@ -393,9 +393,9 @@ export default function TableChart<D extends DataRecord = DataRecord>(
             columnColorFormatters!
               .filter(formatter => formatter.column === column.key)
               .forEach(formatter => {
-                const formatterResult = formatter.getColorFromValue(
-                  value as number,
-                );
+                const formatterResult = value
+                  ? formatter.getColorFromValue(value as number)
+                  : false;
                 if (formatterResult) {
                   backgroundColor = formatterResult;
                 }

diff --git a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
@@ -22,7 +22,8 @@ import TableChart from '../src/TableChart';
 import transformProps from '../src/transformProps';
 import DateWithFormatter from '../src/utils/DateWithFormatter';
 import testData from './testData';
-import { mount } from './enzyme';
+import { mount, ProviderWrapper } from './enzyme';
+import { render, screen } from '@testing-library/react';
 
 describe('plugin-chart-table', () => {
   describe('transformProps', () => {
@@ -105,5 +106,81 @@ describe('plugin-chart-table', () => {
       tree = wrap.render();
       expect(tree.text()).toContain('No records found');
     });
+
+    it('render color with column color formatter', async () => {
+      render(
+        ProviderWrapper({
+          children: (
+            <TableChart
+              {...transformProps({
+                ...testData.advanced,
+                rawFormData: {
+                  ...testData.advanced.rawFormData,
+                  conditional_formatting: [
+                    {
+                      colorScheme: '#ACE1C4',
+                      column: 'sum__num',
+                      operator: '>',
+                      targetValue: 2467,
+                    },
+                  ],
+                },
+              })}
+            />
+          ),
+        }),
+      );
+
+      expect(
+        getComputedStyle(await screen.getByTitle('2467063')).background,
+      ).toBe('rgba(172, 225, 196, 1)');
+      expect(getComputedStyle(await screen.getByTitle('2467')).background).toBe(
+        '',
+      );
+    });
+
+    it('render cell without color', async () => {
+      const dataWithEmptyCell = testData.advanced.queriesData[0];
+      dataWithEmptyCell.data.push({
+        __timestamp: null,
+        name: 'Noah',
+        sum__num: null,
+        '%pct_nice': 0.643,
+        'abc.com': 'bazzinga',
+      });
+
+      render(
+        ProviderWrapper({
+          children: (
+            <TableChart
+              {...transformProps({
+                ...testData.advanced,
+                queriesData: [dataWithEmptyCell],
+                rawFormData: {
+                  ...testData.advanced.rawFormData,
+                  conditional_formatting: [
+                    {
+                      colorScheme: '#ACE1C4',
+                      column: 'sum__num',
+                      operator: '<',
+                      targetValue: 12342,
+                    },
+                  ],
+                },
+              })}
+            />
+          ),
+        }),
+      );
+      expect(getComputedStyle(await screen.getByTitle('2467')).background).toBe(
+        'rgba(172, 225, 196, 0.812)',
+      );
+      expect(
+        getComputedStyle(await screen.getByTitle('2467063')).background,
+      ).toBe('');
+      expect(getComputedStyle(await screen.getByText('N/A')).background).toBe(
+        '',
+      );
+    });
   });
 });