BTraitsObserver: protocol-driven unified trait observation#4
BTraitsObserver: protocol-driven unified trait observation#4
Conversation
Show only directories instead of enumerating individual files. Add missing BroadwayTestHost and BroadwayTesting to README. Made-with: Cursor
Add BTraitsValueObserver protocol and associated type to BTraitsValue so each trait declares its own observer. BTraitsObserver coordinates them behind a single start/stop interface, simplifying BRootViewController. Remove BAccessibility.observe(_:) factory in favor of direct Observer init. Made-with: Cursor
BTraits now owns the set of registered trait types via register<V>() and a .system factory. BTraitsObserver reads registrations and the UIViewController hierarchy from the BTraits instance instead of hardcoding types. initialValue replaced with currentValue(from:) for future injectability. EquatableIgnored gains Hashable conformance. Made-with: Cursor
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4b306ccf5
ℹ️ 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".
| init( | ||
| notificationCenter: NotificationCenter, | ||
| settingsProvider: SettingsProvider, | ||
| notificationCenter: NotificationCenter = .default, | ||
| settingsProvider: any SettingsProvider = BAccessibility.systemSettings, | ||
| onChange: @MainActor @escaping @Sendable (BAccessibility, BAccessibility) -> Void, |
There was a problem hiding this comment.
This change removes the public BAccessibility.observe(...) factory, but the replacement initializer remains internal (init without public), which means modules outside BroadwayCore no longer have any public way to construct an accessibility observer. That is a source-breaking regression for existing consumers that previously observed accessibility updates; please make the initializer public (or keep a public factory) to preserve external usage.
Useful? React with 👍 / 👎.
| public func start() { | ||
| for observer in observers { | ||
| observer.start() |
There was a problem hiding this comment.
BTraitsObserver documents start() as safe to call multiple times, but this implementation always forwards start() to every underlying observer with no guard at the coordinator level. Custom BTraitsValueObserver types are public and may not be idempotent, so repeated start() calls can create duplicate subscriptions/callbacks; add an internal started-state check so the documented behavior is guaranteed.
Useful? React with 👍 / 👎.
Defer child VC creation, trait observation, and context setup until the controller enters a valid view hierarchy (viewIsAppearing). Replace the XCTestCase extension with standalone free functions compatible with Swift Testing. Made-with: Cursor
…e BAccessibility trait conformance into the BAccessibility file.
…ewController Move BTraits, BTraitsObserver, and BAccessibility into BroadwayCore/Sources/Traits/. Extract trait value conformances into Values/. Add BTraits+Overrides for override support and BTraitOverridesViewController for propagating trait overrides through the view hierarchy. Made-with: Cursor
…gnored for caches Flatten BStylesheets to store traits/themes directly instead of through a Config struct. Mark cache and creating arrays with @EquatableIgnored so equality is based solely on traits and themes. Add @unchecked Sendable to EquatableIgnored and mark stylesheets as @EquatableIgnored on BContext. Made-with: Cursor
1 participant
Summary
BTraitsValueObserverprotocol andObserverassociated type onBTraitsValue, so each trait type declares its own observer. ANeverObserverdefault means non-observed traits need no extra work.BTraitsObserveras a unified coordinator that reads registered trait types fromBTraits, initializes values from the view controller hierarchy viacurrentValue(from:), and manages per-type observers behind a singlestart()/stop()lifecycle.BTraitsgains a registration system (.systemfactory,register<V>(),readCurrentValues(from:)) so trait type configuration is centralized and extensible.BRootViewControlleris simplified to a singleBTraitsObserver(traits: .system, from: self)call, removing directBAccessibility.Observermanagement.BAccessibility.observe(on:with:_:)factory in favor of directObserverinit with defaults.AGENTS.mdandREADME.md.Test plan
BTraitsObserverTestscover initial value population and lifecycle safetyBAccessibilityObserverTestsupdated to useObserverinit directlyBRootViewControllerTestspass unchanged (behavioral parity)Made with Cursor