feat: add adaptive SwiftUI colours and text styles - #10
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe SwiftUI package adds an adaptive palette generated from light and dark tokens, a public shape style, and three Dynamic Type-aware font styles backed by bundled fonts. Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to Custom text styles can fall back to a system font if registration fails. This is a bounded issue to fix or explicitly accept before release. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 5 files. (2 skipped: 2 unsupported.)
Comment |
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
platforms/swiftui/Sources/Origin89UI/Theme.swift-32-32 (1)
32-32: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle non-duplicate font registration failures.
The bundled font files exist, and the font test confirms the normal registration path. However,
CTFontManagerRegisterFontsForURLcan returnfalsefor errors other thanCTFontManagerError.alreadyRegistered. This closure ignores that result and marks registration complete, so laterFont.origin89*calls do not retry and can use a fallback font. Capture and report theCFError, while treatingalreadyRegisteredas success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Advanced
Run ID: fd0d439f-cb1b-4b6f-9fe4-17cd3ec0ca83
📒 Files selected for processing (7)
README.mdbrand-provenance.jsonplatforms/swiftui/Sources/Origin89UI/GeneratedTokens.swiftplatforms/swiftui/Sources/Origin89UI/Reading.swiftplatforms/swiftui/Sources/Origin89UI/Theme.swiftplatforms/swiftui/Tests/Origin89UITests/ThemeTests.swiftscripts/generate-brand.mjs
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
Swift apps could read
Origin89Tokens.lightand.dark, but every view had to choose between them fromcolorScheme, and fonts were only available as PostScript name strings. The Apps setup flow (origin89hq/apps#1) needs brand colours and type in its own screens.The generator now also emits
Origin89Tokens.adaptive, whose colours resolve per appearance throughUIColor/NSColorproviders, so it stays generated from@origin89/brandand covered bybrand:check. Views write.foregroundStyle(.origin89.muted)orColor.origin89.surface.Font.origin89Label,.origin89Valueand.origin89Datawrap the bundled faces with Dynamic Type scaling and register them on first use. Registration is now a once-per-process static, soOrigin89Fonts.register()no longer needs the main actor.Origin89Readinguses the new API with no visual change. It needs no changeset: after #11 the Swift package is released withjust swift-release.Validation:
swift test(6 tests: every palette key against the fixed light and dark values, the shorthand, and that the bundled fonts load by name), the iOS 17 Simulator build, andpnpm checkpass. Swapping light and dark in the provider fails 33 expectations. The colour tests run on the main actor: AppKit never returns fromresolve(in:)for appearance-backed colours off the main thread, includingNSColor.labelColor.