From 18416abcaff8bdbf210c4c6fa2a5e66aaf48e5a2 Mon Sep 17 00:00:00 2001
From: Arko <90512504+SA-Ark@users.noreply.github.com>
Date: Mon, 6 Nov 2023 03:36:21 -0800
Subject: [PATCH] fix(table chart): Show Cell Bars correctly #25625 (#25707)

---
 .../plugin-chart-table/src/TableChart.tsx     | 12 +++-
 .../test/TableChart.test.tsx                  | 59 +++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
index 72bcb71d290fa..917abb929a8ec 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
@@ -409,7 +409,15 @@ export default function TableChart<D extends DataRecord = DataRecord>(
 
   const getColumnConfigs = useCallback(
     (column: DataColumnMeta, i: number): ColumnWithLooseAccessor<D> => {
-      const { key, label, isNumeric, dataType, isMetric, config = {} } = column;
+      const {
+        key,
+        label,
+        isNumeric,
+        dataType,
+        isMetric,
+        isPercentMetric,
+        config = {},
+      } = column;
       const columnWidth = Number.isNaN(Number(config.columnWidth))
         ? config.columnWidth
         : Number(config.columnWidth);
@@ -438,7 +446,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
         (config.showCellBars === undefined
           ? showCellBars
           : config.showCellBars) &&
-        (isMetric || isRawRecords) &&
+        (isMetric || isRawRecords || isPercentMetric) &&
         getValueRange(key, alignPositiveNegative);
 
       let className = '';
diff --git a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
index 9bc3d90a09dd9..d6998476baa2b 100644
--- a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
@@ -76,6 +76,7 @@ describe('plugin-chart-table', () => {
       wrap = mount(
         <TableChart {...transformProps(testData.basic)} sticky={false} />,
       );
+
       tree = wrap.render(); // returns a CheerioWrapper with jQuery-like API
       const cells = tree.find('td');
       expect(cells).toHaveLength(12);
@@ -158,6 +159,7 @@ describe('plugin-chart-table', () => {
         }),
       );
       const cells = document.querySelectorAll('td');
+
       expect(document.querySelectorAll('th')[0]).toHaveTextContent('num');
       expect(cells[0]).toHaveTextContent('$ 1.23k');
       expect(cells[1]).toHaveTextContent('$ 10k');
@@ -242,4 +244,61 @@ describe('plugin-chart-table', () => {
       expect(getComputedStyle(screen.getByText('N/A')).background).toBe('');
     });
   });
+
+  it('render cell bars properly, and only when it is toggled on in both regular and percent metrics', () => {
+    const props = transformProps({
+      ...testData.raw,
+      rawFormData: { ...testData.raw.rawFormData },
+    });
+
+    props.columns[0].isMetric = true;
+
+    render(
+      ProviderWrapper({
+        children: <TableChart {...props} sticky={false} />,
+      }),
+    );
+    let cells = document.querySelectorAll('div.cell-bar');
+    cells.forEach(cell => {
+      expect(cell).toHaveClass('positive');
+    });
+    props.columns[0].isMetric = false;
+    props.columns[0].isPercentMetric = true;
+
+    render(
+      ProviderWrapper({
+        children: <TableChart {...props} sticky={false} />,
+      }),
+    );
+    cells = document.querySelectorAll('div.cell-bar');
+    cells.forEach(cell => {
+      expect(cell).toHaveClass('positive');
+    });
+
+    props.showCellBars = false;
+
+    render(
+      ProviderWrapper({
+        children: <TableChart {...props} sticky={false} />,
+      }),
+    );
+    cells = document.querySelectorAll('td');
+
+    cells.forEach(cell => {
+      expect(cell).toHaveClass('test-c7w8t3');
+    });
+
+    props.columns[0].isPercentMetric = false;
+    props.columns[0].isMetric = true;
+
+    render(
+      ProviderWrapper({
+        children: <TableChart {...props} sticky={false} />,
+      }),
+    );
+    cells = document.querySelectorAll('td');
+    cells.forEach(cell => {
+      expect(cell).toHaveClass('test-c7w8t3');
+    });
+  });
 });