kgabryje commented on code in PR #30646:
URL: https://github.com/apache/superset/pull/30646#discussion_r1832799799
##########
superset-frontend/src/utils/colorScheme.ts:
##########
@@ -107,31 +140,67 @@ export const refreshLabelsColorMap = (
*
* @param metadata - the dashboard metadata object
*/
-export const applyColors = (metadata: Record<string, any>, fresh = false) => {
+export const applyColors = (
+ metadata: Record<string, any>,
+ // fresh is used when changing / removing a color scheme
+ fresh = false,
+ // merge is used to catch new labels in the color map
+ merge = false,
+ // shared is used to apply only shared labels colors from a dashboard
+ // used in Explore with a dashboard context without color scheme
+ // or when loading a dashboard with no color scheme set
+ shared = false,
+) => {
const colorNameSpace = getColorNamespace(metadata?.color_namespace);
const categoricalNamespace =
CategoricalColorNamespace.getNamespace(colorNameSpace);
const colorScheme = metadata?.color_scheme;
const customLabelColors = metadata?.label_colors || {};
- // when scheme unset, update only custom label colors
- const labelsColorMap = metadata?.shared_label_colors || {};
+ const sharedLabels = Array.isArray(metadata?.shared_label_colors)
+ ? metadata.shared_label_colors
+ : getSharedLabels();
+ const fullLabelsColor = metadata?.map_label_colors || {};
+ // filter the fullLabelsColor object to only shared labels
+ const sharedLabelsColors = Object.fromEntries(
+ Object.entries(fullLabelsColor).filter(([label]) =>
+ sharedLabels.includes(label),
+ ),
+ );
- // reset forced colors (custom labels + labels color map)
- categoricalNamespace.resetColors();
+ const labelsColorMap = shared
+ ? sharedLabelsColors
+ : metadata?.map_label_colors || {};
- // apply custom label colors first
+ if (fresh) {
+ // reset custom label colors
+ categoricalNamespace.resetColors();
+ }
+
+ // re-apply custom label colors first
Object.keys(customLabelColors).forEach(label => {
categoricalNamespace.setColor(label, customLabelColors[label]);
});
- // re-instantiate a fresh labels color map based on current scheme
- // will consider also just applied custom label colors
- refreshLabelsColorMap(metadata?.color_namespace, colorScheme);
+ if (fresh || merge) {
+ // re-instantiate a fresh labels color map based on current scheme
+ // must consider also just applied customLabelColors if present
+ // will merge with existing colorMap only new labels when merge is true
+ refreshLabelsColorMap(metadata?.color_namespace, colorScheme, merge);
+ }
+
+ const currentColorMapEntries = shared
+ ? getSharedLabelsColorMapEntries(fullLabelsColor)
+ : getLabelsColorMapEntries();
// get the fresh map that was just updated or existing
const labelsColorMapEntries = fresh
- ? getLabelsColorMapEntries()
- : labelsColorMap;
+ ? currentColorMapEntries
+ : merge
+ ? {
+ ...currentColorMapEntries,
+ ...labelsColorMap,
Review Comment:
referring to my previous comment about `merge` - here it seems like the new
colors override the existing ones, rather than the other way around. Is that
intended?
--
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]
