mistercrunch commented on code in PR #34144:
URL: https://github.com/apache/superset/pull/34144#discussion_r2205941823
##########
superset-frontend/src/theme/ThemeController.tsx:
##########
@@ -45,202 +81,618 @@ export class ThemeController {
private storage: ThemeStorage;
- private storageKey: string;
-
private modeStorageKey: string;
private defaultTheme: AnyThemeConfig;
- private systemMode: ThemeMode.DARK | ThemeMode.LIGHT;
+ private darkTheme: AnyThemeConfig | null;
- private currentMode: ThemeMode;
+ private themeSettings: SerializableThemeSettings;
- private customizations: AnyThemeConfig = {};
+ private systemMode: ThemeMode.DARK | ThemeMode.DEFAULT;
- private onChangeCallbacks: Set<(theme: Theme) => void> = new Set();
+ private currentMode: ThemeMode;
- private canUpdateThemeFn: () => boolean;
+ private readonly hasBootstrapThemes: boolean;
- private canUpdateModeFn: () => boolean;
+ private onChangeCallbacks: Set<(theme: Theme) => void> = new Set();
private mediaQuery: MediaQueryList;
- constructor(options: ThemeControllerOptions = { themeObject }) {
- this.storage = options.storage || new LocalStorageAdapter();
- this.storageKey = options.storageKey || 'superset-theme';
- this.modeStorageKey = options.modeStorageKey || `${this.storageKey}-mode`;
- this.defaultTheme = options.defaultTheme || {};
- this.themeObject = options.themeObject;
-
- // Load customizations from storage
- const savedThemeJson = this.storage.getItem(this.storageKey);
- if (savedThemeJson) {
- try {
- this.customizations = JSON.parse(savedThemeJson);
- } catch (e) {
- console.error('Failed to parse saved theme:', e);
- this.storage.removeItem(this.storageKey);
- }
+ constructor(options: ThemeControllerOptions = {}) {
+ const {
+ storage = new LocalStorageAdapter(),
+ modeStorageKey = STORAGE_KEYS.THEME_MODE,
+ themeObject: fallbackThemeObject = supersetThemeObject,
+ defaultTheme = (supersetThemeObject.theme as AnyThemeConfig) ?? {},
+ onChange = null,
+ } = options;
+
+ this.storage = storage;
+ this.modeStorageKey = modeStorageKey;
+
+ // Initialize bootstrap data and themes
+ const {
+ bootstrapDefaultTheme,
+ bootstrapDarkTheme,
+ bootstrapThemeSettings,
+ hasBootstrapThemes,
+ }: BootstrapThemeData = this.loadBootstrapData();
+
+ this.hasBootstrapThemes = hasBootstrapThemes;
+ this.themeSettings = bootstrapThemeSettings || {};
+
+ // Set themes based on bootstrap data availability
+ if (this.hasBootstrapThemes) {
+ this.darkTheme = bootstrapDarkTheme || bootstrapDefaultTheme || null;
+ this.defaultTheme =
+ bootstrapDefaultTheme || bootstrapDarkTheme || defaultTheme;
+ } else {
+ this.darkTheme = null;
+ this.defaultTheme = defaultTheme;
}
- // Determine initial mode
- this.systemMode = ThemeController.getSystemMode();
- const savedMode = this.storage.getItem(this.modeStorageKey) as ThemeMode;
- this.currentMode = savedMode || ThemeMode.SYSTEM;
+ // Initialize system theme detection
+ this.systemMode = ThemeController.getSystemPreferredMode();
- // Apply the initial theme and mode
- this.applyTheme();
+ // Only initialize media query listener if OS preference is allowed
+ if (this.shouldInitializeMediaQueryListener())
+ this.initializeMediaQueryListener();
- if (options.onChange) {
- this.onChangeCallbacks.add(options.onChange);
- }
- this.canUpdateThemeFn = options.canUpdateTheme || (() => true);
- this.canUpdateModeFn = options.canUpdateMode || (() => true);
+ // Initialize theme and mode
+ this.currentMode = this.determineInitialMode();
+ const { theme, themeObject } =
+ this.createInitialThemeObject(fallbackThemeObject);
- // Listen for system theme changes to enable dynamic `SYSTEM` mode
- this.mediaQuery = window.matchMedia('(prefers-color-scheme: dark)');
- this.mediaQuery.addEventListener('change', this.handleSystemThemeChange);
+ this.themeObject = themeObject;
+
+ // Setup change callback
+ if (onChange) this.onChangeCallbacks.add(onChange);
+
+ // Apply initial theme and persist mode
+ this.applyTheme(theme);
+ this.persistMode();
}
+ // Public Methods
+
/**
- * Cleans up listeners. Should be called when the controller is no longer
needed.
+ * Cleans up listeners and references. Should be called when the controller
is no longer needed.
*/
public destroy(): void {
this.mediaQuery.removeEventListener('change',
this.handleSystemThemeChange);
+ this.onChangeCallbacks.clear();
}
- public canUpdateTheme(): boolean {
- return this.canUpdateThemeFn();
+ /**
+ * Check if the user can update the theme.
+ */
+ public canSetTheme(): boolean {
+ return !this.themeSettings?.enforced;
}
- public canUpdateMode(): boolean {
- return this.canUpdateModeFn();
+ /**
+ * Check if the user can update the theme mode.
+ */
+ public canSetMode(): boolean {
+ return this.isModeUpdatable();
}
+ /**
+ * Returns the current theme object.
+ */
public getTheme(): Theme {
return this.themeObject;
}
+ /**
+ * Returns the current theme mode.
+ */
public getCurrentMode(): ThemeMode {
return this.currentMode;
}
/**
- * Sets new theme customizations (e.g., from a JSON editor).
- * This method updates the theme's appearance but preserves the current mode.
+ * Sets new theme.
+ * @param theme - The new theme to apply
+ * @throws {Error} If the user does not have permission to update the theme
*/
- public setTheme(newCustomizations: AnyThemeConfig): void {
- if (!this.canUpdateTheme()) {
- throw new Error('User does not have permission to update the theme');
- }
- this.customizations = newCustomizations;
+ public setTheme(theme: AnyThemeConfig): void {
+ this.validateThemeUpdatePermission();
- if (!newCustomizations.algorithm) {
- this.currentMode = ThemeMode.LIGHT;
- }
+ const { mode, normalizedTheme } = this.normalizeTheme(theme);
+ this.currentMode = mode;
+
+ this.updateTheme(normalizedTheme);
+ }
- if (newCustomizations?.algorithm) {
- this.currentMode = newCustomizations.algorithm as ThemeMode;
+ /**
+ * Sets the theme mode (light, dark, or system).
+ * @param mode - The new theme mode to apply
+ * @throws {Error} If the user does not have permission to update the theme
mode
+ */
+ public setThemeMode(mode: ThemeMode): void {
+ this.validateModeUpdatePermission(mode);
+
+ if (this.currentMode === mode) return;
+
+ const theme: AnyThemeConfig | null = this.getThemeForMode(mode);
+ if (!theme) {
+ console.warn(`Theme for mode ${mode} not found, falling back to
default`);
+ this.fallbackToDefaultMode();
+ return;
}
- this.applyTheme();
- this.persist();
- this.notifyListeners();
+ this.currentMode = mode;
+ this.updateTheme(theme);
}
/**
- * Changes the theme mode (light, dark, or system).
- * This is for the mode switch.
+ * Resets the theme to the default theme.
*/
- public changeThemeMode(newMode: ThemeMode): void {
- if (!this.canUpdateMode()) {
- throw new Error('User does not have permission to update the theme
mode');
+ public resetTheme(): void {
+ this.currentMode = ThemeMode.DEFAULT;
+ const defaultTheme: AnyThemeConfig =
+ this.getThemeForMode(ThemeMode.DEFAULT) || this.defaultTheme;
+
+ this.updateTheme(defaultTheme);
+ }
+
+ /**
+ * Handles system theme changes with error recovery.
+ */
+ private handleSystemThemeChange = (): void => {
+ try {
+ const newSystemMode: ThemeMode.DEFAULT | ThemeMode.DARK =
+ ThemeController.getSystemPreferredMode();
+
+ // Update systemMode regardless of current mode
+ const oldSystemMode: ThemeMode.DEFAULT | ThemeMode.DARK =
this.systemMode;
+ this.systemMode = newSystemMode;
+
+ // Only update theme if currently in SYSTEM mode and the preference
changed
+ if (
+ this.currentMode === ThemeMode.SYSTEM &&
+ oldSystemMode !== newSystemMode
+ ) {
+ const newTheme: AnyThemeConfig | null = this.getThemeForMode(
+ ThemeMode.SYSTEM,
+ );
+
+ if (newTheme) this.updateTheme(newTheme);
+ }
+ } catch (error) {
+ console.error('Failed to handle system theme change:', error);
}
- if (this.currentMode === newMode) return;
+ };
- this.currentMode = newMode;
+ /**
+ * Updates the theme.
+ * @param theme - The new theme to apply
+ */
+ 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;
- this.applyTheme();
- this.persist();
- this.notifyListeners();
+ // Normalize the theme
+ const { normalizedTheme } = this.normalizeTheme(config);
+
+ this.applyTheme(normalizedTheme);
+ this.persistMode();
+ this.notifyListeners();
+ } catch (error) {
+ console.error('Failed to update theme:', error);
+ this.fallbackToDefaultMode();
+ }
}
- public resetTheme(): void {
- this.customizations = this.defaultTheme;
- this.applyTheme();
- this.persist();
+ /**
+ * Fallback to default mode with error recovery.
+ */
+ private fallbackToDefaultMode(): void {
+ this.currentMode = ThemeMode.DEFAULT;
+
+ // Get the default theme which will have the correct algorithm
+ const defaultTheme: AnyThemeConfig =
+ this.getThemeForMode(ThemeMode.DEFAULT) || this.defaultTheme;
+
+ this.applyTheme(defaultTheme);
+ this.persistMode();
this.notifyListeners();
}
+ /**
+ * Registers a callback to be called when the theme changes.
+ * @param callback - The callback to be called on theme change
+ * @returns A function to unsubscribe from the theme change events
+ */
public onChange(callback: (theme: Theme) => void): () => void {
this.onChangeCallbacks.add(callback);
- return () => {
- this.onChangeCallbacks.delete(callback);
+ return () => this.onChangeCallbacks.delete(callback);
+ }
+
+ // Private Helper Methods
+
+ /**
+ * Determines whether the MediaQueryList listener for system theme changes
should be initialized.
+ * This checks if OS preference detection is enabled in the theme settings.
+ * @returns {boolean} True if the media query listener should be
initialized, false otherwise
+ */
+ private shouldInitializeMediaQueryListener(): boolean {
+ const { allowOSPreference = DEFAULT_THEME_SETTINGS.allowOSPreference } =
+ this.themeSettings || {};
+
+ return allowOSPreference === true;
+ }
+
+ /**
+ * 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);
+ }
+
+ /**
+ * Loads and validates bootstrap theme data.
+ */
+ private loadBootstrapData(): BootstrapThemeData {
+ const {
+ common: { theme = {} as BootstrapThemeDataConfig },
+ } = getBootstrapData();
+
+ const {
+ default: defaultTheme,
+ dark: darkTheme,
+ settings: themeSettings,
+ } = theme;
+
+ const hasValidDefault: boolean = this.hasKeys(defaultTheme);
+ const hasValidDark: boolean = this.hasKeys(darkTheme);
+ const hasValidSettings: boolean = this.hasKeys(themeSettings);
+
+ return {
+ bootstrapDefaultTheme: hasValidDefault ? defaultTheme : null,
+ bootstrapDarkTheme: hasValidDark ? darkTheme : null,
+ bootstrapThemeSettings: hasValidSettings ? themeSettings : null,
+ hasBootstrapThemes: hasValidDefault || hasValidDark,
};
}
- private handleSystemThemeChange = (): void => {
- const newSystemMode = ThemeController.getSystemMode();
- if (this.systemMode === newSystemMode) return;
+ /**
+ * Checks if an object has keys (not empty).
+ */
+ private hasKeys(obj: Record<string, any> | undefined | null): boolean {
+ return Boolean(
+ obj && typeof obj === 'object' && Object.keys(obj).length > 0,
+ );
+ }
- this.systemMode = newSystemMode;
- // If the current mode is SYSTEM, we need to re-apply the theme
- if (this.currentMode === ThemeMode.SYSTEM) {
- this.applyTheme();
- this.notifyListeners();
+ /**
+ * Determines if mode updates are allowed.
+ */
+ private isModeUpdatable(): boolean {
+ const {
+ enforced = DEFAULT_THEME_SETTINGS.enforced,
+ allowSwitching = DEFAULT_THEME_SETTINGS.allowSwitching,
+ } = this.themeSettings || {};
+
+ return !enforced && allowSwitching;
+ }
+
+ /**
+ * Normalizes the theme configuration to ensure it has a valid algorithm.
+ * @param theme - The theme configuration to normalize
+ * @returns An object with normalized mode and algorithm.
+ */
+ private normalizeTheme(theme: AnyThemeConfig): {
+ mode: ThemeMode;
+ normalizedTheme: AnyThemeConfig;
+ } {
+ const algorithm: ThemeAlgorithm | ThemeAlgorithm[] =
this.getValidAlgorithm(
+ (theme?.algorithm as ThemeAlgorithm | ThemeAlgorithm[]) ||
+ ThemeAlgorithm.DEFAULT,
+ );
+
+ // Extract the mode from the valid algorithm
+ let mode: ThemeMode;
+
+ if (Array.isArray(algorithm)) {
+ const foundAlgorithm =
+ algorithm.find(
+ (m: ThemeAlgorithm) =>
+ m === ThemeAlgorithm.DARK || m === ThemeAlgorithm.DEFAULT,
+ ) ?? ThemeAlgorithm.DEFAULT;
+
+ mode = this.algorithmToMode(foundAlgorithm);
+ } else mode = this.algorithmToMode(algorithm);
+
+ return {
+ mode,
+ normalizedTheme: {
+ ...theme,
+ algorithm,
+ } as AnyThemeConfig,
+ };
+ }
+
+ /**
+ * Converts a ThemeAlgorithm to its corresponding ThemeMode.
+ * @param algorithm - The theme algorithm to convert
+ * @returns The corresponding theme mode
+ */
+ private algorithmToMode(algorithm: ThemeAlgorithm): ThemeMode {
+ switch (algorithm) {
+ case ThemeAlgorithm.DARK:
+ return ThemeMode.DARK;
+ case ThemeAlgorithm.DEFAULT:
+ return ThemeMode.DEFAULT;
+ case ThemeAlgorithm.COMPACT:
+ return ThemeMode.DEFAULT;
+ default:
+ return ThemeMode.DEFAULT;
}
- };
+ }
+
+ /**
+ * Returns the appropriate theme configuration for a given mode.
+ * @param mode - The theme mode to get the configuration for
+ * @returns The theme configuration for the specified mode or null if not
available
+ */
+ private getThemeForMode(mode: ThemeMode): AnyThemeConfig | null {
+ const { allowOSPreference = DEFAULT_THEME_SETTINGS.allowOSPreference } =
+ this.themeSettings;
+
+ let resolvedMode: ThemeMode = mode;
+
+ if (mode === ThemeMode.SYSTEM) {
+ if (!allowOSPreference) return null;
+ resolvedMode = ThemeController.getSystemPreferredMode();
+ }
+
+ // When we don't have bootstrap themes, we need to create variants using
algorithm
+ if (!this.hasBootstrapThemes) {
+ if (resolvedMode === ThemeMode.DARK)
+ return {
+ ...this.defaultTheme,
+ algorithm: ThemeAlgorithm.DARK,
+ };
+
+ return {
+ ...this.defaultTheme,
+ algorithm: ThemeAlgorithm.DEFAULT,
+ };
+ }
+
+ // When we have bootstrap themes, use them
+ if (resolvedMode === ThemeMode.DARK)
+ return this.darkTheme || this.defaultTheme;
+
+ return this.defaultTheme;
+ }
+
+ /**
+ * 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),
+ };
+ }
+
+ /**
+ * Determines the initial theme mode with error recovery.
+ */
+ private determineInitialMode(): ThemeMode {
+ const {
+ enforced = DEFAULT_THEME_SETTINGS.enforced,
+ allowOSPreference = DEFAULT_THEME_SETTINGS.allowOSPreference,
+ allowSwitching = DEFAULT_THEME_SETTINGS.allowSwitching,
+ } = this.themeSettings;
+
+ // Enforced mode always takes precedence
+ if (enforced) {
+ this.storage.removeItem(this.modeStorageKey);
+ return ThemeMode.DEFAULT;
+ }
+
+ // When OS preference is allowed but switching is not
+ // This means the user MUST follow OS preference and cannot override it
+ if (allowOSPreference && !allowSwitching) {
+ // Clear any saved preference since switching is not allowed
+ this.storage.removeItem(this.modeStorageKey);
+ return ThemeMode.SYSTEM;
+ }
+
+ // Try to restore saved mode
+ const savedMode: ThemeMode | null = this.loadSavedMode();
+ if (savedMode && this.isValidThemeMode(savedMode)) return savedMode;
+
+ // Fallback to system preference if allowed and available
+ if (allowOSPreference && this.getThemeForMode(this.systemMode))
+ return ThemeMode.SYSTEM;
+
+ return ThemeMode.DEFAULT;
+ }
+
+ /**
+ * Safely loads saved theme mode from storage.
+ */
+ private loadSavedMode(): ThemeMode | null {
+ try {
+ const stored: string | null = this.storage.getItem(this.modeStorageKey);
+ if (stored && Object.values(ThemeMode).includes(stored as ThemeMode))
+ return stored as ThemeMode;
+
+ return null;
+ } catch (error) {
+ console.warn('Failed to load saved theme mode:', error);
+ return null;
+ }
+ }
/**
- * Centralized method to apply the current customizations and mode.
+ * Validates if a theme mode is valid and supported.
+ * This checks if the mode is one of the known ThemeMode values.
+ * @param mode - The theme mode to validate
+ * @returns {boolean} True if the mode is valid, false otherwise
*/
- private applyTheme(): void {
- const newConfig = { ...this.customizations };
+ private isValidThemeMode(mode: ThemeMode): boolean {
+ if (!Object.values(ThemeMode).includes(mode)) return false;
- switch (this.currentMode) {
+ // Validate that we have the required theme data for the mode
+ switch (mode) {
case ThemeMode.DARK:
- newConfig.algorithm = 'dark';
- break;
- case ThemeMode.LIGHT:
- newConfig.algorithm = 'default';
- break;
+ return !!(this.darkTheme || this.defaultTheme);
+ case ThemeMode.DEFAULT:
+ return !!this.defaultTheme;
case ThemeMode.SYSTEM:
- newConfig.algorithm =
- this.systemMode === ThemeMode.DARK ? 'dark' : 'default';
- break;
- case ThemeMode.COMPACT:
- newConfig.algorithm = 'compact';
- break;
+ return this.themeSettings?.allowOSPreference !== false;
default:
- newConfig.algorithm = 'default';
- break;
+ return true;
}
+ }
- this.themeObject.setConfig(newConfig);
+ /**
+ * Validates permission to update theme.
+ */
+ private validateThemeUpdatePermission(): void {
+ if (!this.canSetTheme())
+ throw new Error('User does not have permission to update the theme');
}
- private persist(): void {
- this.storage.setItem(this.modeStorageKey, this.currentMode);
+ /**
+ * Validates permission to update mode.
+ * @param newMode - The new mode to validate
+ * @throws {Error} If the user does not have permission to update the theme
mode
+ * @throws {Error} If the new mode is SYSTEM and OS preference is not allowed
+ */
+ private validateModeUpdatePermission(newMode: ThemeMode): void {
+ const {
+ allowOSPreference = DEFAULT_THEME_SETTINGS.allowOSPreference,
+ allowSwitching = DEFAULT_THEME_SETTINGS.allowSwitching,
+ } = this.themeSettings;
+
+ // If OS preference is allowed but switching is not,
+ // don't allow any mode changes
+ if (allowOSPreference && !allowSwitching)
+ throw new Error(
+ 'Theme mode changes are not allowed when OS preference is enforced',
+ );
+
+ // Check if user can set a new theme mode
+ if (!this.canSetMode())
+ throw new Error('User does not have permission to update the theme
mode');
+
+ // Check if user has permissions to set OS preference as a theme mode
+ if (newMode === ThemeMode.SYSTEM && !allowOSPreference)
+ throw new Error('System theme mode is not allowed');
+ }
- const { algorithm, ...persistedCustomizations } = this.customizations;
- this.storage.setItem(
- this.storageKey,
- JSON.stringify(persistedCustomizations),
+ /**
+ * Validates theme algorithm combinations.
+ * This checks if the provided algorithm is a valid combination of ThemeMode
values.
+ * @param algorithm - The theme algorithm to validate
+ * @returns {boolean} True if the algorithms combination is valid, false
otherwise
+ */
+ private isValidAlgorithmCombination(algorithm: ThemeAlgorithm[]): boolean {
+ const inputSet = new Set(algorithm);
+ return VALID_ALGORITHM_COMBINATIONS.some(
+ validCombination =>
+ inputSet.size === validCombination.size &&
+ [...inputSet].every(item => validCombination.has(item)),
);
}
+ /**
+ * Checks if the algorithm is a valid combination or a simple one.
+ * @param algorithm - The theme mode or combination to convert
+ * @returns A valid ThemeAlgorithm or ThemeAlgorithm[]
+ */
+ private getValidAlgorithm(
Review Comment:
Similar to above comment, this looks like dup logic that probably either
already exists or belongs directly in Theme or utils
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
