refactor: couple appearance mode with theme selection (#552)

datlechin · web-flow · commit 0c3738cedb6d · 2026-04-02T10:56:57.000+07:00
* refactor: couple appearance mode with theme selection (#550) * fix: address code review findings from PR #552 * refactor: remove backward compatibility migration from AppearanceSettings * fix: correct system appearance detection and theme slot assignment * fix: handle .auto theme appearance, remove debug logging * docs: simplify changelog entry * docs: rewrite appearance mode section for clarity
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Option to group all connection tabs in one window instead of separate windows per connection
 
+### Changed
+
+- Separate preferred themes for Light and Dark appearance modes, with automatic switching in Auto mode
+
 ## [0.27.1] - 2026-04-01
 
 ### Fixed
diff --git a/TablePro/Core/Storage/AppSettingsManager.swift b/TablePro/Core/Storage/AppSettingsManager.swift
@@ -36,8 +36,11 @@ final class AppSettingsManager {
     var appearance: AppearanceSettings {
         didSet {
             storage.saveAppearance(appearance)
-            ThemeEngine.shared.activateTheme(id: appearance.activeThemeId)
-            ThemeEngine.shared.updateAppearanceMode(appearance.appearanceMode)
+            ThemeEngine.shared.updateAppearanceAndTheme(
+                mode: appearance.appearanceMode,
+                lightThemeId: appearance.preferredLightThemeId,
+                darkThemeId: appearance.preferredDarkThemeId
+            )
             SyncChangeTracker.shared.markDirty(.settings, id: "appearance")
         }
     }
@@ -157,9 +160,12 @@ final class AppSettingsManager {
         // Apply language immediately
         general.language.apply()
 
-        // ThemeEngine initializes itself from persisted theme ID
-        // Apply app-level appearance mode
-        ThemeEngine.shared.updateAppearanceMode(appearance.appearanceMode)
+        // Activate the correct theme based on appearance mode + preferred themes
+        ThemeEngine.shared.updateAppearanceAndTheme(
+            mode: appearance.appearanceMode,
+            lightThemeId: appearance.preferredLightThemeId,
+            darkThemeId: appearance.preferredDarkThemeId
+        )
 
         // Sync editor behavioral settings to ThemeEngine
         ThemeEngine.shared.updateEditorSettings(
diff --git a/TablePro/Models/Settings/AppSettings.swift b/TablePro/Models/Settings/AppSettings.swift
@@ -101,7 +101,7 @@ struct GeneralSettings: Codable, Equatable {
 
 // MARK: - Appearance Settings
 
-/// Controls NSApp.appearance independent of the active theme.
+/// Controls which appearance the app uses: forced light, forced dark, or follow system.
 enum AppAppearanceMode: String, Codable, CaseIterable {
     case light
     case dark
@@ -116,46 +116,41 @@ enum AppAppearanceMode: String, Codable, CaseIterable {
     }
 }
 
-/// Appearance settings
+/// Appearance settings — couples appearance mode with theme selection.
+/// Each appearance (light/dark) has its own preferred theme so the active theme
+/// always matches the window chrome.
 struct AppearanceSettings: Codable, Equatable {
-    var activeThemeId: String
     var appearanceMode: AppAppearanceMode
+    var preferredLightThemeId: String
+    var preferredDarkThemeId: String
 
     static let `default` = AppearanceSettings(
-        activeThemeId: "tablepro.default-light",
-        appearanceMode: .auto
+        appearanceMode: .auto,
+        preferredLightThemeId: "tablepro.default-light",
+        preferredDarkThemeId: "tablepro.default-dark"
     )
 
-    init(activeThemeId: String = "tablepro.default-light", appearanceMode: AppAppearanceMode = .auto) {
-        self.activeThemeId = activeThemeId
+    init(
+        appearanceMode: AppAppearanceMode = .auto,
+        preferredLightThemeId: String = "tablepro.default-light",
+        preferredDarkThemeId: String = "tablepro.default-dark"
+    ) {
         self.appearanceMode = appearanceMode
+        self.preferredLightThemeId = preferredLightThemeId
+        self.preferredDarkThemeId = preferredDarkThemeId
     }
 
     init(from decoder: Decoder) throws {
         let container = try decoder.container(keyedBy: CodingKeys.self)
-        // Migration: try new field first, then fall back to old theme field
-        if let themeId = try container.decodeIfPresent(String.self, forKey: .activeThemeId) {
-            activeThemeId = themeId
-        } else if let oldTheme = try? container.decodeIfPresent(String.self, forKey: .theme) {
-            // Migrate from old AppTheme enum
-            switch oldTheme {
-            case "dark": activeThemeId = "tablepro.default-dark"
-            default: activeThemeId = "tablepro.default-light"
-            }
-        } else {
-            activeThemeId = "tablepro.default-light"
-        }
         appearanceMode = try container.decodeIfPresent(AppAppearanceMode.self, forKey: .appearanceMode) ?? .auto
+        preferredLightThemeId = try container.decodeIfPresent(String.self, forKey: .preferredLightThemeId)
+            ?? "tablepro.default-light"
+        preferredDarkThemeId = try container.decodeIfPresent(String.self, forKey: .preferredDarkThemeId)
+            ?? "tablepro.default-dark"
     }
 
     private enum CodingKeys: String, CodingKey {
-        case activeThemeId, theme, appearanceMode
-    }
-
-    func encode(to encoder: Encoder) throws {
-        var container = encoder.container(keyedBy: CodingKeys.self)
-        try container.encode(activeThemeId, forKey: .activeThemeId)
-        try container.encode(appearanceMode, forKey: .appearanceMode)
+        case appearanceMode, preferredLightThemeId, preferredDarkThemeId
     }
 }
 
diff --git a/TablePro/Theme/ThemeEngine.swift b/TablePro/Theme/ThemeEngine.swift
@@ -109,8 +109,9 @@ internal final class ThemeEngine {
 
     private init() {
         let allThemes = ThemeStorage.loadAllThemes()
-        let activeId = ThemeStorage.loadActiveThemeId()
-        let theme = allThemes.first { $0.id == activeId } ?? .default
+        // Start with the default theme; AppSettingsManager.init() will call
+        // updateAppearanceAndTheme() to activate the correct preferred theme.
+        let theme = ThemeDefinition.default
 
         self.activeTheme = theme
         self.colors = ResolvedThemeColors(from: theme)
@@ -140,7 +141,6 @@ internal final class ThemeEngine {
         editorFonts = EditorFontCache(from: theme.fonts)
         dataGridFonts = DataGridFontCacheResolved(from: theme.fonts)
 
-        ThemeStorage.saveActiveThemeId(theme.id)
         notifyThemeDidChange()
 
         Self.logger.info("Activated theme: \(theme.name) (\(theme.id))")
@@ -163,9 +163,27 @@ internal final class ThemeEngine {
         try ThemeStorage.deleteUserTheme(id: id)
         reloadAvailableThemes()
 
-        // If deleted the active theme, fall back to default
-        if id == activeTheme.id {
-            activateTheme(id: "tablepro.default-light")
+        // If deleted a preferred theme, reset that slot to default
+        var appearance = AppSettingsManager.shared.appearance
+        var changed = false
+        if id == appearance.preferredLightThemeId {
+            appearance.preferredLightThemeId = "tablepro.default-light"
+            changed = true
+        }
+        if id == appearance.preferredDarkThemeId {
+            appearance.preferredDarkThemeId = "tablepro.default-dark"
+            changed = true
+        }
+        if changed {
+            AppSettingsManager.shared.appearance = appearance
+        } else if id == activeTheme.id {
+            // Deleted a non-preferred but currently active theme — re-anchor to preferred
+            let appearance = AppSettingsManager.shared.appearance
+            updateAppearanceAndTheme(
+                mode: appearance.appearanceMode,
+                lightThemeId: appearance.preferredLightThemeId,
+                darkThemeId: appearance.preferredDarkThemeId
+            )
         }
     }
 
@@ -209,6 +227,11 @@ internal final class ThemeEngine {
         activeTheme = theme
         editorFonts = EditorFontCache(from: theme.fonts)
         notifyThemeDidChange()
+
+        // Persist so the zoom survives re-activation (e.g. system appearance change)
+        if theme.isEditable {
+            try? ThemeStorage.saveUserTheme(theme)
+        }
     }
 
     // MARK: - Font Cache Reload (accessibility)
@@ -273,13 +296,51 @@ internal final class ThemeEngine {
     // MARK: - Appearance
 
     @ObservationIgnored private(set) var appearanceMode: AppAppearanceMode = .auto
-
-    func updateAppearanceMode(_ mode: AppAppearanceMode) {
+    private(set) var effectiveAppearance: ThemeAppearance = .light
+    @ObservationIgnored private var currentLightThemeId: String = "tablepro.default-light"
+    @ObservationIgnored private var currentDarkThemeId: String = "tablepro.default-dark"
+    @ObservationIgnored private var systemAppearanceObserver: NSObjectProtocol?
+
+    /// Central entry point: resolves effective appearance, picks the correct theme, activates it,
+    /// and derives NSApp.appearance from the theme's own appearance metadata.
+    func updateAppearanceAndTheme(
+        mode: AppAppearanceMode,
+        lightThemeId: String,
+        darkThemeId: String
+    ) {
         appearanceMode = mode
-        applyAppearance(mode)
+        currentLightThemeId = lightThemeId
+        currentDarkThemeId = darkThemeId
+
+        let resolved = resolveEffectiveAppearance(mode)
+        effectiveAppearance = resolved
+
+        let themeId = resolved == .dark ? darkThemeId : lightThemeId
+        activateTheme(id: themeId)
+        applyNSAppAppearance(mode: mode)
+
+        updateSystemAppearanceObserver(mode: mode)
     }
 
-    private func applyAppearance(_ mode: AppAppearanceMode) {
+    /// Resolve which appearance is in effect right now.
+    private func resolveEffectiveAppearance(_ mode: AppAppearanceMode) -> ThemeAppearance {
+        switch mode {
+        case .light: return .light
+        case .dark: return .dark
+        case .auto: return systemIsDark() ? .dark : .light
+        }
+    }
+
+    /// Check if the system is currently in dark mode.
+    /// Reads the global `AppleInterfaceStyle` default directly so we get the real
+    /// system setting, not the app's own forced appearance.
+    private func systemIsDark() -> Bool {
+        UserDefaults.standard.string(forKey: "AppleInterfaceStyle") == "Dark"
+    }
+
+    /// Set NSApp.appearance based on the appearance mode (not the theme).
+    /// Auto mode sets nil so the system controls the chrome.
+    private func applyNSAppAppearance(mode: AppAppearanceMode) {
         switch mode {
         case .light:
             NSApp?.appearance = NSAppearance(named: .aqua)
@@ -290,6 +351,35 @@ internal final class ThemeEngine {
         }
     }
 
+    // MARK: - System Appearance Observer
+
+    private func updateSystemAppearanceObserver(mode: AppAppearanceMode) {
+        // Remove existing observer
+        if let observer = systemAppearanceObserver {
+            DistributedNotificationCenter.default().removeObserver(observer)
+            systemAppearanceObserver = nil
+        }
+
+        guard mode == .auto else { return }
+
+        // Install observer for system appearance changes
+        systemAppearanceObserver = DistributedNotificationCenter.default().addObserver(
+            forName: Notification.Name("AppleInterfaceThemeChangedNotification"),
+            object: nil,
+            queue: .main
+        ) { [weak self] _ in
+            Task { @MainActor [weak self] in
+                guard let self, self.appearanceMode == .auto else { return }
+                let newAppearance = self.systemIsDark() ? ThemeAppearance.dark : ThemeAppearance.light
+                guard newAppearance != self.effectiveAppearance else { return }
+                self.effectiveAppearance = newAppearance
+                let themeId = newAppearance == .dark ? self.currentDarkThemeId : self.currentLightThemeId
+                self.activateTheme(id: themeId)
+                self.applyNSAppAppearance(mode: .auto)
+            }
+        }
+    }
+
     // MARK: - Notifications
 
     private func notifyThemeDidChange() {
diff --git a/TablePro/Theme/ThemeRegistryInstaller.swift b/TablePro/Theme/ThemeRegistryInstaller.swift
@@ -61,9 +61,21 @@ internal final class ThemeRegistryInstaller {
 
         ThemeEngine.shared.reloadAvailableThemes()
 
-        // Fall back if the active theme was uninstalled
-        if removedThemeIds.contains(ThemeEngine.shared.activeTheme.id) {
-            ThemeEngine.shared.activateTheme(id: "tablepro.default-light")
+        // Reset preferred theme slots if the uninstalled theme was preferred
+        var appearance = AppSettingsManager.shared.appearance
+        var changed = false
+        for id in removedThemeIds {
+            if id == appearance.preferredLightThemeId {
+                appearance.preferredLightThemeId = "tablepro.default-light"
+                changed = true
+            }
+            if id == appearance.preferredDarkThemeId {
+                appearance.preferredDarkThemeId = "tablepro.default-dark"
+                changed = true
+            }
+        }
+        if changed {
+            AppSettingsManager.shared.appearance = appearance
         }
 
         Self.logger.info("Uninstalled registry themes for plugin: \(registryPluginId)")
@@ -102,9 +114,13 @@ internal final class ThemeRegistryInstaller {
         // Single reload after swap is complete — no intermediate flicker
         ThemeEngine.shared.reloadAvailableThemes()
 
-        if ThemeEngine.shared.availableThemes.contains(where: { $0.id == activeId }) {
-            ThemeEngine.shared.activateTheme(id: activeId)
-        }
+        // Re-activate the correct theme for the current appearance
+        let appearance = AppSettingsManager.shared.appearance
+        ThemeEngine.shared.updateAppearanceAndTheme(
+            mode: appearance.appearanceMode,
+            lightThemeId: appearance.preferredLightThemeId,
+            darkThemeId: appearance.preferredDarkThemeId
+        )
 
         Self.logger.info("Updated \(installedThemes.count) theme(s) for registry plugin: \(plugin.id)")
     }
diff --git a/TablePro/Theme/ThemeStorage.swift b/TablePro/Theme/ThemeStorage.swift
@@ -179,18 +179,6 @@ internal struct ThemeStorage {
         logger.info("Exported theme: \(theme.id) to \(destinationURL.lastPathComponent)")
     }
 
-    // MARK: - Active Theme Persistence
-
-    private static let activeThemeKey = "com.TablePro.settings.activeThemeId"
-
-    static func loadActiveThemeId() -> String {
-        UserDefaults.standard.string(forKey: activeThemeKey) ?? "tablepro.default-light"
-    }
-
-    static func saveActiveThemeId(_ id: String) {
-        UserDefaults.standard.set(id, forKey: activeThemeKey)
-    }
-
     // MARK: - Helpers
 
     private static func ensureUserDirectory() {
diff --git a/TablePro/Views/Settings/Appearance/ThemeListView.swift b/TablePro/Views/Settings/Appearance/ThemeListView.swift
@@ -142,7 +142,6 @@ internal struct ThemeListView: View {
         let copy = engine.duplicateTheme(theme, newName: theme.name + " (Copy)")
         do {
             try engine.saveUserTheme(copy)
-            engine.activateTheme(copy)
             selectedThemeId = copy.id
         } catch {
             errorMessage = error.localizedDescription
@@ -190,7 +189,6 @@ internal struct ThemeListView: View {
         guard panel.runModal() == .OK, let url = panel.url else { return }
         do {
             let imported = try engine.importTheme(from: url)
-            engine.activateTheme(imported)
             selectedThemeId = imported.id
         } catch {
             errorMessage = error.localizedDescription
diff --git a/TablePro/Views/Settings/AppearanceSettingsView.swift b/TablePro/Views/Settings/AppearanceSettingsView.swift
diff --git a/docs/customization/appearance.mdx b/docs/customization/appearance.mdx
