SBIN2010 commented on code in PR #42910:
URL: https://github.com/apache/superset/pull/42910#discussion_r4353275203
##########
superset-frontend/plugins/plugin-chart-echarts/src/Gauge/controlPanel.tsx:
##########
@@ -278,15 +279,42 @@ const config: ControlPanelConfig = {
],
[
{
+ // Legacy 1-indexed positions into `color_scheme`, replaced by
+ // `interval_colors` below. Explore builds the chart's query and
+ // save form data from registered controls only, so this stays
+ // registered (hidden) to keep an untouched legacy chart
+ // rendering and saving with its original palette-relative
+ // colors. The renderer ignores it once `interval_colors` is
+ // populated by the first edit in the control below.
name: 'interval_color_indices',
config: {
- type: 'TextControl',
+ type: 'HiddenControl',
+ hidden: true,
+ },
+ },
+ ],
+ [
+ {
+ name: 'interval_colors',
+ config: {
+ type: 'IntervalColorsControl',
label: t('Interval colors'),
description: t(
- 'Comma-separated color picks for the intervals, e.g. 1,2,4.
Integers denote colors from the chosen color scheme and are 1-indexed. Length
must be matching that of interval bounds.',
+ 'Pick a color for each interval band defined above by its
upper bound. Charts saved with the legacy 1-indexed "Interval colors" text
field are automatically resolved against the chosen color scheme the first time
this panel is opened.',
),
Review Comment:
This description says legacy interval indices are resolved the first time
the panel is opened. `handleDeprecatedControls` now leaves
`interval_color_indices` palette-relative until the user edits a color, and the
hidden control is what keeps that value on save. Could the description say the
colors stay scheme-relative until an interval color is edited?
##########
superset-frontend/src/explore/components/controls/ColorPickerControl.tsx:
##########
Review Comment:
With `resolveThemeTokens`, `toDisplayHex` shows `theme.colorSuccess` for the
stored value `"Green"`, but this preset swatch is still `SPECIAL_COLORS.Green`
(`#00960033`). The trigger and the preset chip no longer share a hex, so the
Green preset does not read as the selected value. Could the preset list use the
same `SPECIAL_COLOR_THEME_TOKENS` lookup as `toDisplayHex`?
--
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]
