rusackas commented on code in PR #42910:
URL: https://github.com/apache/superset/pull/42910#discussion_r4401991142
##########
superset-frontend/src/explore/components/controls/ColorPickerControl.tsx:
##########
@@ -193,10 +234,21 @@ export default function ColorPickerControl({
const rgb = color.toRgb();
const hex = rgbaToHex(rgb).toLowerCase();
+ // Match against the same hex the preset chip and trigger display, so a
+ // special color picked from a theme-resolved chip maps back to its key.
+ // Only keys the caller actually offered as presets are candidates;
+ // otherwise an ordinary theme swatch that shares a hex with Green/Red
+ // (e.g. `colorSuccess`) would be misread as the special key.
+ const offeredSpecialKeys = new Set(
+ (customPresets ?? []).flatMap(item =>
+ item.colors.filter(color => color in SPECIAL_COLORS),
+ ),
+ );
const specialEntry = resolveThemeTokens
- ? Object.entries(SPECIAL_COLORS).find(
- ([, rgba]) => rgbaToHex(rgba).toLowerCase() === hex,
- )
+ ? Object.keys(SPECIAL_COLORS)
+ .filter(key => offeredSpecialKeys.has(key))
+ .map(key => [key, toDisplayHex(key, themeColors, true)] as const)
Review Comment:
Good catch. The special-key check now normalizes both sides to hex like
`reverseMap` does, with a test using an `rgb(...)` `colorSuccess`.
##########
superset-frontend/src/explore/store.ts:
##########
@@ -89,6 +110,60 @@ 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: on request, 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. Defaults to off,
+ // unlike the comparison_color_scheme migration above: the Gauge control
+ // panel keeps `interval_color_indices` registered as a hidden control, so
+ // Explore (which builds the chart's query and save form data from
+ // registered controls) still hands an untouched legacy string to the
+ // renderer and keeps it on save, where it stays palette-relative. Eagerly
+ // freezing it into literal colors merely from opening the chart would
+ // silently break a dashboard override of this chart's `color_scheme`
+ // applied afterward -- the exact hazard the dashboard hydration path
+ // below was already written to avoid. `IntervalColorsControl` shows the
+ // resolved colors for display on its own (see its
+ // `legacyIntervalColorIndices` prop), and only the user's first edit of
+ // an interval color makes them literal.
+ if (
+ resolveLegacyGaugeColors &&
Review Comment:
No caller opts in, so I dropped the option, the branch and its tests. Legacy
indices are still resolved for display by `IntervalColorsControl`.
--
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]
