michael-s-molina commented on code in PR #21482:
URL: https://github.com/apache/superset/pull/21482#discussion_r973262606
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -404,16 +429,33 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
const StyledCell = styled.td`
text-align: ${sharedStyle.textAlign};
- background: ${backgroundColor ||
- (valueRange
- ? cellBar({
+ white-space: ${value instanceof Date ? 'nowrap' : undefined};
+ position: relative;
+ background: ${backgroundColor || undefined};
+ `;
+
+ const cellBarStyles = css`
+ position: absolute;
+ height: 100%;
+ display: block;
+ top: 0;
+ ${valueRange &&
Review Comment:
A good way of implementing this behavior would be to use
[composition](https://emotion.sh/docs/composition). You can define the optional
part in its own `css` variable and compound depending on the condition.
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -80,44 +82,67 @@ function getSortTypeByDataType(dataType: GenericDataType):
DefaultSortTypes {
}
/**
- * Cell background to render columns as horizontal bar chart
+ * Cell background width calculation for horizontal bar chart
*/
-function cellBar({
+function cellWidth({
value,
valueRange,
- colorPositiveNegative = false,
alignPositiveNegative,
}: {
value: number;
valueRange: ValueRange;
- colorPositiveNegative: boolean;
alignPositiveNegative: boolean;
}) {
const [minValue, maxValue] = valueRange;
- const r = colorPositiveNegative && value < 0 ? 150 : 0;
if (alignPositiveNegative) {
const perc = Math.abs(Math.round((value / maxValue) * 100));
- // The 0.01 to 0.001 is a workaround for what appears to be a
- // CSS rendering bug on flat, transparent colors
- return (
- `linear-gradient(to right, rgba(${r},0,0,0.2), rgba(${r},0,0,0.2)
${perc}%, ` +
- `rgba(0,0,0,0.01) ${perc}%, rgba(0,0,0,0.001) 100%)`
- );
+ return perc;
}
const posExtent = Math.abs(Math.max(maxValue, 0));
const negExtent = Math.abs(Math.min(minValue, 0));
const tot = posExtent + negExtent;
+ const perc2 = Math.round((Math.abs(value) / tot) * 100);
+ return perc2;
+}
+
+/**
+ * Cell left margin (offset) calculation for horizontal bar chart elements
+ * when alignPositiveNegative is not set
+ */
+function cellOffset({
+ value,
+ valueRange,
+ alignPositiveNegative,
+}: {
+ value: number;
+ valueRange: ValueRange;
+ alignPositiveNegative: boolean;
+}) {
+ if (alignPositiveNegative) {
+ return 0;
+ }
+ const [minValue, maxValue] = valueRange;
+ const posExtent = Math.abs(Math.max(maxValue, 0));
+ const negExtent = Math.abs(Math.min(minValue, 0));
+ const tot = posExtent + negExtent;
const perc1 = Math.round(
(Math.min(negExtent + value, negExtent) / tot) * 100,
);
- const perc2 = Math.round((Math.abs(value) / tot) * 100);
- // The 0.01 to 0.001 is a workaround for what appears to be a
- // CSS rendering bug on flat, transparent colors
- return (
- `linear-gradient(to right, rgba(0,0,0,0.01), rgba(0,0,0,0.001) ${perc1}%,
` +
- `rgba(${r},0,0,0.2) ${perc1}%, rgba(${r},0,0,0.2) ${perc1 + perc2}%, ` +
- `rgba(0,0,0,0.01) ${perc1 + perc2}%, rgba(0,0,0,0.001) 100%)`
- );
+ return perc1;
Review Comment:
Not sure if you created the variable for readability instead of just
returning. If that's the case maybe rename it to `perc`?
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -449,6 +491,15 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
// render `Cell`. This saves some time for large tables.
return (
<StyledCell {...cellProps}>
+ {valueRange && (
+ <div
+ className={cx(
+ 'cell-bar',
+ value && value < 0 ? 'negative' : 'positive',
Review Comment:
Can we add a comment to the code indicating that the intent for these
classes is to support customizations? I just want to avoid the case where
someone removes them because they are not referenced anywhere.
--
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]
