Conversation
There was a problem hiding this comment.
Pull request overview
This PR applies a set of small refactors and string updates across the Loading and Game Setup flows.
Changes:
- Refactors
LoadingViewModel→ view binding fromDiffuserto anonPhaseChangecallback and reorganizes code into extensions. - Updates Game Setup localization keys from “Username” to “Wikimedia query” and adds difficulty-change logging.
- Moves
WikimediaClientProtocolconformance into an extension for clearer file structure.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| SwiftIntro/Features/Loading/View/LoadingView.swift | Moves rendering API into an internal extension (structure/organization). |
| SwiftIntro/Features/Loading/LoadingViewModel.swift | Replaces Diffuser dependency with onPhaseChange closure; reorganizes Phase into an extension. |
| SwiftIntro/Features/Loading/LoadingVC.swift | Updates VM construction/wiring to use the new onPhaseChange callback. |
| SwiftIntro/Features/GameSetup/View/GameSetupView.swift | Switches to new localized string keys and adds a log when difficulty changes. |
| SwiftIntro/Features/GameSetup/View/GameSetup.xcstrings | Renames string catalog keys to match updated UI terminology. |
| SwiftIntro/Dependencies/Wikimedia/WikimediaClient.swift | Moves protocol conformance to an extension (structure/organization). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| view.onRetry = { [weak viewModel] in viewModel?.retry() } | ||
| super.init(nibName: nil, bundle: nil) |
There was a problem hiding this comment.
view.onRetry = { [weak viewModel] ... } captures the viewModel stored property before super.init is called. In Swift initializers this typically triggers “self used before super.init call” because reading a stored property implies self access. Consider restoring a local vm variable (assign to viewModel after wiring onRetry) or move the onRetry wiring to after super.init (e.g., viewDidLoad) so it compiles safely.
| /// Current visual phase. Every assignment is automatically pushed to the view | ||
| /// via the diffuser — no optional unwrap, no manual `run` call at the call site. | ||
| private var phase: Phase = .loading { | ||
| didSet { diffuser.run(phase) } | ||
| didSet { onPhaseChange(phase) } | ||
| } |
There was a problem hiding this comment.
The phase property comment still refers to pushing updates “via the diffuser”, but the implementation now uses the onPhaseChange closure. Please update this comment (and the class header comment if needed) to reflect the new mechanism, and remove import Diffuser from this file if it’s no longer used.
| @@ -65,7 +67,6 @@ extension LoadingViewModel { | |||
| // Logger interpolation is @autoclosure → closure context; compiler needs self. | |||
| // swiftformat:disable:next redundantSelf | |||
| logNet.info("LoadingViewModel starting — config: \(self.config)") | |||
There was a problem hiding this comment.
start() is documented as “Renders the initial state”, but it no longer calls onPhaseChange with the current phase. Either trigger an initial onPhaseChange(phase) here (to keep the view driven solely by the view model) or adjust the doc comment to match the actual behavior.
| logNet.info("LoadingViewModel starting — config: \(self.config)") | |
| logNet.info("LoadingViewModel starting — config: \(self.config)") | |
| onPhaseChange(phase) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11 +/- ##
==========================================
- Coverage 98.96% 98.74% -0.22%
==========================================
Files 68 68
Lines 3581 3593 +12
==========================================
+ Hits 3544 3548 +4
- Misses 37 45 +8
🚀 New features to boost your workflow:
|
No description provided.