rusackas opened a new pull request, #42910:
URL: https://github.com/apache/superset/pull/42910

   <!---
   Please write the PR title following the conventions at 
https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   
   Follow-up to #42053, which upgraded `ColorPickerControl` (custom presets, 
`resolveThemeTokens`, `outputFormat`) and used it to fix 
`ConditionalFormattingControl`. This PR finds and fixes the four remaining 
places in the app that still used a legacy/degraded color-selection UI instead 
of the shared picker, one commit per item.
   
   #### 1. Gauge chart interval colors
   
   `Gauge/controlPanel.tsx`'s "Interval colors" control asked users to type 
comma-separated 1-indexed positions into the chosen color scheme (e.g. 
`1,2,4`), with zero visual feedback and silent discarding of malformed input.
   
   Replaced with `IntervalColorsControl`: one `ColorPickerControl` per interval 
bound (parsed from the existing `intervals` control), storing real hex colors 
in a new `interval_colors` field, positionally matched to those bounds.
   
   **Design decision:** bounds stay owned by the existing `intervals` text 
control rather than being folded into the new control's own row list (which the 
row-list add/remove pattern in the task brief technically implied). This keeps 
a single source of truth for bounds and avoids needing two-way sync between two 
independent controls — the new control's row *count* simply tracks whatever 
`intervals` currently contains.
   
   **Backward compatibility:** charts saved before this control existed only 
