msyavuz commented on code in PR #43820:
URL: https://github.com/apache/superset/pull/43820#discussion_r3933340814
##########
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:
"percentage of the column total" is wrong when the denominator is `Column
max` (the default).
##########
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:
This drops the whole rule, so turning on server pagination silently removes
all coloring for that column. Better to treat the percent bounds as unset here
and fall back to data min/max instead.
--
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]
