This is an automated email from the ASF dual-hosted git repository.
msyavuz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new bbb2279644 fix(theming): Superset theme configurations correctly
applying to charts (#34218)
bbb2279644 is described below
commit bbb227964430ea0b2a01bbe29787d338cfbc9cca
Author: Gabriel Torres Ruiz <[email protected]>
AuthorDate: Fri Jul 18 10:25:48 2025 -0300
fix(theming): Superset theme configurations correctly applying to charts
(#34218)
---
superset-frontend/src/theme/ThemeController.ts | 61 +++++++++-------------
.../src/theme/tests/ThemeController.test.ts | 12 ++---
2 files changed, 31 insertions(+), 42 deletions(-)
diff --git a/superset-frontend/src/theme/ThemeController.ts
b/superset-frontend/src/theme/ThemeController.ts
index d3672092a3..b5c327393b 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<string, any> | undefined | null): boolean {
+ private isNonEmptyObject(
+ obj: Record<string, any> | 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 66ff00bd27..cc242e849d 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,
