This is an automated email from the ASF dual-hosted git repository. diegopucci pushed a commit to branch geido/fix/color-consistency-race-conditions in repository https://gitbox.apache.org/repos/asf/superset.git
commit b8db3777b78c7258e2ef0266af365b171cfd5e6f Author: Diego Pucci <[email protected]> AuthorDate: Thu Oct 17 17:46:58 2024 +0300 fix(Dashboard): Retain colors when color scheme not set --- .../src/dashboard/actions/dashboardState.js | 7 ++-- .../dashboard/components/PropertiesModal/index.tsx | 11 ++++-- superset-frontend/src/utils/colorScheme.ts | 40 +++++++++++++++++----- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 945dda32a3..10fcf518cb 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -716,10 +716,12 @@ export const updateDashboardLabelsColor = () => async (dispatch, getState) => { } // stored labels color map and applied might differ + // new data may appear in the map (data changes) + // or new slices may appear while changing tabs const isMapSynced = isLabelsColorMapSynced(metadata); if (!isMapSynced) { // re-apply a fresh labels color map - applyColors(updatedMetadata, true); + applyColors(updatedMetadata, false, true); // pull and store the just applied labels color map updatedMetadata.shared_label_colors = getLabelsColorMapEntries(); } @@ -727,7 +729,8 @@ export const updateDashboardLabelsColor = () => async (dispatch, getState) => { // the stored color domain registry and fresh might differ at this point const freshColorSchemeDomain = getColorSchemeDomain(colorScheme); const isRegistrySynced = - colorSchemeDomain.toString() !== freshColorSchemeDomain.toString(); + colorSchemeDomain.toString() === freshColorSchemeDomain.toString(); + if (colorScheme && !isRegistrySynced) { updatedMetadata.color_scheme_domain = freshColorSchemeDomain; } diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx index 0bfa1706d5..b44957d71d 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx @@ -318,7 +318,6 @@ const PropertiesModal = ({ return; } - const copyMetadata = { ...metadata }; const colorNamespace = getColorNamespace(metadata?.color_namespace); // color scheme in json metadata has precedence over selection @@ -332,8 +331,14 @@ const PropertiesModal = ({ delete metadata.color_scheme_domain; } - // only apply colors, the user has not saved yet - applyColors(copyMetadata, true); + // only apply colors as the user has not saved yet + applyColors( + { + ...metadata, + color_scheme: currentColorScheme, + }, + true, + ); currentJsonMetadata = jsonStringify(metadata); diff --git a/superset-frontend/src/utils/colorScheme.ts b/superset-frontend/src/utils/colorScheme.ts index be4d3786e0..a3da1427c7 100644 --- a/superset-frontend/src/utils/colorScheme.ts +++ b/superset-frontend/src/utils/colorScheme.ts @@ -107,31 +107,53 @@ 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 = false, + merge = false, +) => { const colorNameSpace = getColorNamespace(metadata?.color_namespace); const categoricalNamespace = CategoricalColorNamespace.getNamespace(colorNameSpace); - const colorScheme = metadata?.color_scheme; + let colorScheme = metadata?.color_scheme; const customLabelColors = metadata?.label_colors || {}; // when scheme unset, update only custom label colors const labelsColorMap = metadata?.shared_label_colors || {}; - // reset forced colors (custom labels + labels color map) - categoricalNamespace.resetColors(); + if (!colorScheme) { + // a fallback color scheme must be set to generate shared label colors + const categoricalSchemes = getCategoricalSchemeRegistry(); + colorScheme = + categoricalSchemes.getDefaultKey()?.toString() ?? 'supersetColors'; + } + + if (fresh) { + // reset forced colors (custom labels + labels color map) + categoricalNamespace.resetColors(); + } // 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) { + // re-instantiate a fresh labels color map based on current scheme + // must consider also just applied customLabelColors + refreshLabelsColorMap(metadata?.color_namespace, colorScheme); + } + + const currentColorMapEntries = getLabelsColorMapEntries(); // get the fresh map that was just updated or existing const labelsColorMapEntries = fresh - ? getLabelsColorMapEntries() - : labelsColorMap; + ? currentColorMapEntries + : merge + ? { + ...currentColorMapEntries, + ...labelsColorMap, + } + : labelsColorMap; // apply the final color map Object.keys(labelsColorMapEntries).forEach(label => {
