geido commented on a change in pull request #17792:
URL: https://github.com/apache/superset/pull/17792#discussion_r778917661
##########
File path: superset-frontend/src/dashboard/actions/dashboardInfo.ts
##########
@@ -23,6 +23,21 @@ import { ChartConfiguration, DashboardInfo } from
'../reducers/types';
export const DASHBOARD_INFO_UPDATED = 'DASHBOARD_INFO_UPDATED';
+export function updateColorSchema(
+ metadata: any,
Review comment:
```suggestion
metadata: Record<string, any>,
```
##########
File path: superset/dashboards/dao.py
##########
@@ -265,6 +265,7 @@ def set_dash_metadata( # pylint: disable=too-many-locals
md["refresh_frequency"] = data.get("refresh_frequency", 0)
md["color_scheme"] = data.get("color_scheme", "")
md["label_colors"] = data.get("label_colors", {})
+ md["shared_label_colors"] = data.get("shared_label_colors", {})
Review comment:
The `shared_label_colors` will be presented to the user in the advanced
settings of the Dashboard. I think we should hide them in the frontend so the
user never sees them if possible.
##########
File path: superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
##########
@@ -157,13 +157,26 @@ export class ExploreChartHeader extends
React.PureComponent {
if (dashboard && dashboard.json_metadata) {
// setting the chart to use the dashboard custom label colors if
any
- const labelColors =
- JSON.parse(dashboard.json_metadata).label_colors || {};
+ const metadata = JSON.parse(dashboard.json_metadata);
+ const sharedLabelColors = metadata.shared_label_colors || {};
+ const labelColors = metadata.label_colors || {};
+
const categoricalNamespace =
CategoricalColorNamespace.getNamespace();
+ Object.keys(sharedLabelColors).forEach(label => {
+ categoricalNamespace.setColor(
+ label,
+ sharedLabelColors[label],
+ metadata.color_scheme,
+ );
+ });
Object.keys(labelColors).forEach(label => {
- categoricalNamespace.setColor(label, labelColors[label]);
+ categoricalNamespace.setColor(
+ label,
+ labelColors[label],
+ metadata.color_scheme,
+ );
Review comment:
What do you think about merging `sharedLabelColors` and `labelColors` in
one map and then only loop through one map? Also, I think it would make sense
to change the name of `labelColors` with `customLabelColors` for clarity.
--
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: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
