-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor FXIOS-9331 [Theming] Fix ThemeManager issues #20667
Changes from all commits
3bbcd5d
dd74e80
2c4df8c
b9bd31f
7492a47
987f88d
8e7197c
bcb9a4c
88df65b
38d49fd
e366b5f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,6 @@ public final class DefaultThemeManager: ThemeManager, Notifiable { | |
|
||
// MARK: - Variables | ||
|
||
private var windowThemeState: [WindowUUID: Theme] = [:] | ||
private var windows: [WindowUUID: UIWindow] = [:] | ||
private var allWindowUUIDs: [WindowUUID] { return Array(windows.keys) } | ||
public var notificationCenter: NotificationProtocol | ||
|
@@ -44,10 +43,6 @@ public final class DefaultThemeManager: ThemeManager, Notifiable { | |
return userDefaults.bool(forKey: ThemeKeys.NightMode.isOn) | ||
} | ||
|
||
private func privateModeIsOn(for window: WindowUUID) -> Bool { | ||
return getPrivateThemeIsOn(for: window) | ||
} | ||
|
||
public var systemThemeIsOn: Bool { | ||
return userDefaults.bool(forKey: ThemeKeys.systemThemeIsOn) | ||
} | ||
|
@@ -83,132 +78,102 @@ public final class DefaultThemeManager: ThemeManager, Notifiable { | |
UIApplication.didBecomeActiveNotification]) | ||
} | ||
|
||
// MARK: - ThemeManager | ||
|
||
public func windowNonspecificTheme() -> Theme { | ||
switch getNormalSavedTheme() { | ||
case .dark, .nightMode, .privateMode: return DarkTheme() | ||
case .light: return LightTheme() | ||
} | ||
} | ||
|
||
public func windowDidClose(uuid: WindowUUID) { | ||
windows.removeValue(forKey: uuid) | ||
windowThemeState.removeValue(forKey: uuid) | ||
} | ||
|
||
public func setWindow(_ window: UIWindow, for uuid: WindowUUID) { | ||
windows[uuid] = window | ||
updateSavedTheme(to: getNormalSavedTheme()) | ||
updateCurrentTheme(to: fetchSavedThemeType(for: uuid), for: uuid) | ||
} | ||
|
||
public func currentTheme(for window: WindowUUID?) -> Theme { | ||
// MARK: - Themeing general functions | ||
public func getCurrentTheme(for window: WindowUUID?) -> Theme { | ||
guard let window else { | ||
assertionFailure("Attempt to get the theme for a nil window UUID.") | ||
return DarkTheme() | ||
} | ||
|
||
return windowThemeState[window] ?? DarkTheme() | ||
return getThemeFrom(type: determineThemeType(for: window)) | ||
} | ||
|
||
public func changeCurrentTheme(_ newTheme: ThemeType, for window: WindowUUID) { | ||
guard currentTheme(for: window).type != newTheme else { return } | ||
|
||
updateSavedTheme(to: newTheme) | ||
|
||
// Although we may have only explicitly changed the state on one specific window, | ||
// we want to be sure we update all windows in case the Light/Dark theme changed. | ||
public func applyThemeUpdatesToWindows() { | ||
allWindowUUIDs.forEach { | ||
updateCurrentTheme(to: fetchSavedThemeType(for: $0), for: $0, notify: false) | ||
applyThemeChanges(for: $0, using: determineThemeType(for: $0)) | ||
} | ||
|
||
// After updating all windows, notify (once). We send the UUID of the window for | ||
// which the change originated though more than 1 window may ultimately update its UI. | ||
notifyCurrentThemeDidChange(for: window) | ||
} | ||
|
||
public func reloadTheme(for window: WindowUUID) { | ||
updateCurrentTheme(to: fetchSavedThemeType(for: window), for: window) | ||
// MARK: - Manual theme functions | ||
public func setManualTheme(to newTheme: ThemeType) { | ||
updateSavedTheme(to: newTheme) | ||
applyThemeUpdatesToWindows() | ||
} | ||
|
||
public func systemThemeChanged() { | ||
allWindowUUIDs.forEach { uuid in | ||
// Ignore if: | ||
// the system theme is off | ||
// OR night mode is on | ||
// OR private mode is on | ||
guard systemThemeIsOn, | ||
!nightModeIsOn, | ||
!privateModeIsOn(for: uuid) | ||
else { return } | ||
|
||
changeCurrentTheme(getSystemThemeType(), for: uuid) | ||
} | ||
public func getUserManualTheme() -> ThemeType { | ||
guard let savedThemeDescription = userDefaults.string(forKey: ThemeKeys.themeName), | ||
let savedTheme = ThemeType(rawValue: savedThemeDescription) | ||
else { return getThemeTypeBasedOnSystem() } | ||
|
||
return savedTheme | ||
} | ||
|
||
// MARK: - System theme functions | ||
public func setSystemTheme(isOn: Bool) { | ||
userDefaults.set(isOn, forKey: ThemeKeys.systemThemeIsOn) | ||
applyThemeUpdatesToWindows() | ||
} | ||
|
||
if isOn { | ||
systemThemeChanged() | ||
} else if automaticBrightnessIsOn { | ||
updateThemeBasedOnBrightness() | ||
} else { | ||
allWindowUUIDs.forEach { reloadTheme(for: $0) } | ||
} | ||
private func getThemeTypeBasedOnSystem() -> ThemeType { | ||
return UIScreen.main.traitCollection.userInterfaceStyle == .dark ? ThemeType.dark : ThemeType.light | ||
} | ||
|
||
// MARK: - Private theme functions | ||
public func setPrivateTheme(isOn: Bool, for window: WindowUUID) { | ||
let currentSetting = getPrivateThemeIsOn(for: window) | ||
guard currentSetting != isOn else { return } | ||
guard getPrivateThemeIsOn(for: window) != isOn else { return } | ||
|
||
var settings: KeyedPrivateModeFlags | ||
= userDefaults.object(forKey: ThemeKeys.PrivateMode.byWindowUUID) as? KeyedPrivateModeFlags ?? [:] | ||
|
||
settings[window.uuidString] = NSNumber(value: isOn) | ||
userDefaults.set(settings, forKey: ThemeKeys.PrivateMode.byWindowUUID) | ||
|
||
updateCurrentTheme(to: fetchSavedThemeType(for: window), for: window) | ||
applyThemeChanges(for: window, using: determineThemeType(for: window)) | ||
} | ||
|
||
public func getPrivateThemeIsOn(for window: WindowUUID) -> Bool { | ||
let settings = userDefaults.object(forKey: ThemeKeys.PrivateMode.byWindowUUID) as? KeyedPrivateModeFlags | ||
if settings == nil { | ||
migrateSingleWindowPrivateDefaultsToMultiWindow(for: window) | ||
} | ||
if settings == nil { migrateSingleWindowPrivateDefaultsToMultiWindow(for: window) } | ||
|
||
let boxedBool = settings?[window.uuidString] as? NSNumber | ||
return boxedBool?.boolValue ?? false | ||
} | ||
|
||
// MARK: - Automatic brightness theme functions | ||
public func setAutomaticBrightness(isOn: Bool) { | ||
guard automaticBrightnessIsOn != isOn else { return } | ||
|
||
userDefaults.set(isOn, forKey: ThemeKeys.AutomaticBrightness.isOn) | ||
brightnessChanged() | ||
applyThemeUpdatesToWindows() | ||
} | ||
|
||
public func setAutomaticBrightnessValue(_ value: Float) { | ||
userDefaults.set(value, forKey: ThemeKeys.AutomaticBrightness.thresholdValue) | ||
brightnessChanged() | ||
applyThemeUpdatesToWindows() | ||
} | ||
|
||
private func getThemeTypeBasedOnBrightness() -> ThemeType { | ||
return Float(UIScreen.main.brightness) < automaticBrightnessValue ? .dark : .light | ||
} | ||
|
||
public func brightnessChanged() { | ||
if automaticBrightnessIsOn { | ||
updateThemeBasedOnBrightness() | ||
// MARK: - Window specific functions | ||
public func windowNonspecificTheme() -> Theme { | ||
switch getUserManualTheme() { | ||
case .dark, .nightMode, .privateMode: return DarkTheme() | ||
case .light: return LightTheme() | ||
} | ||
} | ||
|
||
public func getNormalSavedTheme() -> ThemeType { | ||
guard let savedThemeDescription = userDefaults.string(forKey: ThemeKeys.themeName), | ||
let savedTheme = ThemeType(rawValue: savedThemeDescription) | ||
else { return getSystemThemeType() } | ||
public func windowDidClose(uuid: WindowUUID) { | ||
windows.removeValue(forKey: uuid) | ||
} | ||
|
||
return savedTheme | ||
public func setWindow(_ window: UIWindow, for uuid: WindowUUID) { | ||
windows[uuid] = window | ||
updateSavedTheme(to: getUserManualTheme()) | ||
applyThemeChanges(for: uuid, using: determineThemeType(for: uuid)) | ||
} | ||
|
||
// MARK: - Private methods | ||
// MARK: - Private helper methods | ||
|
||
private func migrateSingleWindowPrivateDefaultsToMultiWindow(for window: WindowUUID) { | ||
// Migrate old private setting to our window-based settings | ||
Comment on lines
178
to
179
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we may be able to remove this migration code and persisted state entirely? IIRC this persisted state was to keep track of private mode between app launches, but now I think we've updated the iOS client to always clear private tabs on launch. And I believe the related setting for that has also been removed. (Though we should double-check, maybe all of this is still behind an experiment. I also have seen several user reviews already complaining about that change so we'd want to confirm with Product whether it's permanent or not.) But if we have decided 100% to remove all of that, then I think we can also remove this migration and persisted state, since the app will never launch with any windows in Private mode. I'd definitely like to get confirmation on that from someone who is a bit more in-the-loop on that however. (cc @OrlaM) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I can make a note to fully look into this afterward, but I feel like this shouldn't be a blocker for merging this PR. It's easily removed later. |
||
|
@@ -218,42 +183,36 @@ public final class DefaultThemeManager: ThemeManager, Notifiable { | |
} | ||
|
||
private func updateSavedTheme(to newTheme: ThemeType) { | ||
guard !newTheme.isOverridingThemeType() else { return } | ||
guard !systemThemeIsOn else { return } | ||
userDefaults.set(newTheme.rawValue, forKey: ThemeKeys.themeName) | ||
} | ||
|
||
private func updateCurrentTheme(to newTheme: ThemeType, for window: WindowUUID, notify: Bool = true) { | ||
windowThemeState[window] = newThemeForType(newTheme) | ||
|
||
private func applyThemeChanges(for window: WindowUUID, using newTheme: ThemeType) { | ||
// Overwrite the user interface style on the window attached to our scene | ||
// once we have multiple scenes we need to update all of them | ||
let style = self.currentTheme(for: window).type.getInterfaceStyle() | ||
let style = self.getCurrentTheme(for: window).type.getInterfaceStyle() | ||
self.windows[window]?.overrideUserInterfaceStyle = style | ||
if notify { | ||
notifyCurrentThemeDidChange(for: window) | ||
} | ||
notifyCurrentThemeDidChange(for: window) | ||
} | ||
|
||
private func notifyCurrentThemeDidChange(for window: WindowUUID) { | ||
mainQueue.ensureMainThread { [weak self] in | ||
self?.notificationCenter.post(name: .ThemeDidChange, withUserInfo: window.userInfo) | ||
self?.notificationCenter.post( | ||
name: .ThemeDidChange, | ||
withUserInfo: window.userInfo | ||
) | ||
} | ||
} | ||
|
||
private func fetchSavedThemeType(for window: WindowUUID) -> ThemeType { | ||
if privateModeIsOn(for: window) { return .privateMode } | ||
private func determineThemeType(for window: WindowUUID) -> ThemeType { | ||
if getPrivateThemeIsOn(for: window) { return .privateMode } | ||
if nightModeIsOn { return .nightMode } | ||
if systemThemeIsOn { return getSystemThemeType() } | ||
if systemThemeIsOn { return getThemeTypeBasedOnSystem() } | ||
if automaticBrightnessIsOn { return getThemeTypeBasedOnBrightness() } | ||
|
||
return getNormalSavedTheme() | ||
return getUserManualTheme() | ||
} | ||
|
||
private func getSystemThemeType() -> ThemeType { | ||
return UIScreen.main.traitCollection.userInterfaceStyle == .dark ? ThemeType.dark : ThemeType.light | ||
} | ||
|
||
private func newThemeForType(_ type: ThemeType) -> Theme { | ||
private func getThemeFrom(type: ThemeType) -> Theme { | ||
switch type { | ||
case .light: | ||
return LightTheme() | ||
|
@@ -266,26 +225,13 @@ public final class DefaultThemeManager: ThemeManager, Notifiable { | |
} | ||
} | ||
|
||
private func updateThemeBasedOnBrightness() { | ||
allWindowUUIDs.forEach { uuid in | ||
let currentValue = Float(UIScreen.main.brightness) | ||
|
||
if currentValue < automaticBrightnessValue { | ||
changeCurrentTheme(.dark, for: uuid) | ||
} else { | ||
changeCurrentTheme(.light, for: uuid) | ||
} | ||
} | ||
} | ||
|
||
// MARK: - Notifiable | ||
|
||
public func handleNotifications(_ notification: Notification) { | ||
switch notification.name { | ||
case UIScreen.brightnessDidChangeNotification: | ||
brightnessChanged() | ||
case UIApplication.didBecomeActiveNotification: | ||
self.systemThemeChanged() | ||
case UIScreen.brightnessDidChangeNotification, | ||
UIApplication.didBecomeActiveNotification: | ||
applyThemeUpdatesToWindows() | ||
default: | ||
return | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't need to block the PR but we probably don't need to store this in
UserDefaults
any longer, and changing it might also fix this issue where the windows are rendering briefly in Private theme incorrectly during launch:launch_flash.mov