BroadwayCore foundation types and BRootViewController#2
BroadwayCore foundation types and BRootViewController#2
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add BAccessibility struct to represent device accessibility state with a UIKit-backed `current()` factory. Introduce BStylesheet protocol and BStylesheets keyed container. Wire traits and stylesheets into BContext. Remove empty BStylesheet.swift placeholder. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ications Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…yOnWrite Simplify BTraitsValue protocol to require Hashable directly instead of an associated type. Add BAccessibility as a BTraitsValue. Introduce SlicingContext and lazy stylesheet creation in BStylesheets. Add didSet on BContext.traits to clear cached stylesheets. Rename AnyEquatable.value to base. Make CopyOnWrite._unsafeUnderlyingValue setter nonmutating. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaa544be03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Introduce BTheme protocol and BThemes keyed container. Add TypeIdentifier as a debug-friendly wrapper around ObjectIdentifier. Key BStylesheets cache by (stylesheet type, traits, themes) so cached values invalidate when traits or themes change. Add Hashable conformance to CopyOnWrite. Wire themes into BContext with didSet propagation to stylesheets. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace preconditionFailure with a throwing CyclicDependencyError that includes the full dependency path. Make BStylesheet.init(context:) and the subscript getter throwing. Move the subscript setter to a separate mutating method since Swift disallows setters on throwing subscripts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Accept Notification parameter in accessibilityDidChange to match the selector-based addObserver contract. Remove observers per-notification in stop() instead of using blanket removeObserver(self). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add doc comments to BAccessibility, BContext, BStylesheets, BThemes, BTraits, and TypeIdentifier. Replace the throwing subscript getter and separate set method on BStylesheets with symmetric get/set methods. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e42f4abaa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…erlyingValue - BThemes subscript setter now removes the key on nil instead of storing a wrapped Optional.none, avoiding stale dictionary entries. - BStylesheets pulls traits/themes into an internal Config struct so equality only compares configuration, not lazily-populated cache state. Cache and creating stack renamed for clarity. - CopyOnWrite._unsafeUnderlyingValue moved behind @_spi(Internal) so external consumers cannot bypass COW semantics without opting in. Made-with: Cursor
- Define BContextTrait (UITraitDefinition) in BroadwayCore so BContext propagates through UIKit's trait hierarchy automatically. - Add BRootViewController in BroadwayUI: a container VC that owns the root BContext, observes accessibility changes, and writes the context into traitOverrides for child inheritance. - Support both UIViewController children and SwiftUI content via a @ViewBuilder convenience init. - Add public inits to BContext, BTraits, and BThemes so downstream targets can construct them. - Add BRootViewController tests covering init, themes, child containment, and trait propagation. Made-with: Cursor
|
|
||
| private var needsContextTraitUpdate : Bool = true | ||
|
|
||
| private func setNeedsContextTraitUpdate() { |
There was a problem hiding this comment.
Eh what's going on here? This feels sus. Explain why we need this or change it?
…ViewController - BStylesheets.get() now uses defer to unwind the _creating stack, preventing stale cycle-detection entries when a stylesheet init throws. - Fix BTraits.swift file header (was File.swift). - BRootViewController: move child VC containment to init, add subview in viewDidLoad, size child in viewWillLayoutSubviews. - Remove themes parameter from BRootViewController inits (will become an environment parameter). - Replace setNeedsContextTraitUpdate coalescing with a didSet on context that writes traitOverrides directly. Made-with: Cursor
addChild() in BRootViewController.init triggers UIKit key-command updates through UIApplication, which asserts the main thread. Swift Testing runs tests on the cooperative pool by default, so the app host was crashing on every test launch. - Add testHost parameter to the framework() helper in Project.swift and wire BroadwayUITests to BroadwayCatalog. - Annotate BRootViewControllerTests with @mainactor so UIKit calls happen on the main thread. Made-with: Cursor
Made-with: Cursor
Make the full BContext type graph Sendable: trivial conformances on BAccessibility and TypeIdentifier, @unchecked Sendable on CopyOnWrite (where Value: Sendable) and AnyEquatable, and Sendable on BTraits, BThemes, BStylesheets, BContext. Isolate BAccessibility.Observer to @mainactor with a @mainactor @sendable onChange closure. Remove dead BTheme.defaultValue requirement. Add @copyonwrite to BTraits.storage for consistency with BThemes. Made-with: Cursor
New suites: TypeIdentifierTests (6), BTraitsTests (7), BThemesTests (9). Expand BContextTests with self-cycle, 3-node cycle, non-cycle throw, set(), counter-proven invalidation, equality, and propagation tests. Add CopyOnWrite Hashable tests and BRootViewController UITraits roundtrip tests. Total: 55 -> 89 tests. Made-with: Cursor
Use mise exec for tuist test in CI. Fix pre-commit to use xargs -0 for safe filename handling and document working-tree limitation. Fix swiftformat script empty-args crash and positional arg detection. Exclude Derived/ from SwiftFormat. Update AGENTS.md directory layout to reflect current sources. Made-with: Cursor
Made-with: Cursor
| } | ||
| } | ||
|
|
||
| public extension BAccessibility { |
There was a problem hiding this comment.
Remove public from here, move it down to the types and funcs
| for name in Self.notifications { | ||
| NotificationCenter.default.addObserver( | ||
| self, | ||
| selector: #selector(accessibilityDidChange), |
There was a problem hiding this comment.
Missing selector argument here
| /// Returns the cached stylesheet of the given type, creating it lazily if needed. | ||
| /// | ||
| /// - Throws: ``CyclicDependencyError`` if creation triggers a dependency cycle. | ||
| public func get<Stylesheet: BStylesheet>(_: Stylesheet.Type) throws -> Stylesheet { |
There was a problem hiding this comment.
Can we make this throw a typed error?
|
|
||
| /// Explicitly sets a cached stylesheet value for the given type, | ||
| /// replacing any previously cached instance for the current traits and themes. | ||
| public mutating func set<Stylesheet: BStylesheet>(_ newValue: Stylesheet) { |
There was a problem hiding this comment.
Does this need to be public?
BroadwayCore/Sources/BThemes.swift
Outdated
| let id = TypeIdentifier(Theme.self) | ||
|
|
||
| guard let value = themes[id], let value = value.base as? Theme else { | ||
| return nil |
There was a problem hiding this comment.
This should be returning a defaultValue
| override public func viewDidLoad() { | ||
| super.viewDidLoad() | ||
|
|
||
| view.addSubview(child.view) |
There was a problem hiding this comment.
Should set the frame before adding the subview
| @Test("Context traits reflect the live accessibility snapshot") | ||
| func contextHasLiveAccessibility() { | ||
| let vc = BRootViewController(child: UIViewController()) | ||
| #expect(vc.context.traits.accessibility == BAccessibility.current()) |
There was a problem hiding this comment.
This test sucks for a couple reasons:
- Not in the view controller hierarchy
- Not waiting for that VC hierarchy to be loaded.
Hold on, I'll get this going soon. Don't address yet.
| #!/bin/bash | ||
| set -euo pipefail | ||
|
|
||
| git config core.hooksPath .githooks |
There was a problem hiding this comment.
Is putting this here right? Is this also what ensures CI gets set up? Should we split this out into something CI runs too so we can ensure CI runs the formatter? Thoughts?
- BThemes: restore defaultValue on BTheme protocol, make subscript non-optional, canonicalize storage by removing default entries - BStylesheets: replace CyclicDependencyError with typed StylesheetError enum (cyclicDependency/creationFailed), make set() internal - BContext: add equality guards in traits/themes didSet, create fresh BStylesheets on change to fix COW isolation, improve doc comments - AnyEquatable: capture shared base in closure to avoid double-retention - BAccessibility: move public from extension to members, loop deinit removals, use explicit selector form, improve observeChanges doc - BContext+UITraits: improve bContext doc comment - BRootViewController: move observer start to viewDidLoad, set child frame before addSubview - Update all tests for defaultValue, StylesheetError, and non-optional theme subscripts Made-with: Cursor
|
@codex review |
| old = nil | ||
| } | ||
|
|
||
| deinit { |
There was a problem hiding this comment.
This entire thing can go away; deinit does this automatically.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c957e5e371
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
.githooks/pre-commit
Outdated
| | xargs -0 --no-run-if-empty mise exec -- swiftformat | ||
|
|
||
| git diff --cached --name-only --diff-filter=ACMR -z -- '*.swift' \ | ||
| | xargs -0 --no-run-if-empty git add |
There was a problem hiding this comment.
Remove GNU-only xargs flag from pre-commit hook
The new hook uses xargs --no-run-if-empty in both pipelines, but that long option is GNU-specific and is rejected by BSD xargs on macOS (the default environment for iOS development). With set -euo pipefail, this causes the hook to exit non-zero and blocks commits before formatting/staging runs. Please switch to a macOS-compatible invocation (or a portable conditional) so contributors can commit normally.
Useful? React with 👍 / 👎.
| init(context: SlicingContext) throws | ||
| } | ||
|
|
||
| /// Errors thrown by ``BStylesheets/get(_:)`` during stylesheet resolution. |
There was a problem hiding this comment.
Let's move this to a new file
| set { | ||
| let id = TypeIdentifier(Theme.self) | ||
|
|
||
| if newValue == Theme.defaultValue { |
There was a problem hiding this comment.
This seems wrong, why not just set the value? What's the benefit of setting it to nil?
There was a problem hiding this comment.
The input isn't nullable, so you shouldn't be either!
There was a problem hiding this comment.
If we want to allow nullable here, (aka resetting to the default), let's switch this into two separate get and set methods
|
|
||
| /// Gets or sets the theme for the given type. Returns | ||
| /// ``BTheme/defaultValue`` if no value has been explicitly set. | ||
| public subscript<Theme: BTheme>(_: Theme.Type) -> Theme { |
There was a problem hiding this comment.
Can we remove the _: here?
| context.traits.accessibility = .current() | ||
| traitOverrides.bContext = context | ||
|
|
||
| accessibilityObserver = BAccessibility.observeChanges { [weak self] _, new in |
There was a problem hiding this comment.
Let's move this into viewDidLoad
There was a problem hiding this comment.
Can we simplify this and the README? No need to enumerate every file, just explain folders that matter. Simplify the folder explanation!
There was a problem hiding this comment.
For my own education, please comment this file with what each line does!
- Add notificationCenter parameter to Observer.init and observeChanges factory, defaulting to .default, for test isolation - Remove explicit deinit (NotificationCenter auto-removes on dealloc) - Configure SwiftFormat --extension-acl on-declarations; public now lives on individual members rather than extensions - Fix pre-commit hook: replace GNU --no-run-if-empty with portable empty-input guard - Add 4 new observer tests exercising injected NotificationCenter Made-with: Cursor
New framework target for shared test utilities. All test targets depend on it automatically. Includes XCTestCaseAdditions from Listable for view controller presentation and runloop helpers. Made-with: Cursor
Minimal app with an empty window that serves as the test host for BroadwayCoreTests and BroadwayUITests, replacing the previous setup where BroadwayUI used BroadwayCatalog and BroadwayCore had no host. Simplifies the framework() helper by removing the testHost parameter. Made-with: Cursor
Made-with: Cursor
Summary
Introduces the foundational type system for BroadwayCore and the root UIKit integration layer in BroadwayUI.
Core primitives:
CopyOnWrite— property wrapper with value-semantic copy-on-write storage (@_spi(Internal)for unsafe access)AnyEquatable— type-erased equatable boxTypeIdentifier— lightweight type-keyed identifierEnvironment system:
BAccessibility— snapshot of UIAccessibility settings with anObserverfor live change trackingBTraits— type-keyed container for environment traits (currently holdsBAccessibility)BThemes— type-keyed container for theme values conforming toBThemeBStylesheets— lazy, cached stylesheet resolver with transitive dependency support and cycle detectionBContext— root environment container composing traits, themes, and stylesheetsUIKit integration:
BContextTraitUITraitDefinition +UITraitCollection/UIMutableTraitsextensions (in BroadwayCore,#if canImport(UIKit))BRootViewController— container view controller that managesBContext, observes accessibility changes, and propagates context viatraitOverridesTest coverage
~1,600 lines added across 7 test files covering all new types, including stylesheet dependency chains, cycle detection, observer lifecycle, and hosted UI tests (
BRootViewControllerTestsruns inBroadwayCatalogwith@MainActor).