From 4316a97e5b47efb6e82dc58aee2f7cf5a213d671 Mon Sep 17 00:00:00 2001 From: Deepak Jain Date: Tue, 22 Sep 2026 22:34:14 -0700 Subject: [PATCH 1/3] fix(dashboard): initialize label colors before mounting charts Signed-off-by: Deepak Jain --- .../DashboardContainer.test.tsx | 131 +++++++++++++++++- .../DashboardBuilder/DashboardContainer.tsx | 11 +- 2 files changed, 135 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx index d941b9f23248..4d318f53dc6b 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx @@ -16,12 +16,20 @@ * specific language governing permissions and limitations * under the License. */ -import { render, screen, waitFor } from 'spec/helpers/testing-library'; +import { StrictMode, useState } from 'react'; +import { act, render, screen, waitFor } from 'spec/helpers/testing-library'; import fetchMock from 'fetch-mock'; import { storeWithState } from 'spec/fixtures/mockStore'; import mockState from 'spec/fixtures/mockState'; import { sliceId } from 'spec/fixtures/mockChartQueries'; -import { ChartCustomizationType, NativeFilterType } from '@superset-ui/core'; +import { + ChartCustomizationType, + NativeFilterType, + CategoricalColorNamespace, +} from '@superset-ui/core'; +import DashboardGrid from 'src/dashboard/containers/DashboardGrid'; +import { applyDashboardLabelsColorOnLoad } from 'src/dashboard/actions/dashboardState'; +import { dashboardInfoChanged } from 'src/dashboard/actions/dashboardInfo'; import { CHART_TYPE } from '../../util/componentTypes'; import DashboardContainer from './DashboardContainer'; import * as nativeFiltersActions from '../../actions/nativeFilters'; @@ -37,7 +45,7 @@ jest.mock('@visx/responsive', () => ({ jest.mock('src/dashboard/containers/DashboardGrid', () => ({ __esModule: true, - default: () =>
, + default: jest.fn(() =>
), })); // DashboardContainer dispatches these on mount, so unit tests stub them. @@ -154,6 +162,12 @@ beforeEach(() => { afterEach(() => { setInScopeStatusMock.mockRestore(); + jest + .mocked(applyDashboardLabelsColorOnLoad) + .mockImplementation(() => async () => {}); + jest + .mocked(DashboardGrid) + .mockImplementation(() =>
); }); test('calculates chartsInScope correctly for filters', async () => { @@ -853,3 +867,114 @@ test('does not dispatch setInScopeStatusOfCustomizations when chart_customizatio spy.mockRestore(); } }); + +test.each([false, true])( + 'applies custom label colors before a cached chart consumes its first color scale (StrictMode=%s)', + strict => { + const namespace = 'initial-label-colors'; + CategoricalColorNamespace.getNamespace(namespace).resetColors(); + jest + .mocked(applyDashboardLabelsColorOnLoad) + .mockImplementation( + jest.requireActual('src/dashboard/actions/dashboardState') + .applyDashboardLabelsColorOnLoad, + ); + jest.mocked(DashboardGrid).mockImplementation(function CachedChart() { + const [color] = useState(() => + CategoricalColorNamespace.getScale(undefined, namespace).getColor( + '20_Passed', + ), + ); + return
; + }); + + const initialState = createTestState({ + dashboardInfo: { + ...mockState.dashboardInfo, + metadata: { + ...mockState.dashboardInfo.metadata, + color_namespace: namespace, + color_scheme: '', + label_colors: { '20_Passed': '#008000' }, + }, + }, + }); + + const { unmount } = render( + strict ? ( + + + + ) : ( + + ), + { useRedux: true, store: storeWithState(initialState) }, + ); + + expect(screen.getByTestId('cached-chart')).toHaveAttribute( + 'data-color', + '#008000', + ); + unmount(); + CategoricalColorNamespace.getNamespace(namespace).resetColors(); + }, +); + +test.each([false, true])( + 'initializes the next dashboard colors before mounting its cached charts (new namespace=%s)', + newNamespace => { + const namespace = 'navigation-label-colors'; + let chartNamespace = namespace; + CategoricalColorNamespace.getNamespace(namespace).resetColors(); + jest + .mocked(applyDashboardLabelsColorOnLoad) + .mockImplementation( + jest.requireActual('src/dashboard/actions/dashboardState') + .applyDashboardLabelsColorOnLoad, + ); + jest.mocked(DashboardGrid).mockImplementation(function CachedChart() { + const [color] = useState(() => + CategoricalColorNamespace.getScale(undefined, chartNamespace).getColor( + '20_Passed', + ), + ); + return
; + }); + const metadata = { + ...mockState.dashboardInfo.metadata, + color_namespace: namespace, + color_scheme: '', + label_colors: { '20_Passed': '#008000' }, + }; + const { store, unmount } = setupWithStore({ + dashboardInfo: { ...mockState.dashboardInfo, metadata }, + }); + expect(screen.getByTestId('cached-chart')).toHaveAttribute( + 'data-color', + '#008000', + ); + + chartNamespace = newNamespace ? 'next-dashboard-colors' : namespace; + act(() => { + store.dispatch( + dashboardInfoChanged({ + id: mockState.dashboardInfo.id + 1, + metadata: { + ...store.getState().dashboardInfo.metadata, + ...metadata, + color_namespace: chartNamespace, + label_colors: { '20_Passed': '#ff0000' }, + }, + }), + ); + }); + + expect(screen.getByTestId('cached-chart')).toHaveAttribute( + 'data-color', + '#ff0000', + ); + unmount(); + CategoricalColorNamespace.getNamespace(namespace).resetColors(); + CategoricalColorNamespace.getNamespace(chartNamespace).resetColors(); + }, +); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index 97f3549c160a..91ae48ca6829 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -164,8 +164,10 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { const renderedChartIds = useRenderedChartIds(); - const [dashboardLabelsColorInitiated, setDashboardLabelsColorInitiated] = - useState(false); + const [colorInitializedDashboardId, setColorInitializedDashboardId] = + useState(); + const dashboardLabelsColorInitiated = + colorInitializedDashboardId === dashboardInfo?.id; const prevRenderedChartIds = useRef([]); const prevTabIndexRef = useRef(); const prevFilterScopesRef = useRef([]); @@ -304,7 +306,7 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { if (dashboardInfo?.id && !dashboardLabelsColorInitiated) { dispatch(applyDashboardLabelsColorOnLoad(dashboardInfo.metadata)); // apply labels color as dictated by stored metadata (if any) - setDashboardLabelsColorInitiated(true); + setColorInitializedDashboardId(dashboardInfo.id); } return () => { @@ -379,7 +381,8 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { return (
- {renderParentSizeChildren({ width })} + {/* Cached charts can consume their color scales on their first render. */} + {dashboardLabelsColorInitiated && renderParentSizeChildren({ width })}
); }; From 7cb92a820122367cbba35df6c669ada9ba3ddcad Mon Sep 17 00:00:00 2001 From: deepujain Date: Wed, 23 Sep 2026 13:42:07 +0000 Subject: [PATCH 2/3] chore: retrigger CI against regenerated messages.pot on master Upstream #44574 landed the messages.pot regeneration on master; the babel-extract failure on this PR was base drift, not contributor code. Fresh CI will run against the new merge commit with the fixed base pot. From 13bf6706af77576135b7f55d84ba0b9ec6f2462c Mon Sep 17 00:00:00 2001 From: Deepak Jain Date: Wed, 23 Sep 2026 14:10:47 -0700 Subject: [PATCH 3/3] fix(dashboard): defer chart mount until dashboard hydration Signed-off-by: Deepak Jain --- .../DashboardBuilder/DashboardContainer.test.tsx | 14 ++++++++++++++ .../DashboardBuilder/DashboardContainer.tsx | 5 +++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx index 4d318f53dc6b..caa490512d0a 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx @@ -978,3 +978,17 @@ test.each([false, true])( CategoricalColorNamespace.getNamespace(chartNamespace).resetColors(); }, ); + +test('waits for dashboard hydration before mounting cached charts', () => { + jest.mocked(applyDashboardLabelsColorOnLoad).mockClear(); + const { store } = setupWithStore({ dashboardInfo: {} }); + expect(screen.queryByTestId('mock-dashboard-grid')).not.toBeInTheDocument(); + expect(applyDashboardLabelsColorOnLoad).not.toHaveBeenCalled(); + + act(() => { + store.dispatch(dashboardInfoChanged({ id: mockState.dashboardInfo.id })); + }); + + expect(applyDashboardLabelsColorOnLoad).toHaveBeenCalled(); + expect(screen.getByTestId('mock-dashboard-grid')).toBeInTheDocument(); +}); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index 91ae48ca6829..335c2d391b08 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -165,7 +165,7 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { const renderedChartIds = useRenderedChartIds(); const [colorInitializedDashboardId, setColorInitializedDashboardId] = - useState(); + useState(null); const dashboardLabelsColorInitiated = colorInitializedDashboardId === dashboardInfo?.id; const prevRenderedChartIds = useRef([]); @@ -381,7 +381,8 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { return (
- {/* Cached charts can consume their color scales on their first render. */} + {/* Defer the grid until hydration and color initialization complete, + before cached charts consume their color scales on first render. */} {dashboardLabelsColorInitiated && renderParentSizeChildren({ width })}
);