rusackas commented on code in PR #42910:
URL: https://github.com/apache/superset/pull/42910#discussion_r4297916779
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/controlPanel.tsx:
##########
@@ -59,6 +62,28 @@ const config: ControlPanelConfig = {
},
},
],
+ [
+ {
+ name: 'range_colors',
Review Comment:
Yep, that would have broken update_chart on any Bullet saved from Explore.
range_colors is now in the preserved key set so it survives updates, and it's
dropped from the typed validation copy so it isn't rejected as an unknown field.
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1196,6 +1197,12 @@ def merge_gauge_update_form_data( # noqa: C901
patch.pop(MCP_DASHBOARD_TIME_FILTER_SUBJECT, None)
merged.update(patch)
+ if (
+ "interval_color_indices" in fields_set
+ and "interval_colors" in existing_form_data
+ ):
+ # Explicit palette indices supersede saved literal interval colors.
+ merged.pop("interval_colors", None)
Review Comment:
Right, the same-dataset update goes through the overlay path, so the pop
never ran there. I applied the same rule in the Gauge plugin's
finalize_update_form_data, so explicit interval_color_indices now drop saved
interval_colors on both paths. Added a test for the same-dataset case.
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -1185,9 +1185,22 @@ def numeric_bound(field_name: str) -> float | None:
}
)
- colors = [
- _GAUGE_COLORS[(int(index) - 1) % len(_GAUGE_COLORS)] for index in
color_indices
- ] or list(_GAUGE_COLORS[: max(1, len(interval_bounds))])
+ explicit_colors = form_data.get("interval_colors")
+ if not isinstance(explicit_colors, list):
+ explicit_colors = []
+ colors = []
+ for position in range(max(1, len(interval_bounds))):
+ # Explicit per-interval colors win; blanks fall back to the legacy
+ # palette index for that band, else the palette cycled by position.
+ custom = explicit_colors[position] if position < len(explicit_colors)
else None
+ if isinstance(custom, str) and custom:
+ colors.append(custom)
+ elif position < len(color_indices):
Review Comment:
Agreed, that diverged from the renderer. Once explicit interval_colors are
saved the preview now ignores the legacy indices and a blank entry uses the
positional palette color, same as Gauge/transformProps. Added a test for your
50,100 / 1,4 example.
--
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]
