diff --git a/superset-frontend/packages/superset-ui-core/src/components/Typography/Typography.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Typography/Typography.test.tsx index 3aa2bb05c05a..5382c87bdc95 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Typography/Typography.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Typography/Typography.test.tsx @@ -50,7 +50,7 @@ describe('Typography Component', () => { it('renders strong text', () => { render(Strong Text); - expect(screen.getByText('Strong Text')).toHaveStyle('font-weight: 500'); + expect(screen.getByText('Strong Text')).toHaveStyle('font-weight: 600'); }); it('renders underlined text', () => { diff --git a/superset-frontend/packages/superset-ui-core/src/theme/GlobalStyles.tsx b/superset-frontend/packages/superset-ui-core/src/theme/GlobalStyles.tsx index d9936471af7e..31cf1547f554 100644 --- a/superset-frontend/packages/superset-ui-core/src/theme/GlobalStyles.tsx +++ b/superset-frontend/packages/superset-ui-core/src/theme/GlobalStyles.tsx @@ -16,6 +16,18 @@ * specific language governing permissions and limitations * under the License. */ + +// @fontsource/* v5.1+ doesn't play nice with eslint-import plugin v2.31+ +/* eslint-disable import/extensions */ +import '@fontsource/inter/200.css'; +import '@fontsource/inter/400.css'; +import '@fontsource/inter/500.css'; +import '@fontsource/inter/600.css'; +import '@fontsource/fira-code/400.css'; +import '@fontsource/fira-code/500.css'; +import '@fontsource/fira-code/600.css'; +/* eslint-enable import/extensions */ + import { css, useTheme, Global } from '@emotion/react'; export const GlobalStyles = () => { diff --git a/superset-frontend/packages/superset-ui-core/src/theme/Theme.test.tsx b/superset-frontend/packages/superset-ui-core/src/theme/Theme.test.tsx index 288f1f6dbe57..7b16e59289af 100644 --- a/superset-frontend/packages/superset-ui-core/src/theme/Theme.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/theme/Theme.test.tsx @@ -49,14 +49,12 @@ describe('Theme', () => { }); describe('fromConfig', () => { - it('creates a theme with default tokens when no config is provided', () => { + it('creates a theme with Ant Design defaults when no config is provided', () => { const theme = Theme.fromConfig(); - // Verify default primary color is set - expect(theme.theme.colorPrimary).toBe('#2893b3'); - - // Verify default font family is set - expect(theme.theme.fontFamily).toContain('Inter'); + // Verify Ant Design default tokens are set + expect(theme.theme.colorPrimary).toBeDefined(); + expect(theme.theme.fontFamily).toBeDefined(); // Verify the theme is initialized with semantic color tokens expect(theme.theme.colorText).toBeDefined(); @@ -79,8 +77,8 @@ describe('Theme', () => { // Verify custom font family is set expect(theme.theme.fontFamily).toBe('CustomFont, sans-serif'); - // But default tokens should still be preserved for unspecified values - expect(theme.theme.colorError).toBe('#e04355'); + // Unspecified values will use Ant Design defaults + expect(theme.theme.colorError).toBeDefined(); }); it('creates a theme with dark mode when dark algorithm is specified', () => { @@ -205,4 +203,586 @@ describe('Theme', () => { expect(serialized.algorithm).toBe(ThemeAlgorithm.DARK); }); }); + + describe('fromConfig with baseTheme', () => { + it('applies base theme tokens under the main config', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#ff0000', + colorError: '#00ff00', + fontFamily: 'BaseFont', + }, + }; + + const userConfig: AnyThemeConfig = { + token: { + colorPrimary: '#0000ff', + }, + }; + + const theme = Theme.fromConfig(userConfig, baseTheme); + + // User config overrides base theme + expect(theme.theme.colorPrimary).toBe('#0000ff'); + + // Base theme tokens are preserved when not overridden + expect(theme.theme.colorError).toBe('#00ff00'); + expect(theme.theme.fontFamily).toBe('BaseFont'); + }); + + it('applies base theme when no user config is provided', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#ff0000', + fontFamily: 'TestFont', + }, + algorithm: antdThemeImport.darkAlgorithm, + }; + + const theme = Theme.fromConfig(undefined, baseTheme); + + // Color may be transformed by dark algorithm, check fontFamily instead + expect(theme.theme.fontFamily).toBe('TestFont'); + + const serialized = theme.toSerializedConfig(); + expect(serialized.algorithm).toBe(ThemeAlgorithm.DARK); + }); + + it('handles empty config with base theme', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#ff0000', + }, + algorithm: antdThemeImport.defaultAlgorithm, + }; + + const emptyConfig: AnyThemeConfig = {}; + + const theme = Theme.fromConfig(emptyConfig, baseTheme); + + // Base theme tokens should be applied + expect(theme.theme.colorPrimary).toBe('#ff0000'); + + const serialized = theme.toSerializedConfig(); + expect(serialized.algorithm).toBe(ThemeAlgorithm.DEFAULT); + }); + + it('merges algorithms correctly with base theme', () => { + const baseTheme: AnyThemeConfig = { + algorithm: antdThemeImport.compactAlgorithm, + }; + + const userConfig: AnyThemeConfig = { + algorithm: antdThemeImport.darkAlgorithm, + }; + + const theme = Theme.fromConfig(userConfig, baseTheme); + + // User algorithm should override base algorithm + const serialized = theme.toSerializedConfig(); + expect(serialized.algorithm).toBe(ThemeAlgorithm.DARK); + }); + + it('merges component overrides with base theme', () => { + const baseTheme: AnyThemeConfig = { + components: { + Button: { + colorPrimary: '#basebutton', + }, + Input: { + colorBorder: '#baseinput', + }, + }, + }; + + const userConfig: AnyThemeConfig = { + components: { + Button: { + colorPrimary: '#userbutton', + }, + }, + }; + + const theme = Theme.fromConfig(userConfig, baseTheme); + const serialized = theme.toSerializedConfig(); + + // User component config overrides base + expect(serialized.components?.Button?.colorPrimary).toBe('#userbutton'); + + // Base component config preserved when not overridden + expect(serialized.components?.Input?.colorBorder).toBe('#baseinput'); + }); + + it('handles undefined config and undefined base theme', () => { + const theme = Theme.fromConfig(undefined, undefined); + + // Should get Ant Design defaults + expect(theme.theme.colorPrimary).toBeDefined(); + expect(theme.theme.fontFamily).toBeDefined(); + }); + + it('preserves custom tokens in base theme', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#ff0000', + // Custom superset-specific tokens + brandLogoAlt: 'CustomLogo', + menuHoverBackgroundColor: '#00ff00', + } as Record, + }; + + const theme = Theme.fromConfig({}, baseTheme); + + // Standard token + expect(theme.theme.colorPrimary).toBe('#ff0000'); + + // Custom tokens should be preserved + expect((theme.theme as any).brandLogoAlt).toBe('CustomLogo'); + expect((theme.theme as any).menuHoverBackgroundColor).toBe('#00ff00'); + }); + }); + + describe('edge cases with base theme and dark mode', () => { + it('correctly applies base theme tokens in dark mode', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#1890ff', + fontFamily: 'TestFont', + }, + algorithm: antdThemeImport.defaultAlgorithm, + }; + + const baseThemeDark: AnyThemeConfig = { + ...baseTheme, + algorithm: antdThemeImport.darkAlgorithm, + }; + + // Simulate light mode with base theme + const lightTheme = Theme.fromConfig({}, baseTheme); + expect(lightTheme.theme.colorPrimary).toBe('#1890ff'); + expect(lightTheme.theme.fontFamily).toBe('TestFont'); + + // Simulate dark mode with base theme dark + const darkTheme = Theme.fromConfig({}, baseThemeDark); + // Dark algorithm transforms colors, but fontFamily should be preserved + expect(darkTheme.theme.fontFamily).toBe('TestFont'); + + // Verify the algorithm is different + const lightSerialized = lightTheme.toSerializedConfig(); + const darkSerialized = darkTheme.toSerializedConfig(); + expect(lightSerialized.algorithm).toBe(ThemeAlgorithm.DEFAULT); + expect(darkSerialized.algorithm).toBe(ThemeAlgorithm.DARK); + }); + + it('handles switching from custom theme back to base theme', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#1890ff', + }, + algorithm: antdThemeImport.defaultAlgorithm, + }; + + // First apply custom theme + const customConfig: AnyThemeConfig = { + token: { + colorPrimary: '#52c41a', + }, + }; + const themeWithCustom = Theme.fromConfig(customConfig, baseTheme); + expect(themeWithCustom.theme.colorPrimary).toBe('#52c41a'); + + // Then switch back to empty config (simulating removal of custom theme) + const themeWithEmpty = Theme.fromConfig({}, baseTheme); + expect(themeWithEmpty.theme.colorPrimary).toBe('#1890ff'); + + // Verify they produce different outputs + expect(themeWithCustom.theme.colorPrimary).not.toBe( + themeWithEmpty.theme.colorPrimary, + ); + }); + + it('handles algorithm-only config with base theme', () => { + const baseTheme: AnyThemeConfig = { + token: { + fontFamily: 'TestFont', + borderRadius: 8, + }, + algorithm: antdThemeImport.defaultAlgorithm, + }; + + // Config that only specifies algorithm (common for THEME_DARK) + const algorithmOnlyConfig: AnyThemeConfig = { + algorithm: antdThemeImport.darkAlgorithm, + }; + + const theme = Theme.fromConfig(algorithmOnlyConfig, baseTheme); + + // Should have base theme tokens + expect(theme.theme.fontFamily).toBe('TestFont'); + expect(theme.theme.borderRadius).toBe(8); + + // Should have user's algorithm + const serialized = theme.toSerializedConfig(); + expect(serialized.algorithm).toBe(ThemeAlgorithm.DARK); + }); + }); + + describe('base theme integration tests', () => { + it('merges base theme tokens with empty user theme', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + colorError: '#e04355', + fontFamily: 'Inter, Helvetica', + }, + }; + + const userTheme: AnyThemeConfig = { + algorithm: antdThemeImport.defaultAlgorithm, + }; + + const theme = Theme.fromConfig(userTheme, baseTheme); + + expect(theme.theme.colorPrimary).toBe('#2893B3'); + expect(theme.theme.colorError).toBe('#e04355'); + expect(theme.theme.fontFamily).toBe('Inter, Helvetica'); + + // Should have user's algorithm + const serialized = theme.toSerializedConfig(); + expect(serialized.algorithm).toBe(ThemeAlgorithm.DEFAULT); + }); + + it('allows user theme to override specific base theme tokens', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + colorError: '#e04355', + fontFamily: 'Inter, Helvetica', + borderRadius: 4, + }, + }; + + const userTheme: AnyThemeConfig = { + algorithm: antdThemeImport.defaultAlgorithm, + token: { + colorPrimary: '#123456', // Override primary color + // Leave other tokens from base + }, + }; + + const theme = Theme.fromConfig(userTheme, baseTheme); + + // User override should win + expect(theme.theme.colorPrimary).toBe('#123456'); + + // Base theme tokens should be preserved + expect(theme.theme.colorError).toBe('#e04355'); + expect(theme.theme.fontFamily).toBe('Inter, Helvetica'); + expect(theme.theme.borderRadius).toBe(4); + }); + + it('handles base theme with dark algorithm correctly', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + fontFamily: 'Inter, Helvetica', + }, + }; + + const baseThemeDark: AnyThemeConfig = { + ...baseTheme, + algorithm: antdThemeImport.darkAlgorithm, + }; + + const userDarkTheme: AnyThemeConfig = { + algorithm: antdThemeImport.darkAlgorithm, + }; + + const theme = Theme.fromConfig(userDarkTheme, baseThemeDark); + + // Should have base tokens + expect(theme.theme.fontFamily).toBe('Inter, Helvetica'); + + // Should be in dark mode + const serialized = theme.toSerializedConfig(); + expect(serialized.algorithm).toBe(ThemeAlgorithm.DARK); + }); + + it('works with real-world Superset base theme configuration', () => { + // Simulate actual Superset base theme (THEME_DEFAULT/THEME_DARK from config) + const supersetBaseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + colorError: '#e04355', + colorWarning: '#fcc700', + colorSuccess: '#5ac189', + colorInfo: '#66bcfe', + fontFamily: "'Inter', Helvetica, Arial", + fontFamilyCode: "'Fira Code', 'Courier New', monospace", + }, + }; + + // Simulate THEME_DEFAULT from config + const themeDefault: AnyThemeConfig = { + algorithm: antdThemeImport.defaultAlgorithm, + }; + + // Simulate THEME_DARK from config + const themeDark: AnyThemeConfig = { + algorithm: antdThemeImport.darkAlgorithm, + }; + + // Test light mode + const lightTheme = Theme.fromConfig(themeDefault, supersetBaseTheme); + expect(lightTheme.theme.colorPrimary).toBe('#2893B3'); + expect(lightTheme.theme.fontFamily).toBe("'Inter', Helvetica, Arial"); + + // Test dark mode + const darkTheme = Theme.fromConfig(themeDark, { + ...supersetBaseTheme, + algorithm: antdThemeImport.darkAlgorithm, + }); + expect(darkTheme.theme.fontFamily).toBe("'Inter', Helvetica, Arial"); + + const darkSerialized = darkTheme.toSerializedConfig(); + expect(darkSerialized.algorithm).toBe(ThemeAlgorithm.DARK); + }); + + it('handles component overrides in base theme', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + }, + components: { + Button: { + primaryColor: '#custom-button', + borderRadius: 8, + }, + }, + }; + + const userTheme: AnyThemeConfig = { + algorithm: antdThemeImport.defaultAlgorithm, + }; + + const theme = Theme.fromConfig(userTheme, baseTheme); + + // Should preserve component overrides + const serialized = theme.toSerializedConfig(); + expect(serialized.components?.Button?.primaryColor).toBe( + '#custom-button', + ); + expect(serialized.components?.Button?.borderRadius).toBe(8); + }); + + it('properly handles algorithm property override', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + }, + algorithm: antdThemeImport.defaultAlgorithm, + }; + + const userTheme: AnyThemeConfig = { + algorithm: antdThemeImport.darkAlgorithm, + token: { + borderRadius: 8, + }, + }; + + const theme = Theme.fromConfig(userTheme, baseTheme); + const serialized = theme.toSerializedConfig(); + + // User algorithm should override base algorithm + expect(serialized.algorithm).toBe(ThemeAlgorithm.DARK); + + // Both base and user tokens should be merged + expect(serialized.token?.colorPrimary).toBeTruthy(); + expect(serialized.token?.borderRadius).toBe(8); + }); + + it('handles cssVar, hashed and inherit properties correctly', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + }, + cssVar: true, + hashed: false, + }; + + const userTheme: AnyThemeConfig = { + token: { + borderRadius: 8, + }, + inherit: true, + }; + + const theme = Theme.fromConfig(userTheme, baseTheme); + const serialized = theme.toSerializedConfig(); + + // User properties override/add to base + expect(serialized.inherit).toBe(true); + expect(serialized.cssVar).toBe(true); + expect(serialized.hashed).toBe(false); + + // Tokens are still merged + expect(serialized.token?.colorPrimary).toBeTruthy(); + expect(serialized.token?.borderRadius).toBe(8); + }); + + it('merges nested component styles correctly', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + fontFamily: 'BaseFont', + }, + components: { + Button: { + colorPrimary: '#basebutton', + fontSize: 14, + }, + Input: { + colorBorder: '#baseinput', + }, + }, + }; + + const userTheme: AnyThemeConfig = { + token: { + borderRadius: 8, + }, + components: { + Button: { + fontSize: 16, // Override Button fontSize + }, + Select: { + colorBorder: '#userselect', // Add new component + }, + }, + }; + + const theme = Theme.fromConfig(userTheme, baseTheme); + const serialized = theme.toSerializedConfig(); + + // Tokens should be merged + // Note: components present may affect color transformation + expect(serialized.token?.colorPrimary).toBeTruthy(); + expect(serialized.token?.borderRadius).toBe(8); + expect(serialized.token?.fontFamily).toBe('BaseFont'); + + // Components should be merged (shallow merge per component) + expect(serialized.components?.Button?.colorPrimary).toBe('#basebutton'); + expect(serialized.components?.Button?.fontSize).toBe(16); // User override + expect(serialized.components?.Input?.colorBorder).toBe('#baseinput'); + expect(serialized.components?.Select?.colorBorder).toBe('#userselect'); + }); + + it('setConfig replaces theme config entirely (does not preserve base theme)', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + fontFamily: 'Inter', + }, + }; + + const theme = Theme.fromConfig({}, baseTheme); + + expect(theme.theme.colorPrimary).toBe('#2893B3'); + expect(theme.theme.fontFamily).toBe('Inter'); + + // Update config (simulating theme change) + theme.setConfig({ + token: { + colorPrimary: '#654321', + }, + algorithm: antdThemeImport.darkAlgorithm, + }); + + // setConfig replaces the entire config, so base theme is NOT preserved + // This is expected behavior - setConfig is for complete replacement + expect(theme.theme.colorPrimary).toBeTruthy(); + // fontFamily reverts to Ant Design default since base theme is not reapplied + expect(theme.theme.fontFamily).not.toBe('Inter'); + }); + + it('minimal theme preserves ALL base theme tokens except overridden ones', () => { + // Simulate a comprehensive base theme with many tokens + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + colorError: '#e04355', + colorWarning: '#fcc700', + colorSuccess: '#5ac189', + colorInfo: '#66bcfe', + fontFamily: 'Inter, Helvetica', + fontSize: 14, + borderRadius: 4, + lineWidth: 1, + controlHeight: 32, + // Custom Superset tokens + brandLogoAlt: 'CustomLogo', + menuHoverBackgroundColor: '#eeeeee', + } as Record, + algorithm: antdThemeImport.defaultAlgorithm, + }; + + // Minimal theme that only overrides primary color and algorithm + const minimalTheme: AnyThemeConfig = { + token: { + colorPrimary: '#ff05dd', // Only override this + }, + algorithm: antdThemeImport.darkAlgorithm, // Change to dark + }; + + const theme = Theme.fromConfig(minimalTheme, baseTheme); + + // User's override should apply + expect(theme.theme.colorPrimary).toBe('#ff05dd'); + + // ALL base theme tokens should be preserved + expect(theme.theme.colorError).toBe('#e04355'); + expect(theme.theme.colorWarning).toBe('#fcc700'); + expect(theme.theme.colorSuccess).toBe('#5ac189'); + expect(theme.theme.colorInfo).toBe('#66bcfe'); + expect(theme.theme.fontFamily).toBe('Inter, Helvetica'); + expect(theme.theme.fontSize).toBe(14); + expect(theme.theme.borderRadius).toBe(4); + expect(theme.theme.lineWidth).toBe(1); + expect(theme.theme.controlHeight).toBe(32); + + // Custom tokens should also be preserved + expect((theme.theme as any).brandLogoAlt).toBe('CustomLogo'); + expect((theme.theme as any).menuHoverBackgroundColor).toBe('#eeeeee'); + + // Algorithm should be updated + const serialized = theme.toSerializedConfig(); + expect(serialized.algorithm).toBe(ThemeAlgorithm.DARK); + }); + + it('arrays in themes are replaced entirely, not merged by index', () => { + const baseTheme: AnyThemeConfig = { + token: { + colorPrimary: '#2893B3', + }, + algorithm: [ + antdThemeImport.compactAlgorithm, + antdThemeImport.defaultAlgorithm, + ], + }; + + const userTheme: AnyThemeConfig = { + algorithm: [antdThemeImport.darkAlgorithm], // Replace with single item array + }; + + const theme = Theme.fromConfig(userTheme, baseTheme); + const serialized = theme.toSerializedConfig(); + + // User's array should completely replace base array + expect(Array.isArray(serialized.algorithm)).toBe(true); + expect(serialized.algorithm).toHaveLength(1); + expect(serialized.algorithm).toContain(ThemeAlgorithm.DARK); + expect(serialized.algorithm).not.toContain(ThemeAlgorithm.COMPACT); + expect(serialized.algorithm).not.toContain(ThemeAlgorithm.DEFAULT); + }); + }); }); diff --git a/superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx b/superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx index 308eda8cce1b..77bbecda7198 100644 --- a/superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx +++ b/superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx @@ -20,31 +20,13 @@ // eslint-disable-next-line no-restricted-syntax import React from 'react'; import { theme as antdThemeImport, ConfigProvider } from 'antd'; - -// @fontsource/* v5.1+ doesn't play nice with eslint-import plugin v2.31+ -/* eslint-disable import/extensions */ -import '@fontsource/inter/200.css'; -/* eslint-disable import/extensions */ -import '@fontsource/inter/400.css'; -/* eslint-disable import/extensions */ -import '@fontsource/inter/500.css'; -/* eslint-disable import/extensions */ -import '@fontsource/inter/600.css'; -/* eslint-disable import/extensions */ -import '@fontsource/fira-code/400.css'; -/* eslint-disable import/extensions */ -import '@fontsource/fira-code/500.css'; -/* eslint-disable import/extensions */ -import '@fontsource/fira-code/600.css'; - import { ThemeProvider, CacheProvider as EmotionCacheProvider, } from '@emotion/react'; import createCache from '@emotion/cache'; -import { noop } from 'lodash'; +import { noop, mergeWith } from 'lodash'; import { GlobalStyles } from './GlobalStyles'; - import { AntdThemeConfig, AnyThemeConfig, @@ -53,63 +35,16 @@ import { allowedAntdTokens, SharedAntdTokens, } from './types'; - import { normalizeThemeConfig, serializeThemeConfig } from './utils'; -/* eslint-disable theme-colors/no-literal-colors */ - export class Theme { theme: SupersetTheme; - private static readonly defaultTokens = { - // Brand - brandLogoAlt: 'Apache Superset', - brandLogoUrl: '/static/assets/images/superset-logo-horiz.png', - brandLogoMargin: '18px', - brandLogoHref: '/', - brandLogoHeight: '24px', - - // Spinner - brandSpinnerUrl: undefined, - brandSpinnerSvg: undefined, - - // Default colors - colorPrimary: '#2893B3', // NOTE: previous lighter primary color was #20a7c9 - colorLink: '#2893B3', - colorError: '#e04355', - colorWarning: '#fcc700', - colorSuccess: '#5ac189', - colorInfo: '#66bcfe', - - // Forcing some default tokens - fontFamily: `'Inter', Helvetica, Arial`, - fontFamilyCode: `'Fira Code', 'Courier New', monospace`, - - // Extra tokens - transitionTiming: 0.3, - brandIconMaxWidth: 37, - fontSizeXS: '8', - fontSizeXXL: '28', - fontWeightNormal: '400', - fontWeightLight: '300', - fontWeightStrong: 500, - }; - private antdConfig: AntdThemeConfig; private constructor({ config }: { config?: AnyThemeConfig }) { this.SupersetThemeProvider = this.SupersetThemeProvider.bind(this); - - // Create a new config object with default tokens - const newConfig: AnyThemeConfig = config ? { ...config } : {}; - - // Ensure token property exists with defaults - newConfig.token = { - ...Theme.defaultTokens, - ...(config?.token || {}), - }; - - this.setConfig(newConfig); + this.setConfig(config || {}); } /** @@ -118,9 +53,24 @@ export class Theme { * If simple tokens are provided as { token: {...} }, they will be applied with defaults * If no config is provided, uses default tokens * Dark mode can be set via the algorithm property in the config + * @param config - The theme configuration + * @param baseTheme - Optional base theme to apply under the config */ - static fromConfig(config?: AnyThemeConfig): Theme { - return new Theme({ config }); + static fromConfig( + config?: AnyThemeConfig, + baseTheme?: AnyThemeConfig, + ): Theme { + let mergedConfig: AnyThemeConfig | undefined = config; + + if (baseTheme && config) { + mergedConfig = mergeWith({}, baseTheme, config, (objValue, srcValue) => + Array.isArray(srcValue) ? srcValue : undefined, + ); + } else if (baseTheme && !config) { + mergedConfig = baseTheme; + } + + return new Theme({ config: mergedConfig }); } private static getFilteredAntdTheme( @@ -148,22 +98,15 @@ export class Theme { setConfig(config: AnyThemeConfig): void { const antdConfig = normalizeThemeConfig(config); - // Apply default tokens to token property - antdConfig.token = { - ...Theme.defaultTokens, - ...(antdConfig.token || {}), - }; - // First phase: Let Ant Design compute the tokens const tokens = Theme.getFilteredAntdTheme(antdConfig); // Set the base theme properties this.antdConfig = antdConfig; this.theme = { - ...Theme.defaultTokens, - ...antdConfig.token, // Passing through the extra, superset-specific tokens - ...tokens, - }; + ...tokens, // First apply Ant Design computed tokens + ...(antdConfig.token || {}), // Then override with our custom tokens + } as SupersetTheme; // Update the providers with the fully formed theme this.updateProviders( @@ -250,5 +193,3 @@ export class Theme { ); } } - -/* eslint-enable theme-colors/no-literal-colors */ diff --git a/superset-frontend/packages/superset-ui-core/src/theme/types.ts b/superset-frontend/packages/superset-ui-core/src/theme/types.ts index b9e2a000ea30..f4ae4f64df2e 100644 --- a/superset-frontend/packages/superset-ui-core/src/theme/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/theme/types.ts @@ -78,6 +78,7 @@ export type SerializableThemeConfig = { algorithm?: ThemeAlgorithmOption; hashed?: boolean; inherit?: boolean; + cssVar?: boolean | { key?: string; prefix?: string }; }; /** diff --git a/superset-frontend/packages/superset-ui-core/src/theme/utils/themeUtils.test.ts b/superset-frontend/packages/superset-ui-core/src/theme/utils/themeUtils.test.ts index e72f8291cd64..da48a93c7973 100644 --- a/superset-frontend/packages/superset-ui-core/src/theme/utils/themeUtils.test.ts +++ b/superset-frontend/packages/superset-ui-core/src/theme/utils/themeUtils.test.ts @@ -16,7 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import { getFontSize, getColorVariants, isThemeDark } from './themeUtils'; +import { theme as antdTheme } from 'antd'; +import { + getFontSize, + getColorVariants, + isThemeDark, + isThemeConfigDark, +} from './themeUtils'; import { Theme } from '../Theme'; import { ThemeAlgorithm } from '../types'; @@ -71,8 +77,7 @@ describe('themeUtils', () => { token: { fontSize: '14' }, }); - // Ant Design provides fontSizeXS: '8' by default - expect(getFontSize(minimalTheme.theme, 'xs')).toBe('8'); + expect(getFontSize(minimalTheme.theme, 'xs')).toBe('14'); expect(getFontSize(minimalTheme.theme, 'm')).toBe('14'); }); }); @@ -131,4 +136,111 @@ describe('themeUtils', () => { expect(variants.bg).toBeUndefined(); }); }); + + describe('isThemeConfigDark', () => { + it('returns true for config with dark algorithm', () => { + const config = { + algorithm: antdTheme.darkAlgorithm, + }; + expect(isThemeConfigDark(config)).toBe(true); + }); + + it('returns true for config with dark algorithm in array', () => { + const config = { + algorithm: [antdTheme.darkAlgorithm, antdTheme.compactAlgorithm], + }; + expect(isThemeConfigDark(config)).toBe(true); + }); + + it('returns false for config without dark algorithm', () => { + const config = { + algorithm: antdTheme.defaultAlgorithm, + }; + expect(isThemeConfigDark(config)).toBe(false); + }); + + it('returns false for config with no algorithm', () => { + const config = { + token: { + colorPrimary: '#1890ff', + }, + }; + expect(isThemeConfigDark(config)).toBe(false); + }); + + it('detects manually-created dark theme without dark algorithm', () => { + // This is the edge case: dark colors without dark algorithm + const config = { + token: { + colorBgContainer: '#1a1a1a', // Dark background + colorBgBase: '#0a0a0a', // Dark base + colorText: '#ffffff', // Light text + }, + }; + expect(isThemeConfigDark(config)).toBe(true); + }); + + it('does not false-positive on light theme with custom colors', () => { + const config = { + token: { + colorBgContainer: '#ffffff', // Light background + colorBgBase: '#f5f5f5', // Light base + colorText: '#000000', // Dark text + }, + }; + expect(isThemeConfigDark(config)).toBe(false); + }); + + it('handles partial color tokens gracefully', () => { + // With actual theme computation, a dark colorBgContainer results in a dark theme + const config = { + token: { + colorBgContainer: '#1a1a1a', // Dark background + // Missing other color tokens + }, + }; + expect(isThemeConfigDark(config)).toBe(true); + }); + + it('respects colorBgContainer as the primary indicator', () => { + // The computed theme uses colorBgContainer as the main background + const darkConfig = { + token: { + colorBgContainer: '#1a1a1a', // Dark background + colorText: '#000000', // Dark text (unusual but doesn't override) + }, + }; + expect(isThemeConfigDark(darkConfig)).toBe(true); + + const lightConfig = { + token: { + colorBgContainer: '#ffffff', // Light background + colorText: '#ffffff', // Light text (unusual but doesn't override) + }, + }; + expect(isThemeConfigDark(lightConfig)).toBe(false); + }); + + it('handles non-string color tokens gracefully', () => { + const config = { + token: { + colorBgContainer: undefined, + colorText: null, + colorBgBase: 123, // Invalid type + }, + }; + expect(isThemeConfigDark(config)).toBe(false); + }); + + it('returns false for empty config', () => { + expect(isThemeConfigDark({})).toBe(false); + }); + + it('returns false for config with empty token object', () => { + const config = { + token: {}, + }; + expect(isThemeConfigDark(config)).toBe(false); + }); + }); }); diff --git a/superset-frontend/packages/superset-ui-core/src/theme/utils/themeUtils.ts b/superset-frontend/packages/superset-ui-core/src/theme/utils/themeUtils.ts index 36859cc887ac..a896a7b5fad4 100644 --- a/superset-frontend/packages/superset-ui-core/src/theme/utils/themeUtils.ts +++ b/superset-frontend/packages/superset-ui-core/src/theme/utils/themeUtils.ts @@ -18,7 +18,14 @@ */ import tinycolor from 'tinycolor2'; import { useTheme as useEmotionTheme } from '@emotion/react'; -import type { SupersetTheme, FontSizeKey, ColorVariants } from '../types'; +import { theme as antdTheme } from 'antd'; +import type { + SupersetTheme, + FontSizeKey, + ColorVariants, + AnyThemeConfig, +} from '../types'; +import { normalizeThemeConfig } from '../utils'; const fontSizeMap: Record = { xs: 'fontSizeXS', @@ -113,6 +120,22 @@ export function isThemeDark(theme: SupersetTheme): boolean { return tinycolor(theme.colorBgContainer).isDark(); } +/** + * Check if a theme configuration results in a dark theme + * @param config - The theme configuration to check + * @returns true if the config results in a dark theme, false otherwise + */ +export function isThemeConfigDark(config: AnyThemeConfig): boolean { + try { + const normalizedConfig = normalizeThemeConfig(config); + const themeConfig = antdTheme.getDesignToken(normalizedConfig); + + return tinycolor(themeConfig.colorBgContainer).isDark(); + } catch { + return false; + } +} + /** * Hook to determine if the current theme is dark mode * @returns true if theme is dark, false if light diff --git a/superset-frontend/src/pages/ThemeList/index.tsx b/superset-frontend/src/pages/ThemeList/index.tsx index 8c570f46cf58..0cb78a1cab96 100644 --- a/superset-frontend/src/pages/ThemeList/index.tsx +++ b/superset-frontend/src/pages/ThemeList/index.tsx @@ -17,7 +17,7 @@ * under the License. */ -import { useMemo, useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; import { t, SupersetClient, styled } from '@superset-ui/core'; import { Tag, @@ -72,6 +72,11 @@ const FlexRowContainer = styled.div` } `; +const IconTag = styled(Tag)` + display: inline-flex; + align-items: center; +`; + const CONFIRM_OVERWRITE_MESSAGE = t( 'You are importing one or more themes that already exist. ' + 'Overwriting might cause you to lose some of your work. Are you ' + @@ -112,6 +117,16 @@ function ThemesList({ const [importingTheme, showImportModal] = useState(false); const [appliedThemeId, setAppliedThemeId] = useState(null); + // State for confirmation modal + const [confirmModalConfig, setConfirmModalConfig] = useState<{ + visible: boolean; + title: string; + message: string; + onConfirm: () => Promise; + successMessage: string; + errorMessage: string; + } | null>(null); + const canCreate = hasPerm('can_write'); const canEdit = hasPerm('can_write'); const canDelete = hasPerm('can_write'); @@ -189,20 +204,23 @@ function ThemesList({ setThemeModalOpen(true); } - function handleThemeApply(themeObj: ThemeObject) { - if (themeObj.json_data) { - try { - const themeConfig = JSON.parse(themeObj.json_data); - setTemporaryTheme(themeConfig); - setAppliedThemeId(themeObj.id || null); - addSuccessToast(t('Local theme set to "%s"', themeObj.theme_name)); - } catch (error) { - addDangerToast( - t('Failed to set local theme: Invalid JSON configuration'), - ); + const handleThemeApply = useCallback( + (themeObj: ThemeObject) => { + if (themeObj.json_data) { + try { + const themeConfig = JSON.parse(themeObj.json_data); + setTemporaryTheme(themeConfig); + setAppliedThemeId(themeObj.id || null); + addSuccessToast(t('Local theme set to "%s"', themeObj.theme_name)); + } catch (error) { + addDangerToast( + t('Failed to set local theme: Invalid JSON configuration'), + ); + } } - } - } + }, + [setTemporaryTheme, addSuccessToast, addDangerToast], + ); function handleThemeModalApply() { // Clear any previously applied theme ID when applying from modal @@ -235,60 +253,83 @@ function ThemesList({ }; // Generic confirmation modal utility to reduce code duplication - const showThemeConfirmation = (config: { - title: string; - content: string; - onConfirm: () => Promise; - successMessage: string; - errorMessage: string; - }) => { - Modal.confirm({ - title: config.title, - content: config.content, - onOk: () => { - config - .onConfirm() - .then(() => { - refreshData(); - addSuccessToast(config.successMessage); - }) - .catch(err => { - addDangerToast(t(config.errorMessage, err.message)); - }); - }, - }); - }; + const showThemeConfirmation = useCallback( + (config: { + title: string; + content: string; + onConfirm: () => Promise; + successMessage: string; + errorMessage: string; + }) => { + setConfirmModalConfig({ + visible: true, + title: config.title, + message: config.content, + onConfirm: config.onConfirm, + successMessage: config.successMessage, + errorMessage: config.errorMessage, + }); + }, + [], + ); - const handleSetSystemDefault = (theme: ThemeObject) => { - showThemeConfirmation({ - title: t('Set System Default Theme'), - content: t( - 'Are you sure you want to set "%s" as the system default theme? This will apply to all users who haven\'t set a personal preference.', - theme.theme_name, - ), - onConfirm: () => setSystemDefaultTheme(theme.id!), - successMessage: t( - '"%s" is now the system default theme', - theme.theme_name, - ), - errorMessage: 'Failed to set system default theme: %s', - }); + const handleConfirmModalOk = async () => { + if (!confirmModalConfig) return; + + try { + await confirmModalConfig.onConfirm(); + refreshData(); + addSuccessToast(confirmModalConfig.successMessage); + setConfirmModalConfig(null); + } catch (err: any) { + addDangerToast(t(confirmModalConfig.errorMessage, err.message)); + } }; - const handleSetSystemDark = (theme: ThemeObject) => { - showThemeConfirmation({ - title: t('Set System Dark Theme'), - content: t( - 'Are you sure you want to set "%s" as the system dark theme? This will apply to all users who haven\'t set a personal preference.', - theme.theme_name, - ), - onConfirm: () => setSystemDarkTheme(theme.id!), - successMessage: t('"%s" is now the system dark theme', theme.theme_name), - errorMessage: 'Failed to set system dark theme: %s', - }); + const handleConfirmModalCancel = () => { + setConfirmModalConfig(null); }; - const handleUnsetSystemDefault = () => { + const handleSetSystemDefault = useCallback( + (theme: ThemeObject) => { + showThemeConfirmation({ + title: t('Set System Default Theme'), + content: t( + 'Are you sure you want to set "%s" as the system default theme? This will apply to all users who haven\'t set a personal preference.', + theme.theme_name, + ), + onConfirm: () => setSystemDefaultTheme(theme.id!), + successMessage: t( + '"%s" is now the system default theme', + theme.theme_name, + ), + errorMessage: 'Failed to set system default theme: %s', + }); + }, + [showThemeConfirmation], + ); + + const handleSetSystemDark = useCallback( + (theme: ThemeObject) => { + showThemeConfirmation({ + title: t('Set System Dark Theme'), + content: t( + 'Are you sure you want to set "%s" as the system dark theme? This will apply to all users who haven\'t set a personal preference.', + theme.theme_name, + ), + onConfirm: () => setSystemDarkTheme(theme.id!), + successMessage: t( + '"%s" is now the system dark theme', + theme.theme_name, + theme.theme_name, + ), + errorMessage: 'Failed to set system dark theme: %s', + }); + }, + [showThemeConfirmation], + ); + + const handleUnsetSystemDefault = useCallback(() => { showThemeConfirmation({ title: t('Remove System Default Theme'), content: t( @@ -298,9 +339,9 @@ function ThemesList({ successMessage: t('System default theme removed'), errorMessage: 'Failed to remove system default theme: %s', }); - }; + }, [showThemeConfirmation]); - const handleUnsetSystemDark = () => { + const handleUnsetSystemDark = useCallback(() => { showThemeConfirmation({ title: t('Remove System Dark Theme'), content: t( @@ -310,7 +351,7 @@ function ThemesList({ successMessage: t('System dark theme removed'), errorMessage: 'Failed to remove system dark theme: %s', }); - }; + }, [showThemeConfirmation]); const initialSort = [{ id: 'theme_name', desc: true }]; const columns = useMemo( @@ -340,16 +381,16 @@ function ThemesList({ )} {original.is_system_default && ( - - {t('Default')} - + }> + {t('Default')} + )} {original.is_system_dark && ( - - {t('Dark')} - + }> + {t('Dark')} + )} @@ -487,12 +528,19 @@ function ThemesList({ }, ], [ + canEdit, canDelete, - canCreate, canApply, canExport, - canSetSystemThemes, + getCurrentCrudThemeId, appliedThemeId, + canSetSystemThemes, + addDangerToast, + handleThemeApply, + handleSetSystemDefault, + handleUnsetSystemDefault, + handleSetSystemDark, + handleUnsetSystemDark, ], ); @@ -570,7 +618,7 @@ function ThemesList({ paginate: true, }, ], - [], + [user], ); return ( @@ -681,6 +729,17 @@ function ThemesList({ }} {preparingExport && } + {confirmModalConfig?.visible && ( + + {confirmModalConfig.message} + + )} ); } diff --git a/superset-frontend/src/theme/ThemeController.ts b/superset-frontend/src/theme/ThemeController.ts index a40dafca5e67..c4f297916d82 100644 --- a/superset-frontend/src/theme/ThemeController.ts +++ b/superset-frontend/src/theme/ThemeController.ts @@ -18,18 +18,15 @@ */ import { type AnyThemeConfig, - type SupersetTheme, type SupersetThemeConfig, type ThemeControllerOptions, type ThemeStorage, + isThemeConfigDark, Theme, ThemeMode, themeObject as supersetThemeObject, } from '@superset-ui/core'; -import { - getAntdConfig, - normalizeThemeConfig, -} from '@superset-ui/core/theme/utils'; +import { normalizeThemeConfig } from '@superset-ui/core/theme/utils'; import type { BootstrapThemeData, BootstrapThemeDataConfig, @@ -79,7 +76,7 @@ export class ThemeController { private modeStorageKey: string; - private defaultTheme: AnyThemeConfig; + private defaultTheme: AnyThemeConfig | null; private darkTheme: AnyThemeConfig | null; @@ -87,8 +84,6 @@ export class ThemeController { private currentMode: ThemeMode; - private hasCustomThemes: boolean; - private onChangeCallbacks: Set<(theme: Theme) => void> = new Set(); private mediaQuery: MediaQueryList; @@ -116,22 +111,13 @@ export class ThemeController { this.globalTheme = themeObject; // Initialize bootstrap data and themes - const { - bootstrapDefaultTheme, - bootstrapDarkTheme, - hasCustomThemes, - }: BootstrapThemeData = this.loadBootstrapData(); + const { bootstrapDefaultTheme, bootstrapDarkTheme }: BootstrapThemeData = + this.loadBootstrapData(); - this.hasCustomThemes = hasCustomThemes; - - // Set themes based on bootstrap data availability - if (this.hasCustomThemes) { - this.darkTheme = bootstrapDarkTheme; - this.defaultTheme = bootstrapDefaultTheme || defaultTheme; - } else { - this.darkTheme = null; - this.defaultTheme = defaultTheme; - } + // Set themes from bootstrap data + // These will be the THEME_DEFAULT and THEME_DARK from config + this.defaultTheme = bootstrapDefaultTheme || defaultTheme || null; + this.darkTheme = bootstrapDarkTheme; // Initialize system theme detection this.systemMode = ThemeController.getSystemPreferredMode(); @@ -147,7 +133,7 @@ export class ThemeController { // Initialize theme and mode this.currentMode = this.determineInitialMode(); const initialTheme = - this.getThemeForMode(this.currentMode) || this.defaultTheme; + this.getThemeForMode(this.currentMode) || this.defaultTheme || {}; // Setup change callback if (onChange) this.onChangeCallbacks.add(onChange); @@ -197,6 +183,7 @@ export class ThemeController { /** * Gets the theme configuration for a specific context (global vs dashboard). + * Dashboard themes are always merged with base theme. * @param forDashboard - Whether to get the dashboard theme or global theme * @returns The theme configuration for the specified context */ @@ -205,7 +192,16 @@ export class ThemeController { ): AnyThemeConfig | null { // For dashboard context, prioritize dashboard CRUD theme if (forDashboard && this.dashboardCrudTheme) { - return this.dashboardCrudTheme; + // Dashboard CRUD themes should be merged with base theme + const normalizedTheme = this.normalizeTheme(this.dashboardCrudTheme); + const isDarkMode = isThemeConfigDark(normalizedTheme); + const baseTheme = isDarkMode ? this.darkTheme : this.defaultTheme; + + if (baseTheme) { + const mergedTheme = Theme.fromConfig(normalizedTheme, baseTheme); + return mergedTheme.toSerializedConfig(); + } + return normalizedTheme; } // For global context or when no dashboard theme, use mode-based theme @@ -241,7 +237,15 @@ export class ThemeController { // Controller creates and owns the dashboard theme const { Theme } = await import('@superset-ui/core'); const normalizedConfig = this.normalizeTheme(themeConfig); - const dashboardTheme = Theme.fromConfig(normalizedConfig); + + // Determine if this is a dark theme and get appropriate base + const isDarkMode = isThemeConfigDark(normalizedConfig); + const baseTheme = isDarkMode ? this.darkTheme : this.defaultTheme; + + const dashboardTheme = Theme.fromConfig( + normalizedConfig, + baseTheme || undefined, + ); // Cache the theme for reuse this.dashboardThemes.set(themeId, dashboardTheme); @@ -325,7 +329,7 @@ export class ThemeController { public resetTheme(): void { this.currentMode = ThemeMode.DEFAULT; const defaultTheme: AnyThemeConfig = - this.getThemeForMode(ThemeMode.DEFAULT) || this.defaultTheme; + this.getThemeForMode(ThemeMode.DEFAULT) || this.defaultTheme || {}; this.updateTheme(defaultTheme); } @@ -373,8 +377,8 @@ export class ThemeController { JSON.stringify(theme), ); - const normalizedTheme = this.normalizeTheme(theme); - this.updateTheme(normalizedTheme); + const mergedTheme = this.getThemeForMode(this.currentMode); + if (mergedTheme) this.updateTheme(mergedTheme); } /** @@ -384,10 +388,14 @@ export class ThemeController { public clearLocalOverrides(): void { this.devThemeOverride = null; this.crudThemeId = null; + this.dashboardCrudTheme = null; this.storage.removeItem(STORAGE_KEYS.DEV_THEME_OVERRIDE); this.storage.removeItem(STORAGE_KEYS.CRUD_THEME_ID); + // Clear dashboard themes cache + this.dashboardThemes.clear(); + this.resetTheme(); } @@ -407,7 +415,7 @@ export class ThemeController { /** * Checks if OS preference detection is allowed. - * Allowed when both themes are available + * Allowed when dark theme is available (including base dark theme) */ public canDetectOSPreference(): boolean { return this.darkTheme !== null; @@ -422,7 +430,6 @@ export class ThemeController { public setThemeConfig(config: SupersetThemeConfig): void { this.defaultTheme = config.theme_default; this.darkTheme = config.theme_dark || null; - this.hasCustomThemes = true; let newMode: ThemeMode; try { @@ -478,13 +485,16 @@ export class ThemeController { private updateTheme(theme?: AnyThemeConfig): void { try { // If no config provided, use current mode to get theme - const config: AnyThemeConfig = - theme || this.getThemeForMode(this.currentMode) || this.defaultTheme; - - // Normalize the theme - const normalizedTheme = this.normalizeTheme(config); + if (!theme) { + // No theme provided, use the current mode's theme + const modeTheme = + this.getThemeForMode(this.currentMode) || this.defaultTheme || {}; + this.applyTheme(modeTheme); + } else { + // Theme provided, apply it directly + this.applyTheme(theme); + } - this.applyTheme(normalizedTheme); this.persistMode(); this.notifyListeners(); } catch (error) { @@ -501,7 +511,7 @@ export class ThemeController { // Get the default theme which will have the correct algorithm const defaultTheme: AnyThemeConfig = - this.getThemeForMode(ThemeMode.DEFAULT) || this.defaultTheme; + this.getThemeForMode(ThemeMode.DEFAULT) || this.defaultTheme || {}; this.applyTheme(defaultTheme); this.persistMode(); @@ -554,10 +564,15 @@ export class ThemeController { const hasValidDefault: boolean = this.isNonEmptyObject(defaultTheme); const hasValidDark: boolean = this.isNonEmptyObject(darkTheme); + // Check if themes have actual custom tokens (not just empty or algorithm-only) + const hasCustomDefault = + hasValidDefault && !this.isEmptyTheme(defaultTheme); + const hasCustomDark = hasValidDark && !this.isEmptyTheme(darkTheme); + return { - bootstrapDefaultTheme: hasValidDefault ? defaultTheme : null, - bootstrapDarkTheme: hasValidDark ? darkTheme : null, - hasCustomThemes: hasValidDefault || hasValidDark, + bootstrapDefaultTheme: hasCustomDefault ? defaultTheme : null, + bootstrapDarkTheme: hasCustomDark ? darkTheme : null, + hasCustomThemes: hasCustomDefault || hasCustomDark, }; } @@ -572,6 +587,20 @@ export class ThemeController { ); } + /** + * Checks if a theme is truly empty (not even an algorithm). + * A theme with just an algorithm is still valid and should be used. + */ + private isEmptyTheme(theme: AnyThemeConfig | undefined): boolean { + if (!theme) return true; + + return !( + theme.algorithm || + (theme.token && Object.keys(theme.token).length > 0) || + (theme.components && Object.keys(theme.components).length > 0) + ); + } + /** * Normalizes the theme configuration to ensure it has a valid algorithm. * @param theme - The theme configuration to normalize @@ -588,49 +617,45 @@ export class ThemeController { * @returns The theme configuration for the specified mode or null if not available */ private getThemeForMode(mode: ThemeMode): AnyThemeConfig | null { - // Priority 1: Dev theme override (highest priority for development) - // Dev overrides affect all contexts if (this.devThemeOverride) { - return this.devThemeOverride; + const normalizedOverride = this.normalizeTheme(this.devThemeOverride); + const isDarkMode = isThemeConfigDark(normalizedOverride); + const baseTheme = isDarkMode ? this.darkTheme : this.defaultTheme; + + if (baseTheme) { + const mergedTheme = Theme.fromConfig(normalizedOverride, baseTheme); + return mergedTheme.toSerializedConfig(); + } + + return normalizedOverride; } - // Priority 2: System theme based on mode (applies to all contexts) let resolvedMode: ThemeMode = mode; if (mode === ThemeMode.SYSTEM) { - // OS preference is allowed when dark theme exists if (this.darkTheme === null) return null; resolvedMode = ThemeController.getSystemPreferredMode(); } - if (!this.hasCustomThemes) { - const baseTheme = this.defaultTheme.token as Partial; - return getAntdConfig(baseTheme, resolvedMode === ThemeMode.DARK); - } - - // Handle bootstrap themes using existing normalization - const selectedTheme: AnyThemeConfig = - resolvedMode === ThemeMode.DARK - ? this.darkTheme || this.defaultTheme - : this.defaultTheme; + if (resolvedMode === ThemeMode.DARK) return this.darkTheme; - return selectedTheme; + return this.defaultTheme; } /** * Determines the initial theme mode with error recovery. */ private determineInitialMode(): ThemeMode { + // Try to restore saved mode first + const savedMode: ThemeMode | null = this.loadSavedMode(); + if (savedMode && this.isValidThemeMode(savedMode)) return savedMode; + // If no dark theme is available, force default mode if (this.darkTheme === null) { this.storage.removeItem(this.modeStorageKey); return ThemeMode.DEFAULT; } - // Try to restore saved mode - const savedMode: ThemeMode | null = this.loadSavedMode(); - if (savedMode && this.isValidThemeMode(savedMode)) return savedMode; - // Default to system preference when both themes are available return ThemeMode.SYSTEM; } @@ -663,11 +688,14 @@ export class ThemeController { // Validate that we have the required theme data for the mode switch (mode) { case ThemeMode.DARK: - return !!(this.darkTheme || this.defaultTheme); + // Dark mode is valid if we have a dark theme + return !!this.darkTheme; case ThemeMode.DEFAULT: + // Default mode is valid if we have a default theme return !!this.defaultTheme; case ThemeMode.SYSTEM: - return this.darkTheme !== null; + // System mode is valid if dark mode is available + return !!this.darkTheme; default: return true; } @@ -698,11 +726,15 @@ export class ThemeController { * Applies the current theme configuration to the global theme. * This method sets the theme on the globalTheme and applies it to the Theme. * It also handles any errors that may occur during the application of the theme. - * @param theme - The theme configuration to apply + * @param theme - The theme configuration to apply (may already include base theme tokens) */ private applyTheme(theme: AnyThemeConfig): void { try { const normalizedConfig = normalizeThemeConfig(theme); + + // Simply apply the theme - it should already be properly merged if needed + // The merging with base theme happens in getThemeForMode() and other methods + // that prepare themes before passing them to applyTheme() this.globalTheme.setConfig(normalizedConfig); } catch (error) { console.error('Failed to apply theme:', error); diff --git a/superset/config.py b/superset/config.py index 662e79576e10..07d86420b9bb 100644 --- a/superset/config.py +++ b/superset/config.py @@ -36,7 +36,7 @@ from datetime import timedelta from email.mime.multipart import MIMEMultipart from importlib.resources import files -from typing import Any, Callable, Iterator, Literal, TYPE_CHECKING, TypedDict +from typing import Any, Callable, Iterator, Literal, Optional, TYPE_CHECKING, TypedDict import click from celery.schedules import crontab @@ -723,46 +723,69 @@ class D3TimeFormat(TypedDict, total=False): # This is merely a default EXTRA_CATEGORICAL_COLOR_SCHEMES: list[dict[str, Any]] = [] -# --------------------------------------------------- -# Theme Configuration for Superset -# --------------------------------------------------- +# ----------------------------------------------------------------------------- +# Theme System Configuration +# ----------------------------------------------------------------------------- # Superset supports custom theming through Ant Design's theme structure. -# This allows users to customize colors, fonts, and other UI elements. -# -# Theme Generation: -# - Use the Ant Design theme editor: https://ant.design/theme-editor -# - Export or copy the generated theme JSON and assign to the variables below -# - For detailed instructions: https://superset.apache.org/docs/configuration/theming/ # -# To expose a JSON theme editor modal that can be triggered from the navbar -# set the `ENABLE_THEME_EDITOR` feature flag to True. +# Theme Hierarchy: +# 1. THEME_DEFAULT/THEME_DARK - Base themes defined in config (foundation) +# 2. System themes - Set by admins via UI (when ENABLE_UI_THEME_ADMINISTRATION=True) +# 3. Dashboard themes - Applied per dashboard using the theme bolt button # -# Theme Structure: -# Each theme should follow Ant Design's theme format. -# To create custom themes, use the Ant Design Theme Editor at https://ant.design/theme-editor -# and copy the generated JSON configuration. +# How it works: +# - Custom themes override base themes for any properties they define +# - Properties not defined in custom themes use the base theme values +# - Admins can set system-wide themes that apply to all users +# - Users can apply specific themes to individual dashboards # -# Example theme definition: -# THEME_DEFAULT = { -# "token": { -# "colorPrimary": "#2893B3", -# "colorSuccess": "#5ac189", -# "colorWarning": "#fcc700", -# "colorError": "#e04355", -# "fontFamily": "'Inter', Helvetica, Arial", -# ... # other tokens -# }, -# ... # other theme properties -# } - - -# Default theme configuration -# Leave empty to use Superset's default theme -THEME_DEFAULT: Theme = {"algorithm": "default"} +# Theme Creation: +# - Use the Ant Design theme editor: https://ant.design/theme-editor +# - Export the generated JSON and use it in your theme configuration +# ----------------------------------------------------------------------------- + +# Default theme configuration - foundation for all themes +# This acts as the base theme for all users +THEME_DEFAULT: Theme = { + "token": { + # Brand + "brandLogoAlt": "Apache Superset", + "brandLogoUrl": APP_ICON, + "brandLogoMargin": "18px", + "brandLogoHref": "/", + "brandLogoHeight": "24px", + # Spinner + "brandSpinnerUrl": None, + "brandSpinnerSvg": None, + # Default colors + "colorPrimary": "#2893B3", # NOTE: previous lighter primary color was #20a7c9 # noqa: E501 + "colorLink": "#2893B3", + "colorError": "#e04355", + "colorWarning": "#fcc700", + "colorSuccess": "#5ac189", + "colorInfo": "#66bcfe", + # Fonts + "fontFamily": "'Inter', Helvetica, Arial", + "fontFamilyCode": "'Fira Code', 'Courier New', monospace", + # Extra tokens + "transitionTiming": 0.3, + "brandIconMaxWidth": 37, + "fontSizeXS": "8", + "fontSizeXXL": "28", + "fontWeightNormal": "400", + "fontWeightLight": "300", + "fontWeightStrong": 500, + }, + "algorithm": "default", +} -# Dark theme configuration -# Applied when user selects dark mode -THEME_DARK: Theme = {"algorithm": "dark"} +# Dark theme configuration - foundation for dark mode +# Inherits all tokens from THEME_DEFAULT and adds dark algorithm +# Set to None to disable dark mode +THEME_DARK: Optional[Theme] = { + **THEME_DEFAULT, + "algorithm": "dark", +} # Theme behavior and user preference settings # To force a single theme on all users, set THEME_DARK = None diff --git a/superset/daos/theme.py b/superset/daos/theme.py index 839b9f74f323..28f9cd28f9e1 100644 --- a/superset/daos/theme.py +++ b/superset/daos/theme.py @@ -32,11 +32,11 @@ def find_by_uuid(cls, uuid: str) -> Optional[Theme]: @classmethod def find_system_default(cls) -> Optional[Theme]: - """Find the current system default theme. - - First looks for a theme with is_system_default=True. - If not found or multiple found, falls back to is_system=True theme - with name 'THEME_DEFAULT'. + """ + Find the current system default theme. + Returns the theme with is_system_default=True if exactly one exists. + Returns None if no theme or multiple themes have + is_system_default=True. """ system_defaults = ( db.session.query(Theme).filter(Theme.is_system_default.is_(True)).all() @@ -45,27 +45,14 @@ def find_system_default(cls) -> Optional[Theme]: if len(system_defaults) == 1: return system_defaults[0] - if len(system_defaults) > 1: - logger.warning( - "Multiple system default themes found (%s), " - "falling back to config theme", - len(system_defaults), - ) - - # Fallback to is_system=True theme with name 'THEME_DEFAULT' - return ( - db.session.query(Theme) - .filter(Theme.is_system.is_(True), Theme.theme_name == "THEME_DEFAULT") - .first() - ) + return None @classmethod def find_system_dark(cls) -> Optional[Theme]: """Find the current system dark theme. - First looks for a theme with is_system_dark=True. - If not found or multiple found, falls back to is_system=True theme - with name 'THEME_DARK'. + Returns the theme with is_system_dark=True if exactly one exists. + Returns None if no theme or multiple themes have is_system_dark=True. """ system_darks = ( db.session.query(Theme).filter(Theme.is_system_dark.is_(True)).all() @@ -74,15 +61,4 @@ def find_system_dark(cls) -> Optional[Theme]: if len(system_darks) == 1: return system_darks[0] - if len(system_darks) > 1: - logger.warning( - "Multiple system dark themes found (%s), falling back to config theme", - len(system_darks), - ) - - # Fallback to is_system=True theme with name 'THEME_DARK' - return ( - db.session.query(Theme) - .filter(Theme.is_system.is_(True), Theme.theme_name == "THEME_DARK") - .first() - ) + return None diff --git a/superset/views/base.py b/superset/views/base.py index 1e6e12d2cc5d..8c40a0c436cb 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -21,7 +21,7 @@ import os import traceback from datetime import datetime -from typing import Any, Callable +from typing import Any, Callable, cast from babel import Locale from flask import ( @@ -60,8 +60,10 @@ from superset.db_engine_specs import get_available_engine_specs from superset.db_engine_specs.gsheets import GSheetsEngineSpec from superset.extensions import cache_manager +from superset.models.core import Theme as ThemeModel from superset.reports.models import ReportRecipientType from superset.superset_typing import FlaskResponse +from superset.themes.types import Theme from superset.themes.utils import ( is_valid_theme, ) @@ -311,6 +313,57 @@ def menu_data(user: User) -> dict[str, Any]: } +def _merge_theme_dicts(base: dict[str, Any], overlay: dict[str, Any]) -> dict[str, Any]: + """ + Recursively merge overlay theme dict into base theme dict. + Arrays and non-dict values are replaced, not merged. + """ + result = base.copy() + for key, value in overlay.items(): + if isinstance(result.get(key), dict) and isinstance(value, dict): + result[key] = _merge_theme_dicts(result[key], value) + else: + result[key] = value + return result + + +def _load_theme_from_model( + theme_model: ThemeModel | None, + fallback_theme: Theme | None, + theme_type: str, +) -> Theme | None: + """Load and parse theme from database model, merging with config theme as base.""" + if theme_model: + try: + db_theme = json.loads(theme_model.json_data) + if fallback_theme: + merged = _merge_theme_dicts(dict(fallback_theme), db_theme) + return cast(Theme, merged) + return db_theme + except json.JSONDecodeError: + logger.error( + "Invalid JSON in system %s theme %s", theme_type, theme_model.id + ) + return fallback_theme + return fallback_theme + + +def _process_theme(theme: Theme | None, theme_type: str) -> Theme: + """Process and validate a theme, returning an empty dict if invalid.""" + if theme is None or theme == {}: + # When config theme is None or empty, don't provide a custom theme + # The frontend will use base theme only + return {} + elif not is_valid_theme(cast(dict[str, Any], theme)): + logger.warning( + "Invalid %s theme configuration: %s, clearing it", + theme_type, + theme, + ) + return {} + return theme or {} + + def get_theme_bootstrap_data() -> dict[str, Any]: """ Returns the theme data to be sent to the client. @@ -318,59 +371,28 @@ def get_theme_bootstrap_data() -> dict[str, Any]: # Check if UI theme administration is enabled enable_ui_admin = app.config.get("ENABLE_UI_THEME_ADMINISTRATION", False) + # Get config themes to use as fallback + config_theme_default = get_config_value("THEME_DEFAULT") + config_theme_dark = get_config_value("THEME_DARK") + if enable_ui_admin: # Try to load themes from database default_theme_model = ThemeDAO.find_system_default() dark_theme_model = ThemeDAO.find_system_dark() # Parse theme JSON from database models - default_theme = {} - if default_theme_model: - try: - default_theme = json.loads(default_theme_model.json_data) - except json.JSONDecodeError: - logger.error( - "Invalid JSON in system default theme %s", - default_theme_model.id, - ) - # Fallback to config - default_theme = get_config_value("THEME_DEFAULT") - else: - # No system default theme in database, use config - default_theme = get_config_value("THEME_DEFAULT") - - dark_theme = {} - if dark_theme_model: - try: - dark_theme = json.loads(dark_theme_model.json_data) - except json.JSONDecodeError: - logger.error( - "Invalid JSON in system dark theme %s", dark_theme_model.id - ) - # Fallback to config - dark_theme = get_config_value("THEME_DARK") - else: - # No system dark theme in database, use config - dark_theme = get_config_value("THEME_DARK") - else: - # UI theme administration disabled, use config-based themes - default_theme = get_config_value("THEME_DEFAULT") - dark_theme = get_config_value("THEME_DARK") - - # Validate theme configurations - if not is_valid_theme(default_theme): - logger.warning( - "Invalid default theme configuration: %s, using empty theme", - default_theme, + default_theme = _load_theme_from_model( + default_theme_model, config_theme_default, "default" ) - default_theme = {} + dark_theme = _load_theme_from_model(dark_theme_model, config_theme_dark, "dark") + else: + # UI theme administration disabled - use config-based themes + default_theme = config_theme_default + dark_theme = config_theme_dark - if not is_valid_theme(dark_theme): - logger.warning( - "Invalid dark theme configuration: %s, using empty theme", - dark_theme, - ) - dark_theme = {} + # Process and validate themes + default_theme = _process_theme(default_theme, "default") + dark_theme = _process_theme(dark_theme, "dark") return { "theme": { diff --git a/tests/unit_tests/daos/test_theme_dao.py b/tests/unit_tests/daos/test_theme_dao.py index 54b4666a565f..bf748fbfb288 100644 --- a/tests/unit_tests/daos/test_theme_dao.py +++ b/tests/unit_tests/daos/test_theme_dao.py @@ -45,8 +45,7 @@ def test_find_system_default_single(self, mock_session): assert result == mock_theme @patch("superset.daos.theme.db.session") - @patch("superset.daos.theme.logger") - def test_find_system_default_multiple(self, mock_logger, mock_session): + def test_find_system_default_multiple(self, mock_session): """Test finding system default theme when multiple exist""" # Create mock themes mock_theme1 = MagicMock(spec=Theme) @@ -54,69 +53,33 @@ def test_find_system_default_multiple(self, mock_logger, mock_session): mock_theme2 = MagicMock(spec=Theme) mock_theme2.is_system_default = True - # Create mock fallback theme - mock_fallback = MagicMock(spec=Theme) - mock_fallback.is_system = True - mock_fallback.theme_name = "THEME_DEFAULT" - - # Mock the query chains - need separate mocks for each query call - mock_query1 = MagicMock() - mock_query2 = MagicMock() - mock_session.query.side_effect = [mock_query1, mock_query2] - - # First query returns multiple themes - mock_filter1 = MagicMock() - mock_query1.filter.return_value = mock_filter1 - mock_filter1.all.return_value = [mock_theme1, mock_theme2] - - # Second query returns fallback theme - mock_filter2 = MagicMock() - mock_query2.filter.return_value = mock_filter2 - mock_filter2.first.return_value = mock_fallback + # Mock the query chain + mock_query = MagicMock() + mock_session.query.return_value = mock_query + mock_filter = MagicMock() + mock_query.filter.return_value = mock_filter + mock_filter.all.return_value = [mock_theme1, mock_theme2] # Call the method result = ThemeDAO.find_system_default() - # Verify warning was logged with lazy logging format - mock_logger.warning.assert_called_once() - call_args = mock_logger.warning.call_args - assert ( - call_args[0][0] - == "Multiple system default themes found (%s), falling back to config theme" - ) - assert call_args[0][1] == 2 - - # Verify the result is the fallback theme - assert result == mock_fallback + assert result is None @patch("superset.daos.theme.db.session") def test_find_system_default_none(self, mock_session): """Test finding system default theme when none exist""" - # Create mock fallback theme - mock_fallback = MagicMock(spec=Theme) - mock_fallback.is_system = True - mock_fallback.theme_name = "THEME_DEFAULT" - - # Mock the query chains - need separate mocks for each query call - mock_query1 = MagicMock() - mock_query2 = MagicMock() - mock_session.query.side_effect = [mock_query1, mock_query2] - - # First query returns no themes - mock_filter1 = MagicMock() - mock_query1.filter.return_value = mock_filter1 - mock_filter1.all.return_value = [] - - # Second query returns fallback theme - mock_filter2 = MagicMock() - mock_query2.filter.return_value = mock_filter2 - mock_filter2.first.return_value = mock_fallback + # Mock the query chain + mock_query = MagicMock() + mock_session.query.return_value = mock_query + mock_filter = MagicMock() + mock_query.filter.return_value = mock_filter + mock_filter.all.return_value = [] # Call the method result = ThemeDAO.find_system_default() - # Verify the result is the fallback theme - assert result == mock_fallback + # Verify the result is None (no fallback) + assert result is None @patch("superset.daos.theme.db.session") def test_find_system_dark_single(self, mock_session): @@ -139,8 +102,7 @@ def test_find_system_dark_single(self, mock_session): assert result == mock_theme @patch("superset.daos.theme.db.session") - @patch("superset.daos.theme.logger") - def test_find_system_dark_multiple(self, mock_logger, mock_session): + def test_find_system_dark_multiple(self, mock_session): """Test finding system dark theme when multiple exist""" # Create mock themes mock_theme1 = MagicMock(spec=Theme) @@ -148,69 +110,33 @@ def test_find_system_dark_multiple(self, mock_logger, mock_session): mock_theme2 = MagicMock(spec=Theme) mock_theme2.is_system_dark = True - # Create mock fallback theme - mock_fallback = MagicMock(spec=Theme) - mock_fallback.is_system = True - mock_fallback.theme_name = "THEME_DARK" - - # Mock the query chains - need separate mocks for each query call - mock_query1 = MagicMock() - mock_query2 = MagicMock() - mock_session.query.side_effect = [mock_query1, mock_query2] - - # First query returns multiple themes - mock_filter1 = MagicMock() - mock_query1.filter.return_value = mock_filter1 - mock_filter1.all.return_value = [mock_theme1, mock_theme2] - - # Second query returns fallback theme - mock_filter2 = MagicMock() - mock_query2.filter.return_value = mock_filter2 - mock_filter2.first.return_value = mock_fallback + # Mock the query chain + mock_query = MagicMock() + mock_session.query.return_value = mock_query + mock_filter = MagicMock() + mock_query.filter.return_value = mock_filter + mock_filter.all.return_value = [mock_theme1, mock_theme2] # Call the method result = ThemeDAO.find_system_dark() - # Verify warning was logged with lazy logging format - mock_logger.warning.assert_called_once() - call_args = mock_logger.warning.call_args - assert ( - call_args[0][0] - == "Multiple system dark themes found (%s), falling back to config theme" - ) - assert call_args[0][1] == 2 - - # Verify the result is the fallback theme - assert result == mock_fallback + assert result is None @patch("superset.daos.theme.db.session") def test_find_system_dark_none_with_fallback(self, mock_session): - """Test finding system dark theme when none exist but fallback does""" - # Create mock fallback theme - mock_fallback = MagicMock(spec=Theme) - mock_fallback.is_system = True - mock_fallback.theme_name = "THEME_DARK" - - # Mock the query chains - need separate mocks for each query call - mock_query1 = MagicMock() - mock_query2 = MagicMock() - mock_session.query.side_effect = [mock_query1, mock_query2] - - # First query returns no themes - mock_filter1 = MagicMock() - mock_query1.filter.return_value = mock_filter1 - mock_filter1.all.return_value = [] - - # Second query returns fallback theme - mock_filter2 = MagicMock() - mock_query2.filter.return_value = mock_filter2 - mock_filter2.first.return_value = mock_fallback + """Test finding system dark theme when none exist""" + # Mock the query chain + mock_query = MagicMock() + mock_session.query.return_value = mock_query + mock_filter = MagicMock() + mock_query.filter.return_value = mock_filter + mock_filter.all.return_value = [] # Call the method result = ThemeDAO.find_system_dark() - # Verify the result is the fallback theme - assert result == mock_fallback + # Verify the result is None (no fallback) + assert result is None @patch("superset.daos.theme.db.session") def test_find_system_dark_none_without_fallback(self, mock_session): diff --git a/tests/unit_tests/views/test_base_theme_helpers.py b/tests/unit_tests/views/test_base_theme_helpers.py new file mode 100644 index 000000000000..7ef93505aa81 --- /dev/null +++ b/tests/unit_tests/views/test_base_theme_helpers.py @@ -0,0 +1,531 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from unittest.mock import MagicMock, patch + +from superset.views.base import ( + _load_theme_from_model, + _merge_theme_dicts, + _process_theme, + get_theme_bootstrap_data, +) + + +class TestThemeHelpers: + """Test theme helper functions in views/base.py""" + + def test_merge_theme_dicts_simple(self): + """Test merging simple theme dictionaries""" + base = {"token": {"colorPrimary": "#000"}} + overlay = {"token": {"colorPrimary": "#fff"}} + result = _merge_theme_dicts(base, overlay) + assert result == {"token": {"colorPrimary": "#fff"}} + + def test_merge_theme_dicts_nested(self): + """Test merging nested theme dictionaries""" + base = {"token": {"colorPrimary": "#000", "fontSize": 14}} + overlay = {"token": {"colorPrimary": "#fff"}} + result = _merge_theme_dicts(base, overlay) + assert result == {"token": {"colorPrimary": "#fff", "fontSize": 14}} + + def test_merge_theme_dicts_algorithm(self): + """Test merging theme with algorithm""" + base = {"token": {"colorPrimary": "#000"}, "algorithm": "default"} + overlay = {"algorithm": "dark"} + result = _merge_theme_dicts(base, overlay) + assert result == {"token": {"colorPrimary": "#000"}, "algorithm": "dark"} + + def test_merge_theme_dicts_arrays_replaced(self): + """Test that arrays are replaced, not merged by index""" + base = { + "token": {"colorPrimary": "#000"}, + "algorithm": ["default", "compact"], + "components": { + "Button": {"sizes": ["small", "medium", "large"]}, + }, + } + overlay = { + "algorithm": ["dark"], + "components": { + "Button": {"sizes": ["xs", "sm"]}, + }, + } + result = _merge_theme_dicts(base, overlay) + + # Arrays should be completely replaced, not merged + assert result["algorithm"] == ["dark"] # Not ["dark", "compact"] + assert result["components"]["Button"]["sizes"] == [ + "xs", + "sm", + ] # Not ["xs", "sm", "large"] + assert result["token"]["colorPrimary"] == "#000" # Preserved + + def test_merge_minimal_theme_preserves_base(self): + """Test that minimal theme overlay preserves all base tokens""" + # Simulate a full base theme from config + base_theme = { + "token": { + "colorPrimary": "#1890ff", + "colorSuccess": "#52c41a", + "colorWarning": "#faad14", + "colorError": "#f5222d", + "fontSize": 14, + "borderRadius": 6, + "wireframe": False, + "colorBgContainer": "#ffffff", + "colorText": "#000000", + }, + "algorithm": "default", + "components": { + "Button": {"colorPrimary": "#1890ff"}, + "Input": {"borderRadius": 4}, + }, + } + + # Minimal overlay theme (like from database) + minimal_overlay = { + "token": { + "colorPrimary": "#ff00ff", # Only override primary color + }, + "algorithm": "dark", # Change to dark mode + } + + result = _merge_theme_dicts(base_theme, minimal_overlay) + + # Should preserve all base tokens except the ones explicitly overridden + assert result["token"]["colorPrimary"] == "#ff00ff" # Overridden + assert result["token"]["colorSuccess"] == "#52c41a" # Preserved from base + assert result["token"]["colorWarning"] == "#faad14" # Preserved from base + assert result["token"]["colorError"] == "#f5222d" # Preserved from base + assert result["token"]["fontSize"] == 14 # Preserved from base + assert result["token"]["borderRadius"] == 6 # Preserved from base + assert result["token"]["wireframe"] is False # Preserved from base + assert result["token"]["colorBgContainer"] == "#ffffff" # Preserved from base + assert result["token"]["colorText"] == "#000000" # Preserved from base + assert result["algorithm"] == "dark" # Overridden + assert result["components"]["Button"]["colorPrimary"] == "#1890ff" # Preserved + assert result["components"]["Input"]["borderRadius"] == 4 # Preserved + + def test_merge_complete_theme_replaces_tokens(self): + """Test that complete theme overlay replaces all specified tokens""" + # Base theme from config + base_theme = { + "token": { + "colorPrimary": "#1890ff", + "colorSuccess": "#52c41a", + "colorWarning": "#faad14", + "fontSize": 14, + "borderRadius": 6, + }, + "algorithm": "default", + } + + # Complete overlay theme that redefines everything + complete_overlay = { + "token": { + "colorPrimary": "#ff0000", + "colorSuccess": "#00ff00", + "colorWarning": "#ffff00", + "fontSize": 16, + "borderRadius": 8, + # Adding new tokens not in base + "colorInfo": "#0000ff", + "lineHeight": 1.5, + }, + "algorithm": "dark", + "components": { + "Button": {"size": "large"}, + }, + } + + result = _merge_theme_dicts(base_theme, complete_overlay) + + # All overlay tokens should replace base tokens + assert result["token"]["colorPrimary"] == "#ff0000" + assert result["token"]["colorSuccess"] == "#00ff00" + assert result["token"]["colorWarning"] == "#ffff00" + assert result["token"]["fontSize"] == 16 + assert result["token"]["borderRadius"] == 8 + # New tokens should be added + assert result["token"]["colorInfo"] == "#0000ff" + assert result["token"]["lineHeight"] == 1.5 + # Algorithm should be replaced + assert result["algorithm"] == "dark" + # New components should be added + assert result["components"]["Button"]["size"] == "large" + + def test_load_theme_from_model_none(self): + """Test _load_theme_from_model with None model""" + fallback = {"token": {"colorPrimary": "#111"}} + result = _load_theme_from_model(None, fallback, "test") + assert result == fallback + + def test_load_theme_from_model_minimal_theme(self): + """Test _load_theme_from_model with minimal theme that merges with base""" + mock_model = MagicMock() + # Minimal theme from database - only overrides primary color + mock_model.json_data = '{"token": {"colorPrimary": "#ff00ff"}}' + mock_model.id = 1 + # Full base theme from config + fallback = { + "token": { + "colorPrimary": "#1890ff", + "colorSuccess": "#52c41a", + "colorWarning": "#faad14", + "fontSize": 14, + "borderRadius": 6, + }, + "algorithm": "default", + } + + result = _load_theme_from_model(mock_model, fallback, "test") + + # Should merge, preserving base tokens + assert result["token"]["colorPrimary"] == "#ff00ff" # From database + assert result["token"]["colorSuccess"] == "#52c41a" # From base + assert result["token"]["colorWarning"] == "#faad14" # From base + assert result["token"]["fontSize"] == 14 # From base + assert result["token"]["borderRadius"] == 6 # From base + assert result["algorithm"] == "default" # From base + + def test_load_theme_from_model_complete_theme(self): + """Test _load_theme_from_model with complete theme that replaces base tokens""" + mock_model = MagicMock() + # Complete theme from database - redefines all tokens + mock_model.json_data = """{ + "token": { + "colorPrimary": "#ff0000", + "colorSuccess": "#00ff00", + "colorWarning": "#ffff00", + "fontSize": 16, + "borderRadius": 8, + "colorInfo": "#0000ff" + }, + "algorithm": "dark" + }""" + mock_model.id = 1 + # Base theme from config + fallback = { + "token": { + "colorPrimary": "#1890ff", + "colorSuccess": "#52c41a", + "colorWarning": "#faad14", + "fontSize": 14, + "borderRadius": 6, + }, + "algorithm": "default", + } + + result = _load_theme_from_model(mock_model, fallback, "test") + + # All database tokens should replace base tokens + assert result["token"]["colorPrimary"] == "#ff0000" # From database + assert result["token"]["colorSuccess"] == "#00ff00" # From database + assert result["token"]["colorWarning"] == "#ffff00" # From database + assert result["token"]["fontSize"] == 16 # From database + assert result["token"]["borderRadius"] == 8 # From database + assert result["token"]["colorInfo"] == "#0000ff" # New from database + assert result["algorithm"] == "dark" # From database + + @patch("superset.views.base.logger") + def test_load_theme_from_model_invalid_json(self, mock_logger): + """Test _load_theme_from_model with invalid JSON""" + mock_model = MagicMock() + mock_model.json_data = "invalid json{" + mock_model.id = 1 + fallback = {"token": {"colorPrimary": "#111"}} + + result = _load_theme_from_model(mock_model, fallback, "test") + assert result == fallback + mock_logger.error.assert_called_once_with( + "Invalid JSON in system %s theme %s", "test", 1 + ) + + def test_process_theme_none(self): + """Test _process_theme with None theme""" + result = _process_theme(None, "test") + assert result == {} + + def test_process_theme_empty(self): + """Test _process_theme with empty theme""" + result = _process_theme({}, "test") + assert result == {} + + @patch("superset.views.base.is_valid_theme") + def test_process_theme_invalid(self, mock_is_valid): + """Test _process_theme with invalid theme""" + mock_is_valid.return_value = False + theme = {"invalid": "theme"} + + with patch("superset.views.base.logger") as mock_logger: + result = _process_theme(theme, "test") + assert result == {} + mock_logger.warning.assert_called_once_with( + "Invalid %s theme configuration: %s, clearing it", + "test", + theme, + ) + + @patch("superset.views.base.is_valid_theme") + def test_process_theme_valid(self, mock_is_valid): + """Test _process_theme with valid theme""" + mock_is_valid.return_value = True + theme = {"token": {"colorPrimary": "#444"}} + + result = _process_theme(theme, "test") + assert result == theme + + def test_process_theme_none_returns_empty(self): + """Test _process_theme with None returns empty dict""" + result = _process_theme(None, "test") + assert result == {} + + +class TestGetThemeBootstrapData: + """Test get_theme_bootstrap_data function with various scenarios""" + + @patch("superset.views.base.app") + @patch("superset.views.base.get_config_value") + @patch("superset.views.base.ThemeDAO") + def test_ui_admin_enabled_with_db_themes( + self, + mock_dao, + mock_get_config, + mock_app, + ): + """Test with UI admin enabled and themes in database""" + # Setup + mock_app.config = MagicMock() + mock_app.config.get.side_effect = lambda k, d=None: { + "ENABLE_UI_THEME_ADMINISTRATION": True, + "BASE_THEME_DEFAULT": {"token": {"colorPrimary": "#base1"}}, + "BASE_THEME_DARK": {"token": {"colorPrimary": "#base2"}}, + }.get(k, d) + + mock_get_config.side_effect = lambda k: { + "THEME_DEFAULT": {"token": {"colorPrimary": "#config1"}}, + "THEME_DARK": {"token": {"colorPrimary": "#config2"}}, + }.get(k) + + mock_default_theme = MagicMock() + mock_default_theme.json_data = '{"token": {"colorPrimary": "#db1"}}' + mock_dark_theme = MagicMock() + mock_dark_theme.json_data = '{"token": {"colorPrimary": "#db2"}}' + + mock_dao.find_system_default.return_value = mock_default_theme + mock_dao.find_system_dark.return_value = mock_dark_theme + + result = get_theme_bootstrap_data() + + # Verify + assert result["theme"]["enableUiThemeAdministration"] is True + assert "default" in result["theme"] + assert "dark" in result["theme"] + assert "baseThemeDefault" not in result["theme"] + assert "baseThemeDark" not in result["theme"] + + @patch("superset.views.base.app") + @patch("superset.views.base.get_config_value") + def test_ui_admin_disabled(self, mock_get_config, mock_app): + """Test with UI admin disabled, uses config themes""" + # Setup + mock_app.config = MagicMock() + mock_app.config.get.side_effect = lambda k, d=None: { + "ENABLE_UI_THEME_ADMINISTRATION": False, + "BASE_THEME_DEFAULT": {"token": {"colorPrimary": "#base1"}}, + "BASE_THEME_DARK": {"token": {"colorPrimary": "#base2"}}, + }.get(k, d) + + mock_get_config.side_effect = lambda k: { + "THEME_DEFAULT": {"token": {"colorPrimary": "#config1"}}, + "THEME_DARK": {"token": {"colorPrimary": "#config2"}}, + }.get(k) + + result = get_theme_bootstrap_data() + + # Verify + assert result["theme"]["enableUiThemeAdministration"] is False + assert result["theme"]["default"] == {"token": {"colorPrimary": "#config1"}} + assert result["theme"]["dark"] == {"token": {"colorPrimary": "#config2"}} + + @patch("superset.views.base.app") + @patch("superset.views.base.get_config_value") + @patch("superset.views.base.ThemeDAO") + def test_ui_admin_enabled_minimal_db_theme( + self, + mock_dao, + mock_get_config, + mock_app, + ): + """Test UI admin with minimal database theme overlaying config theme""" + # Setup + mock_app.config = MagicMock() + mock_app.config.get.side_effect = lambda k, d=None: { + "ENABLE_UI_THEME_ADMINISTRATION": True, + "BASE_THEME_DEFAULT": {"token": {"colorPrimary": "#base1"}}, + "BASE_THEME_DARK": {"token": {"colorPrimary": "#base2"}}, + }.get(k, d) + + # Full config themes with multiple tokens + mock_get_config.side_effect = lambda k: { + "THEME_DEFAULT": { + "token": { + "colorPrimary": "#1890ff", + "colorSuccess": "#52c41a", + "colorWarning": "#faad14", + "fontSize": 14, + }, + "algorithm": "default", + }, + "THEME_DARK": { + "token": { + "colorPrimary": "#1890ff", + "colorSuccess": "#52c41a", + "fontSize": 14, + }, + "algorithm": "dark", + }, + }.get(k) + + # Minimal database themes + mock_default_theme = MagicMock() + mock_default_theme.json_data = '{"token": {"colorPrimary": "#ff00ff"}}' + mock_dark_theme = MagicMock() + mock_dark_theme.json_data = ( + '{"token": {"colorWarning": "#orange"}, "algorithm": "dark"}' + ) + + mock_dao.find_system_default.return_value = mock_default_theme + mock_dao.find_system_dark.return_value = mock_dark_theme + + result = get_theme_bootstrap_data() + + # Verify merging behavior + assert result["theme"]["enableUiThemeAdministration"] is True + # Default theme should merge database with config + assert ( + result["theme"]["default"]["token"]["colorPrimary"] == "#ff00ff" + ) # From DB + assert ( + result["theme"]["default"]["token"]["colorSuccess"] == "#52c41a" + ) # From config + assert ( + result["theme"]["default"]["token"]["colorWarning"] == "#faad14" + ) # From config + assert result["theme"]["default"]["token"]["fontSize"] == 14 # From config + assert result["theme"]["default"]["algorithm"] == "default" # From config + + # Dark theme should merge database with config + assert ( + result["theme"]["dark"]["token"]["colorPrimary"] == "#1890ff" + ) # From config + assert result["theme"]["dark"]["token"]["colorWarning"] == "#orange" # From DB + assert result["theme"]["dark"]["algorithm"] == "dark" # From DB + + @patch("superset.views.base.app") + @patch("superset.views.base.get_config_value") + @patch("superset.views.base.ThemeDAO") + def test_ui_admin_enabled_no_db_themes( + self, + mock_dao, + mock_get_config, + mock_app, + ): + """Test UI admin enabled but no themes in database, falls back to config""" + # Setup + mock_app.config = MagicMock() + mock_app.config.get.side_effect = lambda k, d=None: { + "ENABLE_UI_THEME_ADMINISTRATION": True, + "BASE_THEME_DEFAULT": {"token": {"colorPrimary": "#base1"}}, + "BASE_THEME_DARK": {"token": {"colorPrimary": "#base2"}}, + }.get(k, d) + + mock_get_config.side_effect = lambda k: { + "THEME_DEFAULT": {"token": {"colorPrimary": "#config1"}}, + "THEME_DARK": {"token": {"colorPrimary": "#config2"}}, + }.get(k) + + # No database themes + mock_dao.find_system_default.return_value = None + mock_dao.find_system_dark.return_value = None + + result = get_theme_bootstrap_data() + + # Should fall back to config themes + assert result["theme"]["enableUiThemeAdministration"] is True + assert result["theme"]["default"] == {"token": {"colorPrimary": "#config1"}} + assert result["theme"]["dark"] == {"token": {"colorPrimary": "#config2"}} + + @patch("superset.views.base.app") + @patch("superset.views.base.get_config_value") + @patch("superset.views.base.ThemeDAO") + def test_ui_admin_enabled_invalid_db_theme( + self, + mock_dao, + mock_get_config, + mock_app, + ): + """Test UI admin with invalid JSON in database theme""" + # Setup + mock_app.config = MagicMock() + mock_app.config.get.side_effect = lambda k, d=None: { + "ENABLE_UI_THEME_ADMINISTRATION": True, + "BASE_THEME_DEFAULT": {"token": {"colorPrimary": "#base1"}}, + "BASE_THEME_DARK": {"token": {"colorPrimary": "#base2"}}, + }.get(k, d) + + mock_get_config.side_effect = lambda k: { + "THEME_DEFAULT": {"token": {"colorPrimary": "#config1"}}, + "THEME_DARK": {"token": {"colorPrimary": "#config2"}}, + }.get(k) + + # Invalid JSON in database theme + mock_default_theme = MagicMock() + mock_default_theme.json_data = "{invalid json" + mock_default_theme.id = 1 + + mock_dao.find_system_default.return_value = mock_default_theme + mock_dao.find_system_dark.return_value = None + + with patch("superset.views.base.logger") as mock_logger: + result = get_theme_bootstrap_data() + + # Should fall back to config theme and log error + assert result["theme"]["default"] == {"token": {"colorPrimary": "#config1"}} + mock_logger.error.assert_called_once() + + @patch("superset.views.base.app") + @patch("superset.views.base.get_config_value") + def test_ui_admin_disabled_no_config_themes(self, mock_get_config, mock_app): + """Test with UI admin disabled and no config themes (empty themes)""" + # Setup + mock_app.config = MagicMock() + mock_app.config.get.side_effect = lambda k, d=None: { + "ENABLE_UI_THEME_ADMINISTRATION": False, + "BASE_THEME_DEFAULT": {"token": {"colorPrimary": "#base1"}}, + "BASE_THEME_DARK": {"token": {"colorPrimary": "#base2"}}, + }.get(k, d) + + # No config themes (None values) + mock_get_config.side_effect = lambda k: None + + result = get_theme_bootstrap_data() + + # Should have empty theme objects + assert result["theme"]["enableUiThemeAdministration"] is False + assert result["theme"]["default"] == {} + assert result["theme"]["dark"] == {}