sadpandajoe commented on code in PR #44665:
URL: https://github.com/apache/superset/pull/44665#discussion_r4142831637
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -1188,37 +1237,55 @@ export default function TableChart<D extends DataRecord
= DataRecord>(
};
`;
- const cellBarStyles = css`
- position: absolute;
- height: 100%;
- display: block;
- top: 0;
- ${
- valueRange &&
- typeof value === 'number' &&
- valueRangeFlag &&
- `
- width: ${`${cellWidth({
- value: value as number,
- valueRange,
- alignPositiveNegative,
- })}%`};
- left: ${`${cellOffset({
- value: value as number,
- valueRange,
- alignPositiveNegative,
- })}%`};
- background-color: ${
- backgroundColorCellBar ||
- cellBackground({
- value: value as number,
- colorPositiveNegative,
- theme,
- })
- };
- `
- }
- `;
+ // Whether this particular cell draws a bar. A CELL_BAR rule can
match
+ // some cells of a column and not others, so the bar — and with it
the
+ // click-to-filter suppression, which exists because the bar overlay
+ // would swallow the click — has to be decided per cell rather than
+ // from the column-wide valueRange.
+ const cellDrawsBar =
+ (generalShowCellBars ? !!valueRange : false) ||
+ !!backgroundColorCellBar;
+
+ // Inline style for the same reason as the arrow below: the `css`
prop
+ // needs the emotion JSX pragma, which this codebase's own Jest/Babel
+ // config does not wire up. As a `css` block the bar's geometry was
+ // silently dropped under test, so bar presence was the only thing
+ // any assertion could see — including for a bar that had no width.
+ const barHasGeometry =
+ !!valueRange && numericValue !== undefined && valueRangeFlag;
+ const cellBarStyles: CSSProperties = {
+ position: 'absolute',
+ height: '100%',
+ display: 'block',
+ top: 0,
+ ...(barHasGeometry
+ ? {
+ width: `${cellWidth({
+ value: numericValue!,
+ valueRange: valueRange!,
+ alignPositiveNegative,
+ })}%`,
+ left: `${cellOffset({
+ value: numericValue!,
+ valueRange: valueRange!,
+ alignPositiveNegative,
+ })}%`,
+ backgroundColor:
+ backgroundColorCellBar ||
+ cellBackground({
+ value: numericValue!,
+ colorPositiveNegative,
+ theme,
+ }),
+ }
+ : backgroundColorCellBar
Review Comment:
A matching background rule clears `valueRangeFlag`, so a matching cell-bar
rule falls through to this 100%-width fallback instead of rendering its scaled
geometry. Could the cell-bar path retain its geometry when both rules match?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
