diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 63a7a9a73..c2026955b 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -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); } diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index 36dedb68c..f36520ba3 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -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 = ({ topLevelTabs }) => { const [dashboardLabelsColorInitiated, setDashboardLabelsColorInitiated] = useState(false); const prevRenderedChartIds = useRef([]); - const prevTabIndexRef = useRef(); const tabIndex = useMemo(() => { const nextTabIndex = findTabIndexByComponentId({ @@ -110,6 +111,18 @@ const DashboardContainer: FC = ({ 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 = ({ 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 = ({ topLevelTabs }) => { if ( dashboardLabelsColorInitiated && - dashboardInfo?.id && numRenderedCharts > 0 && prevRenderedChartIds.current.length < numRenderedCharts ) { @@ -170,12 +183,20 @@ const DashboardContainer: FC = ({ 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 = ({ 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 () => { diff --git a/superset-frontend/src/utils/colorScheme.ts b/superset-frontend/src/utils/colorScheme.ts index b617ad56b..7bafc20cf 100644 --- a/superset-frontend/src/utils/colorScheme.ts +++ b/superset-frontend/src/utils/colorScheme.ts @@ -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 */ export const getFreshSharedLabels = ( @@ -74,7 +75,7 @@ export const getSharedLabelsColorMapEntries = ( * @returns all color entries except custom label colors */ export const getLabelsColorMapEntries = ( - customLabelsColor: Record, + customLabelsColor: Record = {}, ): Record => { const labelsColorMapInstance = getLabelsColorMap(); const allEntries = Object.fromEntries(labelsColorMapInstance.getColorMap());