diff --git a/superset-frontend/packages/superset-ui-core/src/color/ColorSchemeRegistry.ts b/superset-frontend/packages/superset-ui-core/src/color/ColorSchemeRegistry.ts index d36b598bc..e270ff6f0 100644 --- a/superset-frontend/packages/superset-ui-core/src/color/ColorSchemeRegistry.ts +++ b/superset-frontend/packages/superset-ui-core/src/color/ColorSchemeRegistry.ts @@ -28,7 +28,16 @@ export default class ColorSchemeRegistry extends RegistryWithDefaultKey { }); } - get(key?: string) { - return super.get(key) as T | undefined; + get(key?: string, strict = false) { + const target = super.get(key) as T | undefined; + + // fallsback to default scheme if any + if (!strict && !target) { + const defaultKey = super.getDefaultKey(); + if (defaultKey) { + return super.get(defaultKey) as T | undefined; + } + } + return target; } } diff --git a/superset-frontend/packages/superset-ui-core/test/color/ColorSchemeRegistry.test.ts b/superset-frontend/packages/superset-ui-core/test/color/ColorSchemeRegistry.test.ts index 462982847..13aa49922 100644 --- a/superset-frontend/packages/superset-ui-core/test/color/ColorSchemeRegistry.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/color/ColorSchemeRegistry.test.ts @@ -18,10 +18,26 @@ */ import ColorSchemeRegistry from '../../src/color/ColorSchemeRegistry'; +import schemes from '../../src/color/colorSchemes/categorical/d3'; +import CategoricalScheme from '../../src/color/CategoricalScheme'; describe('ColorSchemeRegistry', () => { it('exists', () => { expect(ColorSchemeRegistry).toBeDefined(); expect(ColorSchemeRegistry).toBeInstanceOf(Function); }); + it('returns undefined', () => { + const registry = new ColorSchemeRegistry(); + expect(registry.get('something')).toBeUndefined(); + }); + it('returns default', () => { + const registry = new ColorSchemeRegistry(); + registry.registerValue('SUPERSET_DEFAULT', schemes[0]); + expect(registry.get('something')).toBeInstanceOf(CategoricalScheme); + }); + it('returns undefined in strict mode', () => { + const registry = new ColorSchemeRegistry(); + registry.registerValue('SUPERSET_DEFAULT', schemes[0]); + expect(registry.get('something', true)).toBeUndefined(); + }); }); diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 1b9224a3d..c649305dd 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -240,6 +240,7 @@ export function saveDashboardRequest(data, id, saveType) { ...data.metadata, color_namespace: data.metadata?.color_namespace || undefined, color_scheme: data.metadata?.color_scheme || '', + color_scheme_domain: data.metadata?.color_scheme_domain || [], expanded_slices: data.metadata?.expanded_slices || {}, label_colors: data.metadata?.label_colors || {}, shared_label_colors: data.metadata?.shared_label_colors || {}, diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index c763f0726..a98a419ac 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -18,13 +18,15 @@ */ // ParentSize uses resize observer so the dashboard will update size // when its container size changes, due to e.g., builder side panel opening -import React, { FC, useEffect, useMemo } from 'react'; +import React, { FC, useCallback, useEffect, useMemo } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { FeatureFlag, Filter, Filters, + getCategoricalSchemeRegistry, isFeatureEnabled, + SupersetClient, } from '@superset-ui/core'; import { ParentSize } from '@vx/responsive'; import pick from 'lodash/pick'; @@ -32,6 +34,7 @@ import Tabs from 'src/components/Tabs'; import DashboardGrid from 'src/dashboard/containers/DashboardGrid'; import { ChartsState, + DashboardInfo, DashboardLayout, LayoutItem, RootState, @@ -43,9 +46,13 @@ import { import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope'; import findTabIndexByComponentId from 'src/dashboard/util/findTabIndexByComponentId'; import { setInScopeStatusOfFilters } from 'src/dashboard/actions/nativeFilters'; -import { getRootLevelTabIndex, getRootLevelTabsComponent } from './utils'; -import { findTabsWithChartsInScope } from '../nativeFilters/utils'; +import { dashboardInfoChanged } from 'src/dashboard/actions/dashboardInfo'; +import { setColorScheme } from 'src/dashboard/actions/dashboardState'; +import { useComponentDidUpdate } from 'src/hooks/useComponentDidUpdate/useComponentDidUpdate'; +import jsonStringify from 'json-stringify-pretty-compact'; import { NATIVE_FILTER_DIVIDER_PREFIX } from '../nativeFilters/FiltersConfigModal/utils'; +import { findTabsWithChartsInScope } from '../nativeFilters/utils'; +import { getRootLevelTabIndex, getRootLevelTabsComponent } from './utils'; type DashboardContainerProps = { topLevelTabs?: LayoutItem; @@ -73,6 +80,9 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { const dashboardLayout = useSelector( state => state.dashboardLayout.present, ); + const dashboardInfo = useSelector( + state => state.dashboardInfo, + ); const directPathToChild = useSelector( state => state.dashboardState.directPathToChild, ); @@ -122,10 +132,88 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { dispatch(setInScopeStatusOfFilters(scopes)); }, [nativeFilterScopes, dashboardLayout, dispatch]); + const verifyUpdateColorScheme = useCallback(() => { + const currentMetadata = dashboardInfo.metadata; + if (currentMetadata?.color_scheme) { + const metadata = { ...currentMetadata }; + const colorScheme = metadata?.color_scheme; + const colorSchemeDomain = metadata?.color_scheme_domain || []; + const categoricalSchemes = getCategoricalSchemeRegistry(); + const registryColorScheme = + categoricalSchemes.get(colorScheme, true) || undefined; + const registryColorSchemeDomain = registryColorScheme?.colors || []; + const defaultColorScheme = categoricalSchemes.defaultKey; + const colorSchemeExists = !!registryColorScheme; + + const updateDashboardData = () => { + SupersetClient.put({ + endpoint: `/api/v1/dashboard/${dashboardInfo.id}`, + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + json_metadata: jsonStringify(metadata), + }), + }).catch(e => console.log(e)); + }; + const updateColorScheme = (scheme: string) => { + dispatch(setColorScheme(scheme)); + }; + const updateDashboard = () => { + dispatch( + dashboardInfoChanged({ + metadata, + }), + ); + updateDashboardData(); + }; + // selected color scheme does not exist anymore + // must fallback to the available default one + if (!colorSchemeExists) { + const updatedScheme = + defaultColorScheme?.toString() || 'supersetColors'; + metadata.color_scheme = updatedScheme; + metadata.color_scheme_domain = + categoricalSchemes.get(defaultColorScheme)?.colors || []; + + // reset shared_label_colors + // TODO: Requires regenerating the shared_label_colors after + // fixing a bug which affects their generation on dashboards with tabs + metadata.shared_label_colors = {}; + + updateColorScheme(updatedScheme); + updateDashboard(); + } else { + // if this dashboard does not have a color_scheme_domain saved + // must create one and store it for the first time + if (colorSchemeExists && !colorSchemeDomain.length) { + metadata.color_scheme_domain = registryColorSchemeDomain; + updateDashboard(); + } + // if the color_scheme_domain is not the same as the registry domain + // must update the existing color_scheme_domain + if ( + colorSchemeExists && + colorSchemeDomain.length && + registryColorSchemeDomain.toString() !== colorSchemeDomain.toString() + ) { + metadata.color_scheme_domain = registryColorSchemeDomain; + + // reset shared_label_colors + // TODO: Requires regenerating the shared_label_colors after + // fixing a bug which affects their generation on dashboards with tabs + metadata.shared_label_colors = {}; + + updateColorScheme(colorScheme); + updateDashboard(); + } + } + } + }, [charts]); + + useComponentDidUpdate(verifyUpdateColorScheme); + const childIds: string[] = topLevelTabs ? topLevelTabs.children : [DASHBOARD_GRID_ID]; - const min = Math.min(tabIndex, childIds.length - 1); const activeKey = min === 0 ? DASHBOARD_GRID_ID : min.toString(); diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx index 07d48eb98..bffe9f3de 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx @@ -262,6 +262,7 @@ test('submitting with onlyApply:false', async () => { ); spyGetCategoricalSchemeRegistry.mockReturnValue({ keys: () => ['supersetColors'], + get: () => ['#FFFFFF', '#000000'], } as any); put.mockResolvedValue({ json: { @@ -289,7 +290,6 @@ test('submitting with onlyApply:false', async () => { userEvent.click(screen.getByRole('button', { name: 'Save' })); await waitFor(() => { - expect(props.onHide).toBeCalledTimes(1); expect(props.onSubmit).toBeCalledTimes(1); expect(props.onSubmit).toBeCalledWith({ certificationDetails: 'Sample certification', @@ -312,6 +312,7 @@ test('submitting with onlyApply:true', async () => { ); spyGetCategoricalSchemeRegistry.mockReturnValue({ keys: () => ['supersetColors'], + get: () => ['#FFFFFF', '#000000'], } as any); spyIsFeatureEnabled.mockReturnValue(false); const props = createProps(); @@ -328,7 +329,6 @@ test('submitting with onlyApply:true', async () => { userEvent.click(screen.getByRole('button', { name: 'Apply' })); await waitFor(() => { - expect(props.onHide).toBeCalledTimes(1); expect(props.onSubmit).toBeCalledTimes(1); }); }); diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx index 7a80f6155..ebdc75322 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx @@ -100,6 +100,7 @@ const PropertiesModal = ({ const [owners, setOwners] = useState([]); const [roles, setRoles] = useState([]); const saveLabel = onlyApply ? t('Apply') : t('Save'); + const categoricalSchemeRegistry = getCategoricalSchemeRegistry(); const handleErrorResponse = async (response: Response) => { const { error, statusText, message } = await getClientErrorObject(response); @@ -176,9 +177,13 @@ const PropertiesModal = ({ delete metadata.positions; } const metaDataCopy = { ...metadata }; + if (metaDataCopy?.shared_label_colors) { delete metaDataCopy.shared_label_colors; } + + delete metaDataCopy.color_scheme_domain; + setJsonMetadata(metaDataCopy ? jsonStringify(metaDataCopy) : ''); }, [form], @@ -264,7 +269,7 @@ const PropertiesModal = ({ { updateMetadata = true } = {}, ) => { // check that color_scheme is valid - const colorChoices = getCategoricalSchemeRegistry().keys(); + const colorChoices = categoricalSchemeRegistry.keys(); const jsonMetadataObj = getJsonMetadata(); // only fire if the color_scheme is present and invalid @@ -309,12 +314,25 @@ const PropertiesModal = ({ if (metadata?.shared_label_colors) { delete metadata.shared_label_colors; } + if (metadata?.color_scheme_domain) { + delete metadata.color_scheme_domain; + } + const colorMap = getSharedLabelColor().getColorMap( colorNamespace, currentColorScheme, true, ); + metadata.shared_label_colors = colorMap; + + if (metadata?.color_scheme) { + metadata.color_scheme_domain = + categoricalSchemeRegistry.get(colorScheme)?.colors || []; + } else { + metadata.color_scheme_domain = []; + } + currentJsonMetadata = jsonStringify(metadata); } diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index 52d110599..b2617ea77 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -81,6 +81,9 @@ export type DashboardInfo = { native_filter_configuration: JsonObject; show_native_filters: boolean; chart_configuration: JsonObject; + color_scheme: string; + color_namespace: string; + color_scheme_domain: string[]; label_colors: JsonObject; shared_label_colors: JsonObject; }; diff --git a/superset-frontend/src/explore/actions/hydrateExplore.ts b/superset-frontend/src/explore/actions/hydrateExplore.ts index 2479a8c97..0188337ab 100644 --- a/superset-frontend/src/explore/actions/hydrateExplore.ts +++ b/superset-frontend/src/explore/actions/hydrateExplore.ts @@ -26,7 +26,11 @@ import { import { getChartKey } from 'src/explore/exploreUtils'; import { getControlsState } from 'src/explore/store'; import { Dispatch } from 'redux'; -import { ensureIsArray } from '@superset-ui/core'; +import { + ensureIsArray, + getCategoricalSchemeRegistry, + getSequentialSchemeRegistry, +} from '@superset-ui/core'; import { getFormDataFromControls, applyMapStateToPropsToControl, @@ -36,6 +40,11 @@ import { getUrlParam } from 'src/utils/urlUtils'; import { URL_PARAMS } from 'src/constants'; import { findPermission } from 'src/utils/findPermission'; +enum ColorSchemeType { + CATEGORICAL = 'CATEGORICAL', + SEQUENTIAL = 'SEQUENTIAL', +} + export const HYDRATE_EXPLORE = 'HYDRATE_EXPLORE'; export const hydrateExplore = ({ form_data, slice, dataset }: ExplorePageInitialData) => @@ -66,6 +75,31 @@ export const hydrateExplore = initialExploreState, initialFormData, ) as ControlStateMapping; + const colorSchemeKey = initialControls.color_scheme && 'color_scheme'; + const linearColorSchemeKey = + initialControls.linear_color_scheme && 'linear_color_scheme'; + // if the selected color scheme does not exist anymore + // fallbacks and selects the available default one + const verifyColorScheme = (type: ColorSchemeType) => { + const schemes = + type === 'CATEGORICAL' + ? getCategoricalSchemeRegistry() + : getSequentialSchemeRegistry(); + const key = + type === 'CATEGORICAL' ? colorSchemeKey : linearColorSchemeKey; + const registryDefaultScheme = schemes.defaultKey; + const defaultScheme = + type === 'CATEGORICAL' ? 'supersetColors' : 'superset_seq_1'; + const currentScheme = initialFormData[key]; + const colorSchemeExists = !!schemes.get(currentScheme, true); + + if (currentScheme && !colorSchemeExists) { + initialControls[key].value = registryDefaultScheme || defaultScheme; + } + }; + + if (colorSchemeKey) verifyColorScheme(ColorSchemeType.CATEGORICAL); + if (linearColorSchemeKey) verifyColorScheme(ColorSchemeType.SEQUENTIAL); const exploreState = { // note this will add `form_data` to state, diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index bc5006d58..36db7e341 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -266,7 +266,7 @@ class DashboardDAO(BaseDAO): md["color_scheme"] = data.get("color_scheme", "") md["label_colors"] = data.get("label_colors", {}) md["shared_label_colors"] = data.get("shared_label_colors", {}) - + md["color_scheme_domain"] = data.get("color_scheme_domain", []) dashboard.json_metadata = json.dumps(md) if commit: diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 0413c1672..1375f98dd 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -129,6 +129,7 @@ class DashboardJSONMetadataSchema(Schema): positions = fields.Dict(allow_none=True) label_colors = fields.Dict() shared_label_colors = fields.Dict() + color_scheme_domain = fields.List(fields.Str()) # used for v0 import/export import_time = fields.Integer() remote_id = fields.Integer() diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index a027dcffa..e969a319c 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -72,7 +72,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi "slug": "slug1_changed", "position_json": '{"b": "B"}', "css": "css_changed", - "json_metadata": '{"refresh_frequency": 30, "timed_refresh_immune_slices": [], "expanded_slices": {}, "color_scheme": "", "label_colors": {}, "shared_label_colors": {}}', + "json_metadata": '{"refresh_frequency": 30, "timed_refresh_immune_slices": [], "expanded_slices": {}, "color_scheme": "", "label_colors": {}, "shared_label_colors": {}, "color_scheme_domain": []}', "published": False, }