diff --git a/superset-frontend/src/theme/ThemeController.ts b/superset-frontend/src/theme/ThemeController.ts index d3672092a3c..b5c327393b4 100644 --- a/superset-frontend/src/theme/ThemeController.ts +++ b/superset-frontend/src/theme/ThemeController.ts @@ -101,13 +101,14 @@ export class ThemeController { const { storage = new LocalStorageAdapter(), modeStorageKey = STORAGE_KEYS.THEME_MODE, - themeObject: fallbackThemeObject = supersetThemeObject, + themeObject = supersetThemeObject, defaultTheme = (supersetThemeObject.theme as AnyThemeConfig) ?? {}, onChange = null, } = options; this.storage = storage; this.modeStorageKey = modeStorageKey; + this.themeObject = themeObject; // Initialize bootstrap data and themes const { @@ -139,16 +140,14 @@ export class ThemeController { // Initialize theme and mode this.currentMode = this.determineInitialMode(); - const { theme, themeObject } = - this.createInitialThemeObject(fallbackThemeObject); - - this.themeObject = themeObject; + const initialTheme = + this.getThemeForMode(this.currentMode) || this.defaultTheme; // Setup change callback if (onChange) this.onChangeCallbacks.add(onChange); // Apply initial theme and persist mode - this.applyTheme(theme); + this.applyTheme(initialTheme); this.persistMode(); } @@ -158,7 +157,12 @@ export class ThemeController { * Cleans up listeners and references. Should be called when the controller is no longer needed. */ public destroy(): void { - this.mediaQuery.removeEventListener('change', this.handleSystemThemeChange); + if (this.mediaQuery) + this.mediaQuery.removeEventListener( + 'change', + this.handleSystemThemeChange, + ); + this.onChangeCallbacks.clear(); } @@ -329,8 +333,12 @@ export class ThemeController { * Initializes media query listeners if OS preference is allowed */ private initializeMediaQueryListener(): void { - this.mediaQuery = window.matchMedia(MEDIA_QUERY_DARK_SCHEME); - this.mediaQuery.addEventListener('change', this.handleSystemThemeChange); + try { + this.mediaQuery = window.matchMedia(MEDIA_QUERY_DARK_SCHEME); + this.mediaQuery.addEventListener('change', this.handleSystemThemeChange); + } catch (error) { + console.warn('Failed to initialize media query listener:', error); + } } /** @@ -347,9 +355,9 @@ export class ThemeController { settings: themeSettings, } = theme; - const hasValidDefault: boolean = this.hasKeys(defaultTheme); - const hasValidDark: boolean = this.hasKeys(darkTheme); - const hasValidSettings: boolean = this.hasKeys(themeSettings); + const hasValidDefault: boolean = this.isNonEmptyObject(defaultTheme); + const hasValidDark: boolean = this.isNonEmptyObject(darkTheme); + const hasValidSettings: boolean = this.isNonEmptyObject(themeSettings); return { bootstrapDefaultTheme: hasValidDefault ? defaultTheme : null, @@ -360,9 +368,11 @@ export class ThemeController { } /** - * Checks if an object has keys (not empty). + * Checks if an object is non-empty (has at least one property). */ - private hasKeys(obj: Record | undefined | null): boolean { + private isNonEmptyObject( + obj: Record | undefined | null, + ): boolean { return Boolean( obj && typeof obj === 'object' && Object.keys(obj).length > 0, ); @@ -417,28 +427,7 @@ export class ThemeController { ? this.darkTheme || this.defaultTheme : this.defaultTheme; - return normalizeThemeConfig(selectedTheme); - } - - /** - * Creates the initial theme object. - * This sets the theme based on the current mode and ensures it has the correct algorithm. - * @param defaultThemeObject - The fallback theme object to use if no theme is set - * @returns An object containing the theme and the themeObject - */ - private createInitialThemeObject(defaultThemeObject: Theme): { - theme: AnyThemeConfig; - themeObject: Theme; - } { - let theme: AnyThemeConfig | null = this.getThemeForMode(this.currentMode); - theme = theme || (defaultThemeObject.theme as AnyThemeConfig); - - const normalizedTheme = this.normalizeTheme(theme); - - return { - theme: normalizedTheme, - themeObject: Theme.fromConfig(normalizedTheme), - }; + return selectedTheme; } /** diff --git a/superset-frontend/src/theme/tests/ThemeController.test.ts b/superset-frontend/src/theme/tests/ThemeController.test.ts index 66ff00bd274..cc242e849dc 100644 --- a/superset-frontend/src/theme/tests/ThemeController.test.ts +++ b/superset-frontend/src/theme/tests/ThemeController.test.ts @@ -230,13 +230,13 @@ describe('ThemeController', () => { expect(controller.getTheme()).toBe(mockThemeObject); }); - it('should use BootsrapData themes when available', () => { + it('should use BootstrapData themes when available', () => { controller = new ThemeController({ themeObject: mockThemeObject, }); - expect(mockThemeFromConfig).toHaveBeenCalledTimes(1); - expect(mockThemeFromConfig).toHaveBeenCalledWith( + expect(mockSetConfig).toHaveBeenCalledTimes(1); + expect(mockSetConfig).toHaveBeenCalledWith( expect.objectContaining({ token: expect.objectContaining({ colorBgBase: '#ededed', @@ -246,7 +246,7 @@ describe('ThemeController', () => { ); }); - it('should fallback to Superset default theme when BootsrapData themes are empty', () => { + it('should fallback to Superset default theme when BootstrapData themes are empty', () => { mockGetBootstrapData.mockReturnValue( createMockBootstrapData({ default: {}, @@ -267,8 +267,8 @@ describe('ThemeController', () => { defaultTheme: fallbackTheme, }); - expect(mockThemeFromConfig).toHaveBeenCalledTimes(1); - expect(mockThemeFromConfig).toHaveBeenCalledWith( + expect(mockSetConfig).toHaveBeenCalledTimes(1); + expect(mockSetConfig).toHaveBeenCalledWith( expect.objectContaining({ ...fallbackTheme, algorithm: antdThemeImport.defaultAlgorithm,