sadpandajoe commented on code in PR #44665:
URL: https://github.com/apache/superset/pull/44665#discussion_r4140273965
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -1140,6 +1195,29 @@ export default function TableChart<D extends DataRecord
= DataRecord>(
} else {
valueToFormat = value;
}
+ // String cells that read as numbers ("1.00") must compare
+ // numerically, or comparator rules like `= 1` never match.
+ // Two kinds of rule are the exception, because both match on
+ // the string itself and parsing first would hide the value
+ // from them: a text comparator, and any rule whose target the
+ // control stored as text (a text column renders a text input,
+ // and Equal compares that target strictly).
+ if (
+ formatter.objectFormatting ===
+ ObjectFormattingEnum.CELL_BAR &&
+ !(
+ formatter.operator !== undefined &&
+ STRING_COMPARATORS.has(formatter.operator)
+ ) &&
+ typeof formatter.targetValue !== 'string' &&
+ valueToFormat !== null &&
+ valueToFormat !== undefined
+ ) {
+ const coerced = parseNumeric(valueToFormat);
Review Comment:
A numeric-string cell is now coerced before this formatter runs, but the
formatter's percentage bounds are still calculated from the raw string values
in `getColorFormatters`. Those values are filtered out when resolving percent
bounds, so a CELL_BAR rule using percentage bounds falls back to automatic
bounds and renders a different gradient. Could the formatter receive the same
numeric values used for the cell-bar comparison/range?
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -1050,11 +1084,29 @@ export default function TableChart<D extends DataRecord
= DataRecord>(
basicColorFormatters.length > 0;
const generalShowCellBars =
config.showCellBars === undefined ? showCellBars : config.showCellBars;
+ // A Cell bar conditional-formatting rule must keep working even when the
+ // generic "Show cell bars" toggle is off: the toggle controls the
default
+ // gradient, not whether an explicit formatter rule can draw its bar.
+ const hasCellBarFormatter =
+ hasColumnColorFormatters &&
+ columnColorFormatters.some(
+ formatter =>
+ formatter.objectFormatting === ObjectFormattingEnum.CELL_BAR &&
+ (formatter.columnFormatting
+ ? formatter.columnFormatting === key
+ : formatter.column === key),
+ );
+ // Render geometry parses numeric-looking strings, so the range has to be
+ // built from the same values or a string cell gets a width measured
+ // against a range that excluded it. An explicit CELL_BAR rule also lifts
+ // the numeric-column gate: a DECIMAL metric delivered as a string is not
+ // flagged isMetric, and it still needs proportionally scaled bars rather
+ // than a full-width band.
const valueRange =
!hasBasicColorFormatters &&
- generalShowCellBars &&
- (isMetric || isRawRecords || isPercentMetric) &&
- getValueRange(key, alignPositiveNegative);
+ (generalShowCellBars || hasCellBarFormatter) &&
+ (isMetric || isRawRecords || isPercentMetric || hasCellBarFormatter) &&
+ getValueRange(key, alignPositiveNegative, true);
Review Comment:
This now parses every raw-record column before building a generic bar range,
so numeric-looking string identifiers such as `"00123"` get bars when **Show
cell bars** is enabled. That also removes their cross-filter click handler
because the cell draws a bar. Should coercion be limited to columns whose
declared type is numeric, while still accepting DECIMAL values returned as
strings?
--
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]
