From 01a16b4d2eac0dac492d6bf604db90b546ea08f7 Mon Sep 17 00:00:00 2001 From: Arko Date: Thu, 19 Oct 2023 04:18:32 -0700 Subject: [PATCH 1/3] fixed the bug where the percentage cell bars were not showing --- .../plugins/plugin-chart-table/src/TableChart.tsx | 12 ++++++++++-- 1 file changed, 10 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( const getColumnConfigs = useCallback( (column: DataColumnMeta, i: number): ColumnWithLooseAccessor => { - 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( (config.showCellBars === undefined ? showCellBars : config.showCellBars) && - (isMetric || isRawRecords) && + (isMetric || isRawRecords || isPercentMetric) && getValueRange(key, alignPositiveNegative); let className = ''; From 4684ccce02d4be124509242228f605e2718f4758 Mon Sep 17 00:00:00 2001 From: Arko Date: Wed, 25 Oct 2023 04:55:02 -0700 Subject: [PATCH 2/3] created a test to make sure the cell bars are showing properly on the table charts --- .../test/TableChart.test.tsx | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) 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..e4c6e16a08034 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( , ); + 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: , + }), + ); + let cells = document.querySelectorAll('div.cell-bar'); + cells.forEach(cell => { + expect(cell.classList.contains('positive')).toBe(true); + }); + props.columns[0].isMetric = false; + props.columns[0].isPercentMetric = true; + + render( + ProviderWrapper({ + children: , + }), + ); + cells = document.querySelectorAll('div.cell-bar'); + cells.forEach(cell => { + expect(cell.classList.contains('positive')).toBe(true); + }); + + props.showCellBars = false; + + render( + ProviderWrapper({ + children: , + }), + ); + cells = document.querySelectorAll('td'); + + cells.forEach(cell => { + expect(cell.classList.contains('test-c7w8t3')).toBe(true); + }); + + props.columns[0].isPercentMetric = false; + props.columns[0].isMetric = true; + + render( + ProviderWrapper({ + children: , + }), + ); + cells = document.querySelectorAll('td'); + cells.forEach(cell => { + expect(cell.classList.contains('test-c7w8t3')).toBe(true); + }); + }); }); From cef71e02eb9a86ce83140fcf0a5f76089658fb76 Mon Sep 17 00:00:00 2001 From: Arko Date: Mon, 30 Oct 2023 14:19:20 -0700 Subject: [PATCH 3/3] lint fix --- .../plugins/plugin-chart-table/test/TableChart.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 e4c6e16a08034..d6998476baa2b 100644 --- a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx +++ b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx @@ -260,7 +260,7 @@ describe('plugin-chart-table', () => { ); let cells = document.querySelectorAll('div.cell-bar'); cells.forEach(cell => { - expect(cell.classList.contains('positive')).toBe(true); + expect(cell).toHaveClass('positive'); }); props.columns[0].isMetric = false; props.columns[0].isPercentMetric = true; @@ -272,7 +272,7 @@ describe('plugin-chart-table', () => { ); cells = document.querySelectorAll('div.cell-bar'); cells.forEach(cell => { - expect(cell.classList.contains('positive')).toBe(true); + expect(cell).toHaveClass('positive'); }); props.showCellBars = false; @@ -285,7 +285,7 @@ describe('plugin-chart-table', () => { cells = document.querySelectorAll('td'); cells.forEach(cell => { - expect(cell.classList.contains('test-c7w8t3')).toBe(true); + expect(cell).toHaveClass('test-c7w8t3'); }); props.columns[0].isPercentMetric = false; @@ -298,7 +298,7 @@ describe('plugin-chart-table', () => { ); cells = document.querySelectorAll('td'); cells.forEach(cell => { - expect(cell.classList.contains('test-c7w8t3')).toBe(true); + expect(cell).toHaveClass('test-c7w8t3'); }); }); });