Refactor: support style overrides and multiple Adaptation axes#12
Conversation
ThemeAdaptiveStyle now supports pluggable resolution via a Resolver struct, enabling per-token resolution based on full EnvironmentValues (not just colorScheme). This allows accessibility-aware tokens, size-class-dependent styles, and other environment-driven adaptations. Key changes: - Add ThemeAdaptiveStyle.Resolver with id-based Equatable - light/dark become optional (nil for resolver-only tokens) - Data tokens auto-generate deterministic resolver IDs via JSON+Hasher - Custom resolvers get unique UUID-based IDs by default - Codable encode throws for resolver-only tokens (by design) - Generated code uses resolved(in:) for full environment access - Update tests to use EnvironmentValues-based resolution
Refactor ThemeAdaptiveStyle to support multiple adaptation axes beyond color scheme. The new Defaults enum (.colorScheme, .sizeClass, .value) replaces the hardcoded light/dark properties, with each case owning its resolver creation logic. Backward compatibility preserved via computed .light/.dark accessors and unchanged init(light:dark:) signature.
There was a problem hiding this comment.
Pull request overview
This pull request significantly refactors the ThemeAdaptiveStyle type to support flexible, serializable theme adaptation beyond the previous hardcoded light/dark system. The refactoring introduces a Defaults enum with three adaptation axes (colorScheme, sizeClass, and constant value) and a Resolver struct that enables custom adaptation logic while maintaining Codable support for standard cases.
Changes:
- Refactored
ThemeAdaptiveStyleto use aDefaultsenum andResolverstruct, replacing directlight/darkproperties with computed accessors - Added support for size class adaptation (compact/regular) and constant value adaptation alongside the existing color scheme adaptation
- Updated all API usage from
resolved(for:)toresolved(in:)to accept fullEnvironmentValues
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/ThemeKit/ThemeAdaptiveStyle.swift | Core refactoring to support Defaults and Resolver with custom Codable implementation |
| Sources/ThemeKit/ThemeAdaptiveStyle+Defaults.swift | New Defaults enum with colorScheme, sizeClass, and value cases plus Codable logic |
| Sources/ThemeKit/ThemeAdaptiveStyle+Resolver.swift | New Resolver struct with ID-based equality for adaptation closures |
| Sources/ThemeKit/ThemeAdaptiveStyle+ShapeStyle.swift | Updated to use resolver instead of direct colorScheme access |
| Sources/ThemeKitGenerator/ThemeShapeStyleGenerator.swift | Updated generated code to use resolved(in:) |
| Sources/ThemeKitGenerator/ThemeShadowedStyleGenerator.swift | Updated generated code to use resolved(in:) |
| Tests/ThemeKitTests/ThemeAdaptiveStyleTests.swift | Comprehensive test updates for new API, added tests for all adaptation axes |
| Tests/ThemeKitGeneratorTests/ThemeFileGeneratorTests.swift | Updated test to verify new API in generated code |
| README.md | Documentation update for new resolution API |
| IMPLEMENTATION_DETAILS.md | Documentation update for new resolution API |
| CLAUDE.md | Updated architecture documentation to reflect new adaptation model |
| .gitignore | Added Vite cache directory to gitignore |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Resolver(id: Self.resolverID(self)) { env in | ||
| env.colorScheme == .dark ? dark : light | ||
| } | ||
| case .sizeClass(let compact, let regular): |
There was a problem hiding this comment.
The sizeClass resolver only checks horizontalSizeClass but completely ignores verticalSizeClass. This design choice should be documented. On some devices (like iPad in landscape), both horizontal and vertical size classes can affect layout decisions.
Consider either:
- Adding a comment explaining why only horizontal size class is used
- Providing a way to adapt based on vertical size class (perhaps through a custom resolver)
- Or at minimum, documenting this limitation in the API documentation
| case .sizeClass(let compact, let regular): | |
| case .sizeClass(let compact, let regular): | |
| // Note: This resolver intentionally considers only the horizontal size class. | |
| // If your layout depends on verticalSizeClass as well, define a custom Resolver. |
| private enum ColorSchemeCodingKeys: String, CodingKey { | ||
| case light, dark | ||
| } | ||
|
|
||
| private enum SizeClassCodingKeys: String, CodingKey { | ||
| case compact, regular | ||
| } | ||
|
|
||
| public init(from decoder: Decoder) throws { | ||
| // Try keyed container first for colorScheme / sizeClass | ||
| if let container = try? decoder.container(keyedBy: ColorSchemeCodingKeys.self), | ||
| container.contains(.light) { | ||
| self = .colorScheme( | ||
| light: try container.decode(Style.self, forKey: .light), | ||
| dark: try container.decode(Style.self, forKey: .dark) | ||
| ) | ||
| } else if let container = try? decoder.container(keyedBy: SizeClassCodingKeys.self), | ||
| container.contains(.compact) { | ||
| self = .sizeClass( | ||
| compact: try container.decode(Style.self, forKey: .compact), | ||
| regular: try container.decode(Style.self, forKey: .regular) | ||
| ) | ||
| } else { | ||
| // Plain value | ||
| let container = try decoder.singleValueContainer() | ||
| self = .value(try container.decode(Style.self)) | ||
| } | ||
| } | ||
|
|
||
| public func encode(to encoder: Encoder) throws { | ||
| switch self { | ||
| case .colorScheme(let light, let dark): | ||
| var container = encoder.container(keyedBy: ColorSchemeCodingKeys.self) | ||
| try container.encode(light, forKey: .light) | ||
| try container.encode(dark, forKey: .dark) | ||
| case .sizeClass(let compact, let regular): | ||
| var container = encoder.container(keyedBy: SizeClassCodingKeys.self) |
There was a problem hiding this comment.
The decoding logic has a potential ambiguity if JSON contains both "light" and "compact" keys (or other overlapping combinations). The current implementation checks for "light" first, then "compact", then falls back to a plain value. If a malicious or malformed JSON contains keys from multiple adaptation axes (e.g., {"light": "#FF0000", "dark": "#0000FF", "compact": "#00FF00"}), only the colorScheme values would be decoded while the compact key would be silently ignored.
Consider adding validation to ensure that the JSON contains keys for exactly one adaptation axis, and throw a descriptive error if multiple axes are detected or if keys are incomplete (e.g., "light" without "dark").
| private enum ColorSchemeCodingKeys: String, CodingKey { | |
| case light, dark | |
| } | |
| private enum SizeClassCodingKeys: String, CodingKey { | |
| case compact, regular | |
| } | |
| public init(from decoder: Decoder) throws { | |
| // Try keyed container first for colorScheme / sizeClass | |
| if let container = try? decoder.container(keyedBy: ColorSchemeCodingKeys.self), | |
| container.contains(.light) { | |
| self = .colorScheme( | |
| light: try container.decode(Style.self, forKey: .light), | |
| dark: try container.decode(Style.self, forKey: .dark) | |
| ) | |
| } else if let container = try? decoder.container(keyedBy: SizeClassCodingKeys.self), | |
| container.contains(.compact) { | |
| self = .sizeClass( | |
| compact: try container.decode(Style.self, forKey: .compact), | |
| regular: try container.decode(Style.self, forKey: .regular) | |
| ) | |
| } else { | |
| // Plain value | |
| let container = try decoder.singleValueContainer() | |
| self = .value(try container.decode(Style.self)) | |
| } | |
| } | |
| public func encode(to encoder: Encoder) throws { | |
| switch self { | |
| case .colorScheme(let light, let dark): | |
| var container = encoder.container(keyedBy: ColorSchemeCodingKeys.self) | |
| try container.encode(light, forKey: .light) | |
| try container.encode(dark, forKey: .dark) | |
| case .sizeClass(let compact, let regular): | |
| var container = encoder.container(keyedBy: SizeClassCodingKeys.self) | |
| private enum AdaptiveCodingKeys: String, CodingKey { | |
| case light | |
| case dark | |
| case compact | |
| case regular | |
| } | |
| public init(from decoder: Decoder) throws { | |
| // Try a keyed container first for colorScheme / sizeClass axes. | |
| if let container = try? decoder.container(keyedBy: AdaptiveCodingKeys.self) { | |
| let hasLight = container.contains(.light) | |
| let hasDark = container.contains(.dark) | |
| let hasCompact = container.contains(.compact) | |
| let hasRegular = container.contains(.regular) | |
| let hasColorSchemeKeys = hasLight || hasDark | |
| let hasSizeClassKeys = hasCompact || hasRegular | |
| // If any axis keys are present, we must validate them. | |
| if hasColorSchemeKeys || hasSizeClassKeys { | |
| // Mixed axes are not allowed. | |
| if hasColorSchemeKeys && hasSizeClassKeys { | |
| throw DecodingError.dataCorruptedError( | |
| forKey: hasLight ? .light : .compact, | |
| in: container, | |
| debugDescription: "ThemeAdaptiveStyle.Defaults must use exactly one adaptation axis. Found both colorScheme (light/dark) and sizeClass (compact/regular) keys." | |
| ) | |
| } | |
| if hasColorSchemeKeys { | |
| // Require both light and dark for a valid colorScheme axis. | |
| guard hasLight, hasDark else { | |
| let missingKey: AdaptiveCodingKeys = hasLight ? .dark : .light | |
| throw DecodingError.dataCorruptedError( | |
| forKey: missingKey, | |
| in: container, | |
| debugDescription: "Incomplete colorScheme axis for ThemeAdaptiveStyle.Defaults. Both 'light' and 'dark' keys are required." | |
| ) | |
| } | |
| self = .colorScheme( | |
| light: try container.decode(Style.self, forKey: .light), | |
| dark: try container.decode(Style.self, forKey: .dark) | |
| ) | |
| return | |
| } | |
| // Size class axis | |
| guard hasCompact, hasRegular else { | |
| let missingKey: AdaptiveCodingKeys = hasCompact ? .regular : .compact | |
| throw DecodingError.dataCorruptedError( | |
| forKey: missingKey, | |
| in: container, | |
| debugDescription: "Incomplete sizeClass axis for ThemeAdaptiveStyle.Defaults. Both 'compact' and 'regular' keys are required." | |
| ) | |
| } | |
| self = .sizeClass( | |
| compact: try container.decode(Style.self, forKey: .compact), | |
| regular: try container.decode(Style.self, forKey: .regular) | |
| ) | |
| return | |
| } | |
| } | |
| // If we reach here, no recognized axis keys are present: decode as a plain value. | |
| let singleValueContainer = try decoder.singleValueContainer() | |
| self = .value(try singleValueContainer.decode(Style.self)) | |
| } | |
| public func encode(to encoder: Encoder) throws { | |
| switch self { | |
| case .colorScheme(let light, let dark): | |
| var container = encoder.container(keyedBy: AdaptiveCodingKeys.self) | |
| try container.encode(light, forKey: .light) | |
| try container.encode(dark, forKey: .dark) | |
| case .sizeClass(let compact, let regular): | |
| var container = encoder.container(keyedBy: AdaptiveCodingKeys.self) |
| if let container = try? decoder.container(keyedBy: ColorSchemeCodingKeys.self), | ||
| container.contains(.light) { | ||
| self = .colorScheme( | ||
| light: try container.decode(Style.self, forKey: .light), | ||
| dark: try container.decode(Style.self, forKey: .dark) | ||
| ) | ||
| } else if let container = try? decoder.container(keyedBy: SizeClassCodingKeys.self), | ||
| container.contains(.compact) { | ||
| self = .sizeClass( | ||
| compact: try container.decode(Style.self, forKey: .compact), | ||
| regular: try container.decode(Style.self, forKey: .regular) | ||
| ) |
There was a problem hiding this comment.
The decoding logic only checks if the "light" key exists to determine if this is a colorScheme case, but doesn't verify that the "dark" key also exists before attempting to decode it. If JSON contains only {"light": "#FF0000"} without a "dark" key, the decode will fail with a potentially confusing KeyNotFound error rather than a clear validation error.
The same issue exists for sizeClass - only "compact" is checked for existence, but "regular" is assumed to exist. Consider checking for both keys in each case before attempting to decode, and provide a clear error message if one is missing.
| private static func resolverID(_ defaults: Defaults) -> String { | ||
| let encoder = JSONEncoder() | ||
| encoder.outputFormatting = .sortedKeys | ||
| guard let data = try? encoder.encode(defaults) else { | ||
| return UUID().uuidString | ||
| } | ||
| var hasher = Hasher() | ||
| hasher.combine(data) | ||
| return String(hasher.finalize()) | ||
| } |
There was a problem hiding this comment.
The Hasher type in Swift does not guarantee stable hashes across different executions of a program. Hash values can change between app launches, OS versions, or even different runs. Using Hasher.finalize() to generate a stable ID for the resolver means that the same Defaults could produce different resolver IDs in different contexts, which could break equality comparisons or cause issues with serialization round-trips if the ID is persisted.
Consider using a stable hashing algorithm (such as SHA-256 from CryptoKit) or a deterministic string representation for the resolver ID to ensure consistency across app launches and platforms.
| } | ||
|
|
||
| // MARK: - Equality across forms | ||
|
|
||
| @Test func equality_sameColorScheme_areEqual() { | ||
| let a = ThemeAdaptiveStyle(light: Color(hex: 0xFF0000), dark: Color(hex: 0x0000FF)) | ||
| let b = ThemeAdaptiveStyle(light: Color(hex: 0xFF0000), dark: Color(hex: 0x0000FF)) | ||
| #expect(a == b) | ||
| } | ||
|
|
||
| @Test func equality_sameSizeClass_areEqual() { | ||
| let a = ThemeAdaptiveStyle(compact: Color(hex: 0xFF0000), regular: Color(hex: 0x0000FF)) | ||
| let b = ThemeAdaptiveStyle(compact: Color(hex: 0xFF0000), regular: Color(hex: 0x0000FF)) | ||
| #expect(a == b) | ||
| } | ||
|
|
||
| @Test func equality_sameValue_areEqual() { | ||
| let a = ThemeAdaptiveStyle(value: Color(hex: 0xFF0000)) | ||
| let b = ThemeAdaptiveStyle(value: Color(hex: 0xFF0000)) | ||
| #expect(a == b) | ||
| } | ||
|
|
||
| @Test func equality_differentForms_areNotEqual() { | ||
| let colorScheme = ThemeAdaptiveStyle(light: Color(hex: 0xFF0000), dark: Color(hex: 0x0000FF)) | ||
| let sizeClass = ThemeAdaptiveStyle(compact: Color(hex: 0xFF0000), regular: Color(hex: 0x0000FF)) | ||
| let value = ThemeAdaptiveStyle(value: Color(hex: 0xFF0000)) | ||
| #expect(colorScheme != sizeClass) | ||
| #expect(colorScheme != value) | ||
| #expect(sizeClass != value) | ||
| } |
There was a problem hiding this comment.
There are no tests that verify the behavior when decoding malformed or incomplete JSON. Consider adding tests for edge cases such as:
- JSON with only "light" key but missing "dark"
- JSON with only "compact" key but missing "regular"
- JSON with mixed keys from different adaptation axes (e.g., both "light" and "compact")
- Empty JSON object
{}
These tests would help ensure that the decoder provides clear error messages for invalid input rather than failing with generic KeyNotFound or other confusing errors.
| struct Resolver: Sendable { | ||
| public let id: String | ||
| public let resolve: @Sendable (EnvironmentValues) -> Style | ||
|
|
||
| public init(id: String, resolve: @escaping @Sendable (EnvironmentValues) -> Style) { | ||
| self.id = id | ||
| self.resolve = resolve | ||
| } | ||
|
|
||
| public init(resolve: @escaping @Sendable (EnvironmentValues) -> Style) { | ||
| self.init(id: UUID().uuidString, resolve: resolve) | ||
| } | ||
| } |
There was a problem hiding this comment.
The test verifies that two Resolvers with the same explicit ID are considered equal even though they have different resolve closures (one returns white, one returns black). While this is the intended behavior based on ID-based equality, it creates a potential footgun where developers might accidentally create resolvers with the same ID but different behavior, leading to subtle bugs.
Consider adding documentation to the Resolver struct warning that equality is based solely on ID, not on the actual resolve behavior, and that developers should ensure unique IDs for semantically different resolvers.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12 +/- ##
==========================================
+ Coverage 99.30% 99.35% +0.05%
==========================================
Files 28 30 +2
Lines 1154 1405 +251
==========================================
+ Hits 1146 1396 +250
- Misses 8 9 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This pull request refactors the
ThemeAdaptiveStyletype to support more flexible and serializable theme adaptation by introducing a newDefaultsenum andResolverstruct, replacing the previous hardcoded light/dark variant system. It adds support for adapting by color scheme, size class, or constant value, and updates all related APIs, Codable conformances, and tests accordingly.Core API Redesign:
ThemeAdaptiveStyleto use aDefaultsenum (with.colorScheme,.sizeClass,.valuecases) and aResolverclosure, allowing adaptation by color scheme, size class, or a constant value. The previouslight/darkproperties are now convenience accessors, and the main value is resolved via the environment. (Sources/ThemeKit/ThemeAdaptiveStyle.swift[1]Sources/ThemeKit/ThemeAdaptiveStyle+Defaults.swift[2]Sources/ThemeKit/ThemeAdaptiveStyle+Resolver.swift[3]resolved(for:)method toresolved(in:), which now resolves the style using the fullEnvironmentValuesrather than justColorScheme. (Sources/ThemeKit/ThemeAdaptiveStyle.swiftSources/ThemeKit/ThemeAdaptiveStyle.swiftL3-R93)Codable & Serialization Improvements:
ThemeAdaptiveStyleand itsDefaultsenum, enabling automatic detection of the adaptation axis (color scheme, size class, or value) based on JSON keys. Serialization now fails if a custom (non-default) resolver is used. (Sources/ThemeKit/ThemeAdaptiveStyle.swift[1]Sources/ThemeKit/ThemeAdaptiveStyle+Defaults.swift[2]CLAUDE.md[1] [2]Integration & API Usage Updates:
resolved(for:)toresolved(in:)throughout the codebase, includingShapeStyleconformances, generator code, and tests, to use the new environment-based resolution. (Sources/ThemeKit/ThemeAdaptiveStyle+ShapeStyle.swift[1]Sources/ThemeKitGenerator/ThemeShapeStyleGenerator.swift[2]Sources/ThemeKitGenerator/ThemeShadowedStyleGenerator.swift[3]README.md[4]IMPLEMENTATION_DETAILS.md[5]Testing & Validation:
Tests/ThemeKitTests/ThemeAdaptiveStyleTests.swift[1] [2] [3] [4] [5] [6] [7]Tests/ThemeKitGeneratorTests/ThemeFileGeneratorTests.swiftTests/ThemeKitGeneratorTests/ThemeFileGeneratorTests.swiftL170-R170)Documentation:
CLAUDE.md[1] [2]This refactor provides a more extensible and robust foundation for theme adaptation, supporting both built-in and custom adaptation strategies with full Codable support where possible.