bikash-barnwal opened a new pull request, #43180:
URL: https://github.com/apache/superset/pull/43180
### SUMMARY
On a dashboard whose `map_label_colors` is empty or incomplete, a single
chart could paint two series the same color — reproducible by importing the
dashboard from #36406 and reloading a few times, and it disappears once the
dashboard has been edited (which fills `map_label_colors`).
`CategoricalColorScale.getColor()` checked collisions only against colors
already handed out *within the current chart* (`isColorUsed` →
`chartLabelsColorMap`). A label missing from the shared map takes the next
ordinal-scale color, which can be exactly the color that a *shared* label of
the same chart resolves to moments later. The repair block added in #39297 then
reassigns the earlier label in `chartLabelsColorMap` and the shared map — but
the color had already been returned to the chart for this render, so the frame
still paints two identical colors. Hence the dependence on chart load order and
on the shared map being incomplete.
The fix adds `getReservedColors(label)`: colors owned by *other* labels, via
`forcedColors` always plus the shared `LabelsColorMap` when the source is a
dashboard. A brand-new label now avoids reserved colors as well as chart-used
ones, and `getNextAvailableColor` takes an optional reserved set it prefers to
avoid when picking an unused color.
Invariants kept: forced colors (custom label colors) still always win;
reservation is a *preference*, so once the palette is exhausted the previous
least-used/adjacency fallback applies unchanged and no chart is pushed into
analogous colors that wasn't already. Only an optional third parameter was
added — no public API shape changed.
**Trade-off worth a maintainer's opinion:** reserving shared-map colors
makes label→color assignment effectively dashboard-wide unique while the
palette lasts. Previously every chart restarted at the head of the palette, so
two charts with entirely disjoint labels both used colors 1..n; now the second
chart starts after the colors the first one claimed. Same-label-same-color
across charts is unaffected. This also makes live rendering agree with the
`map_label_colors` written on dashboard save, since
`LabelsColorMap.updateColorMap` goes through the same `getColor`. If per-chart
palette restart is considered deliberate, the reservation would have to be
scoped to the chart's own label set — which is not knowable on a first
dashboard load, as `chartsLabelsMap` is only populated by `addSlice` during
render, and so would not fix this bug.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: in the reporter's dashboard, "Call of duty Sales" renders PC and
Playstation in the same light blue.
After: every label in the chart gets a distinct color.
### TESTING INSTRUCTIONS
```bash
cd superset-frontend
npm run test -- packages/superset-ui-core/test/color
```
139 tests pass, including 4 new cases naming #36406: incomplete shared map,
empty shared map, forced colors still winning, and a new label avoiding
forced-color colors.
One existing expectation in `LabelsColorMapSingleton.test.ts` was updated
for the intended behavior change (label `c` in slice 2 now gets `blue` rather
than duplicating `a`'s `yellow`) — that assertion was pinning the bug.
Manually: import the dashboard attached to #36406, confirm via
`/api/v1/dashboard/{pk}` that `json_metadata` has no `map_label_colors`, open
the dashboard and reload several times.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #36406
- [ ] 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
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
