iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(Dashboard): Ensure shared label colors are updated (#31031)

This commit is contained in:
Geido 2024-11-23 16:39:40 +02:00 committed by GitHub
parent 67ad7da5cc
commit 91301bcd5b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 65 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
superset-frontend/src/dashboard/actions/dashboardState.js
View File

@ -28,6 +28,7 @@ import {
t,
getClientErrorObject,
getCategoricalSchemeRegistry,
promiseTimeout,
} from '@superset-ui/core';
import {
addChart,
@ -737,9 +738,9 @@ export const persistDashboardLabelsColor = () => async (dispatch, getState) => {
} = getState();
if (labelsColorMapMustSync || sharedLabelsColorsMustSync) {
storeDashboardMetadata(id, metadata);
dispatch(setDashboardLabelsColorMapSynced());
dispatch(setDashboardSharedLabelsColorsSynced());
storeDashboardMetadata(id, metadata);
}
};
@ -755,16 +756,13 @@ export const applyDashboardLabelsColorOnLoad = metadata => async dispatch => {
try {
const updatedMetadata = { ...metadata };
const customLabelsColor = metadata.label_colors || {};
const sharedLabelsColor = metadata.shared_label_colors || [];
let hasChanged = false;
// backward compatibility of shared_label_colors
const sharedLabels = metadata.shared_label_colors || [];
if (!Array.isArray(sharedLabels) && Object.keys(sharedLabels).length > 0) {
hasChanged = true;
updatedMetadata.shared_label_colors = getFreshSharedLabels(
Object.keys(sharedLabelsColor),
);
updatedMetadata.shared_label_colors = [];
}
// backward compatibility of map_label_colors
const hasMapLabelColors =
@ -828,27 +826,28 @@ export const applyDashboardLabelsColorOnLoad = metadata => async dispatch => {
* @returns void
*/
export const ensureSyncedLabelsColorMap = metadata => (dispatch, getState) => {
const {
dashboardState: { labelsColorMapMustSync },
} = getState();
const updatedMetadata = { ...metadata };
const customLabelsColor = metadata.label_colors || {};
const isMapSynced = isLabelsColorMapSynced(metadata);
const mustSync = !isMapSynced;
const syncLabelsColorMap = () => {
const {
dashboardState: { labelsColorMapMustSync },
} = getState();
const updatedMetadata = { ...metadata };
const customLabelsColor = metadata.label_colors || {};
const isMapSynced = isLabelsColorMapSynced(metadata);
const mustSync = !isMapSynced;
if (mustSync) {
const freshestColorMapEntries = getLabelsColorMapEntries(customLabelsColor);
updatedMetadata.map_label_colors = freshestColorMapEntries;
dispatch(setDashboardMetadata(updatedMetadata));
}
if (mustSync) {
const freshestColorMapEntries =
getLabelsColorMapEntries(customLabelsColor);
updatedMetadata.map_label_colors = freshestColorMapEntries;
dispatch(setDashboardMetadata(updatedMetadata));
}
if (mustSync && !labelsColorMapMustSync) {
// prepare to persist the just applied labels color map
dispatch(setDashboardLabelsColorMapSync());
}
if (!mustSync && labelsColorMapMustSync) {
dispatch(setDashboardLabelsColorMapSynced());
}
if (mustSync && !labelsColorMapMustSync) {
// prepare to persist the just applied labels color map
dispatch(setDashboardLabelsColorMapSync());
}
};
promiseTimeout(syncLabelsColorMap, 500);
};
/**
@ -856,18 +855,21 @@ export const ensureSyncedLabelsColorMap = metadata => (dispatch, getState) => {
* Ensure that the stored shared labels colors match current.
*
* @param {*} metadata - the dashboard metadata
* @param {*} forceFresh - when true it will use the fresh shared labels ignoring stored ones
* @returns void
*/
export const ensureSyncedSharedLabelsColors =
metadata => (dispatch, getState) => {
// using a timeout to let the rendered charts finish processing labels
setTimeout(() => {
(metadata, forceFresh = false) =>
(dispatch, getState) => {
const syncSharedLabelsColors = () => {
const {
dashboardState: { sharedLabelsColorsMustSync },
} = getState();
const updatedMetadata = { ...metadata };
const sharedLabelsColors = metadata.shared_label_colors || [];
const freshLabelsColors = getFreshSharedLabels(sharedLabelsColors);
const freshLabelsColors = getFreshSharedLabels(
forceFresh ? [] : sharedLabelsColors,
);
const isSharedLabelsColorsSynced = isEqual(
sharedLabelsColors,
freshLabelsColors,
@ -884,10 +886,8 @@ export const ensureSyncedSharedLabelsColors =
// prepare to persist the shared labels colors
dispatch(setDashboardSharedLabelsColorsSync());
}
if (!mustSync && sharedLabelsColorsMustSync) {
dispatch(setDashboardSharedLabelsColorsSynced());
}
}, 500);
};
promiseTimeout(syncSharedLabelsColors, 500);
};
/**
@ -909,7 +909,6 @@ export const updateDashboardLabelsColor =
const fullLabelsColors = metadata.map_label_colors || {};
const sharedLabelsColors = metadata.shared_label_colors || [];
const customLabelsColors = metadata.label_colors || {};
const updatedMetadata = { ...metadata };
// for dashboards with no color scheme, the charts should always use their individual schemes
// this logic looks for unique labels (not shared across multiple charts) of each rendered chart
@ -965,11 +964,7 @@ export const updateDashboardLabelsColor =
const shouldGoFresh = shouldReset.length > 0 ? shouldReset : false;
const shouldMerge = !shouldGoFresh;
// re-apply the color map first to get fresh maps accordingly
applyColors(updatedMetadata, shouldGoFresh, shouldMerge);
// new data may have appeared in the map (data changes)
// or new slices may have appeared while changing tabs
dispatch(ensureSyncedLabelsColorMap(updatedMetadata));
dispatch(ensureSyncedSharedLabelsColors(updatedMetadata));
applyColors(metadata, shouldGoFresh, shouldMerge);
} catch (e) {
console.error('Failed to update colors for new charts and labels:', e);
}

39
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
View File

@ -47,6 +47,8 @@ import {
applyDashboardLabelsColorOnLoad,
updateDashboardLabelsColor,
persistDashboardLabelsColor,
ensureSyncedSharedLabelsColors,
ensureSyncedLabelsColorMap,
} from 'src/dashboard/actions/dashboardState';
import { getColorNamespace, resetColors } from 'src/utils/colorScheme';
import { NATIVE_FILTER_DIVIDER_PREFIX } from '../nativeFilters/FiltersConfigModal/utils';
@ -96,7 +98,6 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
const [dashboardLabelsColorInitiated, setDashboardLabelsColorInitiated] =
useState(false);
const prevRenderedChartIds = useRef<number[]>([]);
const prevTabIndexRef = useRef();
const tabIndex = useMemo(() => {
const nextTabIndex = findTabIndexByComponentId({
@ -110,6 +111,18 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
prevTabIndexRef.current = nextTabIndex;
return nextTabIndex;
}, [dashboardLayout, directPathToChild]);
// when all charts have rendered, enforce fresh shared labels
const shouldForceFreshSharedLabelsColors =
dashboardLabelsColorInitiated &&
renderedChartIds.length > 0 &&
chartIds.length === renderedChartIds.length &&
prevRenderedChartIds.current.length < renderedChartIds.length;
const onBeforeUnload = useCallback(() => {
dispatch(persistDashboardLabelsColor());
resetColors(getColorNamespace(dashboardInfo?.metadata?.color_namespace));
prevRenderedChartIds.current = [];
}, [dashboardInfo?.metadata?.color_namespace, dispatch]);
useEffect(() => {
if (nativeFilterScopes.length === 0) {
@ -148,11 +161,12 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
const activeKey = min === 0 ? DASHBOARD_GRID_ID : min.toString();
const TOP_OF_PAGE_RANGE = 220;
const onBeforeUnload = useCallback(() => {
dispatch(persistDashboardLabelsColor());
resetColors(getColorNamespace(dashboardInfo?.metadata?.color_namespace));
prevRenderedChartIds.current = [];
}, [dashboardInfo?.metadata?.color_namespace, dispatch]);
useEffect(() => {
if (shouldForceFreshSharedLabelsColors) {
// all available charts have rendered, enforce freshest shared label colors
dispatch(ensureSyncedSharedLabelsColors(dashboardInfo.metadata, true));
}
}, [dashboardInfo.metadata, dispatch, shouldForceFreshSharedLabelsColors]);
useEffect(() => {
// verify freshness of color map
@ -161,7 +175,6 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
if (
dashboardLabelsColorInitiated &&
dashboardInfo?.id &&
numRenderedCharts > 0 &&
prevRenderedChartIds.current.length < numRenderedCharts
) {
@ -170,12 +183,20 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
);
prevRenderedChartIds.current = renderedChartIds;
dispatch(updateDashboardLabelsColor(newRenderedChartIds));
// new data may have appeared in the map (data changes)
// or new slices may have appeared while changing tabs
dispatch(ensureSyncedLabelsColorMap(dashboardInfo.metadata));
if (!shouldForceFreshSharedLabelsColors) {
dispatch(ensureSyncedSharedLabelsColors(dashboardInfo.metadata));
}
}
}, [
dashboardInfo?.id,
renderedChartIds,
dispatch,
dashboardLabelsColorInitiated,
dashboardInfo.metadata,
shouldForceFreshSharedLabelsColors,
]);
useEffect(() => {
@ -183,9 +204,9 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
labelsColorMap.source = LabelsColorMapSource.Dashboard;
if (dashboardInfo?.id && !dashboardLabelsColorInitiated) {
dispatch(applyDashboardLabelsColorOnLoad(dashboardInfo.metadata));
// apply labels color as dictated by stored metadata (if any)
setDashboardLabelsColorInitiated(true);
dispatch(applyDashboardLabelsColorOnLoad(dashboardInfo.metadata));
}
return () => {

3
superset-frontend/src/utils/colorScheme.ts
View File

@ -35,6 +35,7 @@ export const getColorNamespace = (namespace?: string) => namespace || undefined;
* Get labels shared across all charts in a dashboard.
* Merges a fresh instance of shared label colors with a stored one.
*
* @param currentSharedLabels - existing shared labels to merge with fresh
* @returns Record<string, string>
*/
export const getFreshSharedLabels = (
@ -74,7 +75,7 @@ export const getSharedLabelsColorMapEntries = (
* @returns all color entries except custom label colors
*/
export const getLabelsColorMapEntries = (
customLabelsColor: Record<string, string>,
customLabelsColor: Record<string, string> = {},
): Record<string, string> => {
const labelsColorMapInstance = getLabelsColorMap();
const allEntries = Object.fromEntries(labelsColorMapInstance.getColorMap());