diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx index d941b9f23248..caa490512d0a 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,128 @@ 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(); + }, +); + +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 97f3549c160a..335c2d391b08 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(null); + 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,9 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { return (
- {renderParentSizeChildren({ width })} + {/* Defer the grid until hydration and color initialization complete, + before cached charts consume their color scales on first render. */} + {dashboardLabelsColorInitiated && renderParentSizeChildren({ width })}
); };