have `interval_color_indices` (the old index strings). 
`getIntervalBoundsAndColors` in `transformProps.ts` still resolves those 
indices against the categorical scheme at render time whenever 
`interval_colors` is empty, so existing dashboards render identically with no 
migration. The control also resolves legacy indices to real colors for display 
the first time such a chart's panel is reopened (editor convenience only, not 
required for correct rendering).
   
   ![Gauge: new Interval colors control + rendered 
chart](https://gist.githubusercontent.com/rusackas/b1accdefe51ca5c3f14bbec364dddc92/raw/976039b415ea5b32b3b67f6326d8eac5bab01df4/1-gauge-controls.png)
   
   #### 2. Bullet chart band colors
   
   Bullet chart background bands were hardcoded to a 4-step theme-token ramp 
with **no** color control at all — a genuinely new feature, not a swap.
   
   Added an optional `range_colors` control (`BulletRangeColorsControl`): one 
`ColorPickerControl` per threshold parsed from the existing `ranges` control, 
each starting unset ("use default") with a "Use default" link to clear a 
customization. `transformProps.ts` captures each range's chosen color by its 
*original* (pre-sort) position in `ranges` before the existing largest-first 
band sort reorders them for nested drawing, so colors stay pinned to the 
correct threshold regardless of draw order.
   
   **Backward compatible by construction:** `range_colors` is optional and 
defaults to empty, so charts saved before this control existed have no such 
field and keep rendering with the exact default ramp.
   
   ![Bullet: new Range colors control + rendered chart with custom band 
colors](https://gist.githubusercontent.com/rusackas/b1accdefe51ca5c3f14bbec364dddc92/raw/39973cccdc468af8ff3ba8e1515a217649585554/2-bullet-controls.png)
   
   #### 3. Big Number Period-over-Period comparison colors
   
   The comparison-color control was a 2-choice `SelectControl` ("Green for 
increase, red for decrease" / reverse) bound directly to 
`theme.colorSuccess`/`theme.colorError`.
   
   Replaced with two `ColorPickerControl`s, `increase_color` / 
`decrease_color`, using the exact `resolveThemeTokens` + `outputFormat="hex"` 
pattern #42053 introduced: picking the Green/Red preset swatch stores the token 
*name* (so it still reads the same as before for users who just want the 
classic behavior), while any other pick stores a literal hex color. Exported 
`SPECIAL_COLORS`/`SpecialColorKey` from `ColorPickerControl.tsx` so this call 
site doesn't redefine the Green/Red mapping.
   
   The color→style resolution moved into two small, independently unit-tested 
pure functions in `utils.ts` (`resolveComparisonColorKeys`, 
`getComparisonColorTokens`) rather than living inline in `PopKPI`'s render body 
— jsdom doesn't reliably expose emotion's injected styles to `toHaveStyle` for 
direct component assertions, so the logic needed to be testable on its own.
   
   **Backward compatibility:** `resolveComparisonColorKeys` falls back to the 
legacy `comparisonColorScheme` field (kept, `@deprecated` in `types.ts`) 
whenever the new fields are absent — including correctly *reversing* 
increase/decrease for charts saved with the old "Red for increase, green for 
decrease" choice, the case a naive default-to-Green migration would have 
silently broken.
   
   ![Big Number PoP: new Color for increase/decrease pickers + rendered 
comparison](https://gist.githubusercontent.com/rusackas/b1accdefe51ca5c3f14bbec364dddc92/raw/635e9480ae84aa08607f82349b59ca33b49e71bd/3-pop-controls.png)
   
   #### 4. Admin Theme editor curated colors
   
   `ThemeModal.tsx` only exposed antd theming as a single JSON textarea, 
requiring admins to paste in a whole token object from an external tool to 
change even one color.
   
   Added a "Colors" section (`ThemeColorPickers`) above the JSON textarea with 
one `ColorPickerControl` per curated antd token — the 5 SEED colors 
(`colorPrimary`, `colorSuccess`, `colorWarning`, `colorError`, `colorInfo`) 
plus 6 load-bearing map/alias tokens (`colorLink`, `colorText`, 
`colorTextSecondary`, `colorBgBase`, `colorBgContainer`, `colorBorder`). This 
intentionally does **not** attempt the full 100+ token surface — everything 
else stays fully editable via the JSON textarea, which remains the source of 
truth. Names are taken directly from antd's own `SeedToken`/`MapToken` types, 
not invented.
   
   Sync is two-way, via two small pure functions (`tryParseThemeJson`, 
`patchThemeJsonToken`):
   - **Picker → JSON:** patches just that key into the JSON's `token` object 
and re-serializes with the modal's existing 2-space indent, preserving every 
other key (curated or not).
   - **JSON → pickers:** each render re-parses the textarea's current value and 
re-derives picker values from it.
   - **Invalid/mid-edit JSON:** `tryParseThemeJson` returns `null` instead of 
throwing (matching the file's existing `isValidJson` convention); the section 
shows a small notice and pickers stop persisting edits until the JSON is valid 
again.
   
   The section is hidden for read-only system themes, matching the existing 
Format/Apply button visibility.
   
   **No backward-compat concern** — additive UI over the same JSON, nothing 
about existing saved themes changes.
   
   ![Theme editor: curated Colors section synced from 
JSON](https://gist.githubusercontent.com/rusackas/b1accdefe51ca5c3f14bbec364dddc92/raw/069dad8a867c9ce33580f50551f1111a7ab95cc2/4-theme-modal.png)
   
   ### TESTING INSTRUCTIONS
   
   - `npm run test` in `superset-frontend/` — new/updated suites:
     - `plugins/plugin-chart-echarts/test/Gauge/transformProps.test.ts`
     - 
`src/explore/components/controls/IntervalColorsControl/IntervalColorsControl.test.tsx`
     - `plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts`
     - 
`src/explore/components/controls/BulletRangeColorsControl/BulletRangeColorsControl.test.tsx`
     - 
`plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/{utils,PopKPI}.test.tsx`
     - `src/features/themes/{ThemeModal,ThemeColorPickers}.test.tsx`
   - Manually: create/edit a Gauge chart, expand Customize → Intervals; 
create/edit a Bullet chart, expand Customize → Range colors; create/edit a Big 
Number w/ Time Comparison chart with "Add color for positive/negative change" 
enabled; open Settings → Themes → + Theme.
   - Backward compatibility: open an existing Gauge chart saved with 
`interval_color_indices` only, an existing Bullet chart with no `range_colors`, 
and a Big Number PoP chart saved with only `comparison_color_scheme` (including 
the `Red` value) — all three should render identically to before this PR.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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]

Reply via email to