From e3ae421f7cca0c4b299859b34b04a069eb08fc1a Mon Sep 17 00:00:00 2001 From: Aman Harwara Date: Sat, 23 Mar 2024 00:43:02 +0530 Subject: [PATCH] chore: fix issues with multiple non-layerable themes being active on color scheme change [skip e2e] --- .../Component/ComponentManagerInterface.ts | 1 + .../ComponentManager/ComponentManager.ts | 26 +++++++++++-------- .../ui-services/src/Theme/ThemeManager.ts | 22 +++++++++++++--- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/packages/services/src/Domain/Component/ComponentManagerInterface.ts b/packages/services/src/Domain/Component/ComponentManagerInterface.ts index e2161a195..fd1902df9 100644 --- a/packages/services/src/Domain/Component/ComponentManagerInterface.ts +++ b/packages/services/src/Domain/Component/ComponentManagerInterface.ts @@ -37,6 +37,7 @@ export interface ComponentManagerInterface { isThemeActive(theme: UIFeature): boolean toggleTheme(theme: UIFeature, skipEntitlementCheck?: boolean): Promise + toggleOtherNonLayerableThemes(uiFeature: UIFeature): void getActiveThemes(): UIFeature[] getActiveThemesIdentifiers(): { features: NativeFeatureIdentifier[]; uuids: Uuid[] } diff --git a/packages/snjs/lib/Services/ComponentManager/ComponentManager.ts b/packages/snjs/lib/Services/ComponentManager/ComponentManager.ts index 4773b2a77..e6523e238 100644 --- a/packages/snjs/lib/Services/ComponentManager/ComponentManager.ts +++ b/packages/snjs/lib/Services/ComponentManager/ComponentManager.ts @@ -390,6 +390,19 @@ export class ComponentManager return this.viewers.find((viewer) => viewer.sessionKey === key) } + public toggleOtherNonLayerableThemes(uiFeature: UIFeature): void { + const activeThemes = this.getActiveThemes() + for (const candidate of activeThemes) { + if (candidate.featureIdentifier === uiFeature.featureIdentifier) { + continue + } + + if (!candidate.layerable) { + this.removeActiveTheme(candidate) + } + } + } + public async toggleTheme(uiFeature: UIFeature, skipEntitlementCheck = false): Promise { this.logger.info('Toggling theme', uiFeature.uniqueIdentifier) @@ -410,16 +423,7 @@ export class ComponentManager if (!uiFeature.layerable) { await sleep(10) - const activeThemes = this.getActiveThemes() - for (const candidate of activeThemes) { - if (candidate.featureIdentifier === uiFeature.featureIdentifier) { - continue - } - - if (!candidate.layerable) { - this.removeActiveTheme(candidate) - } - } + this.toggleOtherNonLayerableThemes(uiFeature) } } @@ -454,7 +458,7 @@ export class ComponentManager const features: NativeFeatureIdentifier[] = [] const uuids: Uuid[] = [] - const strings = this.preferences.getLocalValue(LocalPrefKey.ActiveThemes, []) + const strings = new Set(this.preferences.getLocalValue(LocalPrefKey.ActiveThemes, [])) for (const string of strings) { const nativeIdentifier = NativeFeatureIdentifier.create(string) if (!nativeIdentifier.isFailed()) { diff --git a/packages/ui-services/src/Theme/ThemeManager.ts b/packages/ui-services/src/Theme/ThemeManager.ts index f1e339baf..946068733 100644 --- a/packages/ui-services/src/Theme/ThemeManager.ts +++ b/packages/ui-services/src/Theme/ThemeManager.ts @@ -217,6 +217,12 @@ export class ThemeManager extends AbstractUIService { } } + const shouldSetThemeAsPerColorScheme = this.preferences.getLocalValue(LocalPrefKey.UseSystemColorScheme, false) + if (shouldSetThemeAsPerColorScheme) { + const prefersDarkColorScheme = window.matchMedia('(prefers-color-scheme: dark)').matches + hasChange = this.setThemeAsPerColorScheme(prefersDarkColorScheme) + } + if (hasChange) { void this.cacheThemeState() } @@ -230,7 +236,9 @@ export class ThemeManager extends AbstractUIService { } } - private setThemeAsPerColorScheme(prefersDarkColorScheme: boolean) { + private setThemeAsPerColorScheme(prefersDarkColorScheme: boolean): boolean { + let didChangeTheme = false + const preference = prefersDarkColorScheme ? LocalPrefKey.AutoDarkThemeIdentifier : LocalPrefKey.AutoLightThemeIdentifier @@ -251,6 +259,7 @@ export class ThemeManager extends AbstractUIService { const toggleActiveTheme = () => { if (activeTheme) { void this.components.toggleTheme(activeTheme) + didChangeTheme = true } } @@ -258,10 +267,17 @@ export class ThemeManager extends AbstractUIService { toggleActiveTheme() } else { const theme = themes.find((theme) => theme.featureIdentifier === themeIdentifier) - if (theme && !this.components.isThemeActive(theme)) { - this.components.toggleTheme(theme, true).catch(console.error) + if (theme) { + if (!this.components.isThemeActive(theme)) { + this.components.toggleTheme(theme, true).catch(console.error) + } else { + this.components.toggleOtherNonLayerableThemes(theme) + } + didChangeTheme = true } } + + return didChangeTheme } private async activateCachedThemes() {