sadpandajoe commented on code in PR #42910:
URL: https://github.com/apache/superset/pull/42910#discussion_r4250508646
##########
superset-frontend/src/explore/store.ts:
##########
@@ -89,6 +105,50 @@ export function handleDeprecatedControls(formData:
FormData): void {
formData.matrixify_mode_columns = 'disabled';
}
}
+
+ // #42910: migrate the legacy BigNumberPeriodOverPeriod
+ // `comparison_color_scheme` ('Green' | 'Red', where 'Red' reverses
+ // increase/decrease colors) into the `increase_color` / `decrease_color`
+ // ColorPickerControls that replaced it. `comparison_color_scheme` is no
+ // longer a registered control, so `getFormDataFromControls` drops it the
+ // next time the chart is saved -- without this migration, that silently
+ // discards a reversed-color choice the first time an old chart is resaved.
+ if (
+ formData.viz_type === VizType.BigNumberPeriodOverPeriod &&
+ formData.comparison_color_scheme &&
+ formData.increase_color === undefined &&
+ formData.decrease_color === undefined
+ ) {
+ const legacyReversed =
+ formData.comparison_color_scheme === ColorSchemeEnum.Red;
+ formData.increase_color = legacyReversed
+ ? ColorSchemeEnum.Red
+ : ColorSchemeEnum.Green;
+ formData.decrease_color = legacyReversed
+ ? ColorSchemeEnum.Green
+ : ColorSchemeEnum.Red;
+ }
+
+ // #42910: migrate the legacy Gauge `interval_color_indices`
+ // (comma-separated, 1-indexed positions into `color_scheme`) into real hex
+ // colors stored by `interval_colors`, which replaced it. Same rationale as
+ // the comparison_color_scheme migration above -- `interval_color_indices`
+ // is no longer a registered control, so it's otherwise dropped on save.
+ if (
+ formData.viz_type === VizType.Gauge &&
+ formData.interval_color_indices &&
+ (!formData.interval_colors || formData.interval_colors.length === 0)
+ ) {
+ const bounds = parseIntervalBounds(formData.intervals);
+ const resolved = resolveLegacyIntervalColors(
+ bounds,
+ formData.interval_color_indices,
+ formData.color_scheme,
+ );
+ if (resolved.some(color => color !== '')) {
+ formData.interval_colors = resolved;
Review Comment:
This resolves legacy gauge indices against the chart's own `color_scheme`
when the form data is hydrated, and the result is stored as literal
`interval_colors`. On a dashboard with a color scheme override
(`getFormDataWithExtraFilters` sets `color_scheme` from the dashboard after
`applyDefaultFormData` runs), a legacy gauge that used to take its interval
colors from the dashboard palette will now keep the old chart-level palette
colors, since `interval_colors` wins in `getIntervalBoundsAndColors`. Existing
dashboards would change color without anyone editing or saving the chart.
Should the migration be limited to the Explore edit path, or should the legacy
indices stay bound to the effective scheme at render time?
--
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]
