EnxDev commented on code in PR #43820:
URL: https://github.com/apache/superset/pull/43820#discussion_r3933399788
##########
superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts:
##########
@@ -328,9 +487,13 @@ export const getColorFormatters = memoizeOne(
data: DataRecord[],
theme?: Record<string, any>,
alpha?: boolean,
+ disablePercentBounds = false,
) =>
columnConfig?.reduce(
(acc: ColorFormatters, config: ConditionalFormattingConfig) => {
+ if (disablePercentBounds && config.boundUnit === BoundUnit.Percent) {
Review Comment:
Fixed in 792a456896. Enabling server pagination no longer drops the
formatter. For a saved percentage rule, only minBound, maxBound, and
centerValue are ignored and the rule falls back to the automatic data range,
preserving comparator and solid-color behavior. The shared-util regression and
the AG Grid transform regression now assert that the formatter remains present.
The tooltip and docs also make this fallback explicit.
##########
superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx:
##########
@@ -154,9 +276,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;
+ // Percentage bounds require the complete result set. Existing percentage
+ // configurations remain editable here, but the formatter suppresses them
+ // while server pagination is enabled.
+ const boundUnitSelectOptions = serverPagination
+ ? boundUnitOptions.map(option =>
+ option.value === boundUnitOptions[1].value
+ ? { ...option, disabled: true }
+ : option,
+ )
+ : boundUnitOptions;
+
+ return (
+ <>
+ <Row gutter={12}>
+ <Col span={12}>
+ <FormItem
+ name="boundUnit"
+ label={t('Bound unit')}
+ initialValue={boundUnitOptions[0].value}
+ tooltip={
+ serverPagination
+ ? t(
+ 'Value: type the exact numbers used for coloring below. %
of column is unavailable with Server pagination enabled, since each page would
compute a different percentage.',
+ )
+ : t(
+ 'Value: type the exact numbers used for coloring below. %
of column: type a percentage of the column total instead, so the rule keeps
working as the data changes.',
Review Comment:
Fixed in 792a456896. The tooltip now says that percent bounds use the column
maximum or sum selected below. The server-pagination version also explains that
existing percentage rules use the automatic data range while pagination is
enabled.
##########
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:
Follow-up in 792a456896: testing the documented minBound=0 and maxBound=100
case showed that preserving a negative maximum reverses the resolved endpoints.
A non-positive Column max denominator now treats the percentage bounds as unset
and falls back to the ordered, data-derived range. The regression covers [-100,
-5] with both 0% and 100%, and the docs describe the fallback.
--
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]
