Skip to content

Commit

Permalink
fix(table chart): Show Cell Bars correctly apache#25625 (apache#25707)
Browse files Browse the repository at this point in the history
  • Loading branch information
SA-Ark authored and sfirke committed Mar 22, 2024
1 parent f8558b4 commit 18416ab
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 2 deletions.
12 changes: 10 additions & 2 deletions superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 = '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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');
});
});
});

0 comments on commit 18416ab

Please sign in to comment.