EnxDev commented on code in PR #43820:
URL: https://github.com/apache/superset/pull/43820#discussion_r3933335482
##########
superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx:
##########
@@ -154,9 +269,158 @@ const renderOperator = ({
);
};
+const renderBoundFields = (
+ operator?: Comparator,
+ serverPagination?: boolean,
+) => {
+ const { showMin, showMax } = getBoundVisibility(operator);
+ if (!showMin && !showMax) {
+ return null;
+ }
+ // Cross-validate min/max only when both are shown; a lone bound
+ // validates against targetValue instead, its other end of the scale.
+ const useCrossFieldRules = showMin && showMax;
+ const minRules = useCrossFieldRules ? rulesMinBound : rulesMinBoundTarget;
+ const maxRules = useCrossFieldRules ? rulesMaxBound : rulesMaxBoundTarget;
+ const minDependencies = useCrossFieldRules ? minBoundDeps : targetValueDeps;
+ const maxDependencies = useCrossFieldRules ? maxBoundDeps : targetValueDeps;
+ // % of column derives its denominator from the loaded rows, which under
+ // server pagination is just the current page -- disable picking it there
+ // so the scale doesn't drift per page. Existing saved configs keep working.
+ const boundUnitSelectOptions = serverPagination
Review Comment:
Good catch. This is addressed in 3ae2f00aa4. getColorFormatters now accepts
a disablePercentBounds flag, and both Table and AG Grid pass serverPagination
through to it. As a result, an already-saved percent rule is suppressed instead
of being evaluated against the loaded page. The saved configuration remains
visible and editable, and the tooltip explains why percent mode is unavailable
under server pagination.
##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts:
##########
@@ -83,17 +138,61 @@ export const getColorFunction = (
targetValueRight,
colorScheme,
useGradient,
+ minBound: rawMinBound,
+ maxBound: rawMaxBound,
+ centerValue: rawCenterValue,
+ lowColor,
+ midColor,
+ highColor,
+ boundUnit,
+ percentDenominator,
}: ConditionalFormattingConfig,
columnValues: number[] | string[] | (boolean | null)[],
alpha?: boolean,
) => {
+ const resolvePercentBound = (bound: number | undefined) => {
+ if (boundUnit !== BoundUnit.Percent || bound === undefined) {
+ return bound;
+ }
+ const numericColumnValues = (
+ columnValues as (number | string | boolean | null)[]
+ ).filter((value): value is number => typeof value === 'number');
+ if (numericColumnValues.length === 0) {
+ return undefined;
+ }
+ // Use the magnitude so a negative denominator doesn't flip the scale;
+ // a zero denominator falls back to unset rather than collapsing it.
+ const denominatorValue = Math.abs(
Review Comment:
Agreed. Since the option is Column max, preserving the actual signed maximum
is the least surprising behavior. Addressed in f8c9745dc3: Column sum still
uses its magnitude, while Column max retains its sign. A regression test with
[-100, -5] verifies that 100% resolves to -5, and the documentation makes the
distinction explicit. This commit will appear on the PR after the next push.
##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts:
##########
@@ -129,7 +239,10 @@ export const getColorFunction = (
typeof targetValue === 'number' && value > targetValue!
? {
cutoffValue: targetValue!,
- extremeValue: Math.max(...allValues),
+ extremeValue:
+ maxBound !== undefined && maxBound > targetValue
Review Comment:
The popover catches this for Value mode: > and >= require maxBound >
targetValue, while < and <= require minBound < targetValue. In percent mode the
direct comparison is intentionally skipped because the bound and target use
different units until the column denominator is known. The formatter fallback
remains as a defensive path for imported or API-provided configurations. The
validation behavior is included in 3ae2f00aa4.
##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts:
##########
@@ -72,6 +75,58 @@ export const getOpacity = (
);
};
+const parseColorToRgb = (color: RGBColor | string) => {
+ if (typeof color === 'string') {
+ const { r, g, b } = tinycolor(color).toRgb();
+ return { r, g, b };
+ }
+ return { r: color.r, g: color.g, b: color.b };
+};
+
+export const getDivergingColor = (
+ value: number,
+ cutoffValue: number,
+ centerValue: number,
+ extremeValue: number,
+ lowColor: RGBColor | string,
+ midColor: RGBColor | string,
+ highColor: RGBColor | string,
+): string => {
+ const clampedValue = Math.min(Math.max(value, cutoffValue), extremeValue);
+ const belowCenter = clampedValue <= centerValue;
+ const from = parseColorToRgb(belowCenter ? lowColor : midColor);
+ const to = parseColorToRgb(belowCenter ? midColor : highColor);
+ const rangeStart = belowCenter ? cutoffValue : centerValue;
+ const rangeEnd = belowCenter ? centerValue : extremeValue;
+ const ratio =
+ rangeEnd === rangeStart
+ ? 1
+ : (clampedValue - rangeStart) / (rangeEnd - rangeStart);
+ return rgbaToHex({
+ r: from.r + (to.r - from.r) * ratio,
+ g: from.g + (to.g - from.g) * ratio,
+ b: from.b + (to.b - from.b) * ratio,
+ a: 1,
+ });
+};
+
+const isValidDivergingConfig = (
+ centerValue: number | undefined,
+ lowColor: RGBColor | string | undefined,
+ midColor: RGBColor | string | undefined,
+ highColor: RGBColor | string | undefined,
+ cutoffValue: number | string,
+ extremeValue: number | string,
+): centerValue is number =>
+ centerValue !== undefined &&
+ lowColor !== undefined &&
+ midColor !== undefined &&
+ highColor !== undefined &&
+ typeof cutoffValue === 'number' &&
+ typeof extremeValue === 'number' &&
+ centerValue > cutoffValue &&
+ centerValue < extremeValue;
Review Comment:
Confirmed that the popover surfaces this before submission. Its center
validators use strict comparisons, so centerValue must be greater than minBound
and smaller than maxBound; equality at either endpoint is rejected. The
formatter fallback remains defensive for imported or API-provided invalid
configurations.
##########
docs/docs/using-superset/creating-your-first-dashboard.mdx:
##########
@@ -328,6 +328,8 @@ Conditional formatting rules highlight cells based on their
values. Rules can be
Each rule has a **"Use gradient"** toggle: enabled applies a varying opacity
(lighter = further from threshold), disabled applies a solid fill at full
opacity regardless of value.
+For numeric rules, the optional **"Min bound"** / **"Max bound"** fields let
you override the auto-detected color range with fixed values instead of relying
on the minimum/maximum found in the data — useful when you want consistent
coloring across dashboards or data refreshes.
Review Comment:
Added in 3ae2f00aa4. The documentation now explains the diverging
low/mid/high scale, how percent bounds use Column max or Column sum, and why
percent mode is unavailable with server pagination.
--
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]
