From 60689c9fbe98e69e730792e9f94147b0874dfb5d Mon Sep 17 00:00:00 2001 From: Marco Eidinger Date: Mon, 14 Nov 2022 21:21:44 -0800 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20Custom=20colors=20were=20?= =?UTF-8?q?not=20updated=20when=20appearance=20changed=20(#482)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 🐛 Custom colors were not updated when appearance changed ✅ Closes: #481 * refactor: use new API (available with iOS 14) * test: fix unit tests * style: format code Co-authored-by: dyongxu <61523257+dyongxu@users.noreply.github.com> --- .../Examples/FioriThemeManager/Colors.swift | 26 ++++++++-- .../FioriThemeManagerContentView.swift | 2 +- .../Colors/Color+Extension.swift | 21 ++------ Sources/FioriThemeManager/ThemeManager.swift | 51 +++++++++---------- .../StyleSheetSettingsIntegrationTests.swift | 6 +-- .../FioriThemeManager/ThemeManagerTests.swift | 11 ++-- 6 files changed, 59 insertions(+), 58 deletions(-) diff --git a/Apps/Examples/Examples/FioriThemeManager/Colors.swift b/Apps/Examples/Examples/FioriThemeManager/Colors.swift index ee69259d3..017dad966 100644 --- a/Apps/Examples/Examples/FioriThemeManager/Colors.swift +++ b/Apps/Examples/Examples/FioriThemeManager/Colors.swift @@ -39,7 +39,7 @@ struct CustomColors: View { } ForEach(colorStyles, id: \.self) { colorStyle in - ColorView(colorStyle: colorStyle) + ColorViewWithNoHexDescription(colorStyle: colorStyle) } } .onAppear(perform: { @@ -47,11 +47,11 @@ struct CustomColors: View { case .customPalette(let provider): StyleSheetSettings.reset() ThemeManager.shared.setPalette(Palette(provider)) - case .programmatic(let color): + case .programmatic(let lightColor, let darkColor): StyleSheetSettings.reset() ThemeManager.shared.setPalette(PaletteVersion.latest.rawValue) - ThemeManager.shared.setColor(color, for: .primaryLabel, variant: .light) - ThemeManager.shared.setColor(color, for: .primaryLabel, variant: .dark) + ThemeManager.shared.setColor(lightColor, for: .primaryLabel, variant: .light) + ThemeManager.shared.setColor(darkColor, for: .primaryLabel, variant: .dark) case .styleSheet(let content): StyleSheetSettings.reset() ThemeManager.shared.setPalette(PaletteVersion.latest.rawValue) @@ -86,9 +86,25 @@ struct ColorView: View { } } +struct ColorViewWithNoHexDescription: View { + var colorStyle: ColorStyle + + var body: some View { + HStack { + Circle() + .fill(Color.preferredColor(colorStyle)) + .frame(width: 50, height: 50) + .padding() + VStack { + Text(colorStyle.rawValue) + } + } + } +} + enum ColorTestData { case customPalette(PaletteProvider) - case programmatic(Color) + case programmatic(Color, Color) case styleSheet(String) static var sampleStyleSheet: String { """ diff --git a/Apps/Examples/Examples/FioriThemeManager/FioriThemeManagerContentView.swift b/Apps/Examples/Examples/FioriThemeManager/FioriThemeManagerContentView.swift index 6895a7061..6cb6ed110 100644 --- a/Apps/Examples/Examples/FioriThemeManager/FioriThemeManagerContentView.swift +++ b/Apps/Examples/Examples/FioriThemeManager/FioriThemeManagerContentView.swift @@ -17,7 +17,7 @@ struct FioriThemeManagerContentView: View { Text("Colors - custom palette (random)") } NavigationLink( - destination: CustomColors(testData: .programmatic(.green))) { + destination: CustomColors(testData: .programmatic(.green, .red))) { Text("Colors - developer override") } NavigationLink( diff --git a/Sources/FioriThemeManager/Colors/Color+Extension.swift b/Sources/FioriThemeManager/Colors/Color+Extension.swift index a594a1852..5f1b79f95 100644 --- a/Sources/FioriThemeManager/Colors/Color+Extension.swift +++ b/Sources/FioriThemeManager/Colors/Color+Extension.swift @@ -27,24 +27,9 @@ public extension Color { } } -extension Color { - public func uiColor() -> UIColor { - let components = self.components() - return UIColor(red: components.r, green: components.g, blue: components.b, alpha: components.a) - } - - private func components() -> (r: CGFloat, g: CGFloat, b: CGFloat, a: CGFloat) { - let scanner = Scanner(string: self.description.trimmingCharacters(in: CharacterSet.alphanumerics.inverted)) - var hexNumber: UInt64 = 0 - var r: CGFloat = 0.0, g: CGFloat = 0.0, b: CGFloat = 0.0, a: CGFloat = 0.0 - let result = scanner.scanHexInt64(&hexNumber) - if result { - r = CGFloat((hexNumber & 0xff000000) >> 24) / 255 - g = CGFloat((hexNumber & 0x00ff0000) >> 16) / 255 - b = CGFloat((hexNumber & 0x0000ff00) >> 8) / 255 - a = CGFloat(hexNumber & 0x000000ff) / 255 - } - return (r, g, b, a) +public extension Color { + func uiColor() -> UIColor { + UIColor(self) } } diff --git a/Sources/FioriThemeManager/ThemeManager.swift b/Sources/FioriThemeManager/ThemeManager.swift index da07a9cea..5641fee2a 100644 --- a/Sources/FioriThemeManager/ThemeManager.swift +++ b/Sources/FioriThemeManager/ThemeManager.swift @@ -156,37 +156,34 @@ public class ThemeManager { internal func color(for style: ColorStyle, background scheme: BackgroundColorScheme?, interface level: InterfaceLevel?, display mode: ColorDisplayMode?) -> Color { let uiColor = self.uiColor(for: style, background: scheme, interface: level, display: mode) let color = Color(uiColor) - - guard let hexColor = self.hexColor(for: style) else { return color } - - let variant = hexColor.getVariant(traits: UITraitCollection.current, background: scheme, interface: level, display: mode) - - if let ssOverrideColor = self.styleSheetOverrides[style, default: [:]][variant] { - return ssOverrideColor - } - - if let ssHexColor = nssHexColor(for: style, with: variant) { - if let ssColor: Color = StyleSheetConverter.toColor(value: ssHexColor) { - self.styleSheetOverrides[style, default: [:]].updateValue(ssColor, forKey: variant) - return ssColor - } - } - - if let devOverrideColor = developerOverrides[style, default: [:]][variant] { - return devOverrideColor - } - return color } func uiColor(for style: ColorStyle, background scheme: BackgroundColorScheme?, interface level: InterfaceLevel?, display mode: ColorDisplayMode?) -> UIColor { - guard let hc = self.hexColor(for: style) else { return UIColor() } - let uc = UIColor { traitCollection in - let variant: ColorVariant = hc.getVariant(traits: traitCollection, background: scheme, interface: level, display: mode) + guard let hc = self.hexColor(for: style) else { return .clear } + let uc = UIColor { [weak self] traitCollection in + guard let self = self else { return .clear } + guard let hexColor = self.hexColor(for: style) else { return .clear } + let variant = hexColor.getVariant(traits: traitCollection, background: scheme, interface: level, display: mode) + let hexColorString: String = hc.hex(variant) - let components = hc.rgba(hexColorString) - return UIColor(red: CGFloat(components.r), green: CGFloat(components.g), - blue: CGFloat(components.b), alpha: CGFloat(components.a)) + + if let ssOverrideColor = self.styleSheetOverrides[style, default: [:]][variant] { + return ssOverrideColor.uiColor() + } + + if let ssHexColor = self.nssHexColor(for: style, with: variant) { + if let ssColor: Color = StyleSheetConverter.toColor(value: ssHexColor) { + self.styleSheetOverrides[style, default: [:]].updateValue(ssColor, forKey: variant) + return ssColor.uiColor() + } + } + + if let devOverrideColor = self.developerOverrides[style, default: [:]][variant] { + return devOverrideColor.uiColor() + } + + return Color(hex: hexColorString)?.uiColor() ?? .clear } return uc } @@ -194,7 +191,7 @@ public class ThemeManager { func nssHexColor(for style: ColorStyle, with variant: ColorVariant) -> String? { var developerOverriddenColors = [String: String]() -// // Get the developer defined color definitions from the .nss file + // Get the developer defined color definitions from the .nss file developerOverriddenColors = StyleSheetSettings.shared.globalDefinitions // NUISettings.getInstance()._globalDefinitions // In .nss file color is defined using background scheme, so check for inversed color variant diff --git a/Tests/FioriSwiftUITests/FioriThemeManager/StyleSheetSettingsIntegrationTests.swift b/Tests/FioriSwiftUITests/FioriThemeManager/StyleSheetSettingsIntegrationTests.swift index bf0181c6d..e2a2a37e3 100644 --- a/Tests/FioriSwiftUITests/FioriThemeManager/StyleSheetSettingsIntegrationTests.swift +++ b/Tests/FioriSwiftUITests/FioriThemeManager/StyleSheetSettingsIntegrationTests.swift @@ -54,7 +54,7 @@ class StyleSheetSettingsIntegrationTests: XCTestCase { XCTAssertNoThrow(try? StyleSheetSettings.loadStylesheetByString(content: sampleStyleSheetContent)) - XCTAssertNotEqual(originalColor.toHex(), Color.preferredColor(.primaryLabel).toHex(), "Color.preferredColor should return the color specified in the styleSheet") + XCTAssertNotEqual(originalColor.resolvedColor(with: .light).uiColor(), Color.preferredColor(.primaryLabel).uiColor(), "Color.preferredColor should return the color specified in the styleSheet") XCTAssertEqual(ThemeManager.shared.styleSheetOverrides.keys.count, 1) } @@ -63,12 +63,12 @@ class StyleSheetSettingsIntegrationTests: XCTestCase { XCTAssertNoThrow(try? StyleSheetSettings.loadStylesheetByURL(url: Bundle.module.url(forResource: "styleSheet", withExtension: "nss")!)) - XCTAssertNotEqual(originalColor.toHex(), Color.preferredColor(.primaryLabel).toHex(), "Color.preferredColor should return the color specified in the styleSheet") + XCTAssertNotEqual(originalColor.resolvedColor(with: .light).uiColor(), Color.preferredColor(.primaryLabel).uiColor(), "Color.preferredColor should return the color specified in the styleSheet") XCTAssertEqual(ThemeManager.shared.styleSheetOverrides.keys.count, 1) let tintColor = Color.preferredColor(.tintColor, background: .darkConstant) let expectedColor = Color(hex: "ff4466")! - XCTAssertEqual(tintColor.cgColor, expectedColor.cgColor) + XCTAssertEqual(tintColor.toHex(), expectedColor.toHex()) } func testReset() { diff --git a/Tests/FioriSwiftUITests/FioriThemeManager/ThemeManagerTests.swift b/Tests/FioriSwiftUITests/FioriThemeManager/ThemeManagerTests.swift index d723174a8..d9eb75429 100644 --- a/Tests/FioriSwiftUITests/FioriThemeManager/ThemeManagerTests.swift +++ b/Tests/FioriSwiftUITests/FioriThemeManager/ThemeManagerTests.swift @@ -92,8 +92,12 @@ class ThemeManagerTests: XCTestCase { let tm = ThemeManager.shared tm.setPaletteVersion(.v7) - tm.setColor(.red, for: .grey1, variant: .light) - XCTAssertEqual(Color.preferredColor(.grey1, background: .darkConstant), Color.red) + let expectedColor = Color.red + tm.setColor(expectedColor, for: .grey1, variant: .light) + + let actualColor = Color.preferredColor(.grey1, background: .darkConstant) + + XCTAssertEqual(expectedColor.toHex(), actualColor.toHex()) } func testSetHexColor() throws { @@ -122,7 +126,6 @@ class ThemeManagerTests: XCTestCase { tm.setHexColor("#f46", for: .tintColor, variant: .light) let tintColor = Color.preferredColor(.tintColor, background: .darkConstant) - let expectedColor = Color(hex: "ff4466")! - XCTAssertEqual(tintColor.cgColor, expectedColor.cgColor) + XCTAssertEqual(tintColor.toHex(), "FF4466") } }