Embedded browser #332
Embedded browser #332
Conversation
WalkthroughReplaces the onboarding feature with a WhatsNew flow, adds an embedded in-app browser and comments drawer, introduces a LinkBrowserMode enum replacing a boolean setting, and applies UI/component refactors across app, domain, data, design system and tests. (47 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ContentView
participant WhatsNewCoordinator
participant FeedView
participant NavigationStore
participant BrowserController
participant EmbeddedWebView
participant PostCommentsSheet
User->>ContentView: Launch app / interact
ContentView->>WhatsNewCoordinator: shouldShowWhatsNew()
alt showWhatsNew == true
WhatsNewCoordinator-->>ContentView: show view
ContentView->>WhatsNewCoordinator: makeWhatsNewView(onDismiss)
ContentView->>ContentView: present WhatsNew sheet
end
User->>FeedView: Tap a post link
FeedView->>NavigationStore: showPostLink(post)
NavigationStore->>BrowserController: set embedded URL / create controller
BrowserController->>EmbeddedWebView: load(URL)
User->>EmbeddedWebView: open comments
EmbeddedWebView->>PostCommentsSheet: present drawer
PostCommentsSheet->>PostCommentsSheet: expand/collapse, show comments
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
weiran
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Features/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swift (1)
72-92: Add@MainActorand remove unnecessary availability check – iOS 26 is the minimum deployment target.The
#available(iOS 26.0, *)check should be removed entirely sincePackage.swiftspecifies.iOS(.v26)as the platform requirement. This creates unreachable fallback code (lines 82–92) that will never execute. Use.glassEffectunconditionally.Additionally, the struct is missing the
@MainActorannotation required for UI code inFeatures/**/*.swiftper coding guidelines.Features/Feed/Sources/Feed/FeedView.swift (1)
367-388: Unreachable code after early return.Lines 379-386 will never execute when
mode == .customBrowserand the device is not an iPad, because line 376 returns early. This appears to be a logic error where the sidebar handling andopenURLInPrimaryContextfallback should only occur when the custom browser path isn't taken.🐛 Proposed fix
private func handleLinkTap(post: Domain.Post) { guard !isHackerNewsItemURL(post.url) else { navigationStore.showPost(post) return } let mode = DependencyContainer.shared.getSettingsUseCase().linkBrowserMode if mode == .customBrowser, UIDevice.current.userInterfaceIdiom != .pad { navigationStore.showPostLink(post) return } if isSidebar { navigationStore.showPost(post) selectedPostId = post.id + return } if navigationStore.openURLInPrimaryContext(post.url, pushOntoDetailStack: !isSidebar) { return } LinkOpener.openURL(post.url, with: post) }Alternatively, if the sidebar logic should execute alongside the URL opening, the control flow needs restructuring.
🤖 Fix all issues with AI agents
In @App/EmbeddedWebView.swift:
- Around line 428-456: Replace the deprecated NavigationView usage with
NavigationStack: swap the NavigationView { ... } wrapper around CommentsView to
NavigationStack { ... }, remove the .navigationViewStyle(.stack) modifier, and
keep the existing modifiers (.scrollDisabled, .toolbar,
.toolbar(showsExpandedToolbar ? .visible : .hidden, for: .navigationBar),
.padding(.top, ...), .opacity(...), .allowsHitTesting(...)) applied to the
NavigationStack so toolbar items (ToolbarItem with placement:
.navigationBarLeading and the Back button) and visibility behavior remain
unchanged; verify the Back button action (onDismiss) and bindings
(isScrollAtTop, isExpanded) are passed through unchanged.
In @Data/Sources/Data/PostRepository+Voting.swift:
- Around line 112-117: The throwing expression in the for-where clause of
linkWithExactText(_:, in:) causes a compile error; move the try out of the where
and perform the throwing call inside the loop body: iterate over links, call let
linkText = try link.text() (inside the loop), then compare
linkText.localizedCaseInsensitiveCompare(text) == .orderedSame and return link
when it matches; keep the function's throws signature so the thrown error
propagates.
- Around line 105-110: The for-where clause uses a throwing expression (try
link.attr("id")) which won't compile; change linkWithIDPrefix(_:, in:) to
perform the try inside the loop body instead: iterate with "for link in links {
... }", call let id = try link.attr("id") (or use a do-catch if you want to
ignore individual errors), then check if id.starts(with: prefix) and return link
if so, otherwise continue; keep the function throwing so callers still receive
propagated errors.
In @Features/Comments/Sources/Comments/CommentRow.swift:
- Around line 23-31: The computed property baseCommentText mutates
comment.parsedText during view evaluation, which can break SwiftUI; remove
mutation from baseCommentText and make it purely a getter returning
comment.parsedText ?? CommentHTMLParser.parseHTMLText(comment.text) without
assigning, then perform the caching assignment elsewhere (e.g., in the view’s
.task(id: comment.id) or .onAppear) by checking if comment.parsedText == nil and
setting comment.parsedText = CommentHTMLParser.parseHTMLText(comment.text);
ensure references to baseCommentText, comment.parsedText, and
CommentHTMLParser.parseHTMLText are updated accordingly.
In @Features/Feed/Tests/FeedTests/FeedViewTests.swift:
- Around line 33-35: The mock function showPostLink(_ post: Post) only assigns
selectedPostId; update it to also assign selectedPost = post so it matches the
other test mocks (e.g., in SettingsViewTests.swift and
VotingViewModelTests.swift) and ensures tests that rely on selectedPost being
populated will behave consistently; locate the mock implementation of
showPostLink and add the selectedPost assignment alongside selectedPostId.
In @Features/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swift:
- Around line 34-37: The WhatsNewData title is out of date — change the
hardcoded title string in the WhatsNewData(...) return (the title: "What's New
in Hackers 5.2.1") to match the project's MARKETING_VERSION (e.g., "What's New
in Hackers 5.3.0"); update the title value inside the WhatsNewData initializer
so the displayed release version matches the Xcode project version.
🧹 Nitpick comments (23)
DesignSystem/Sources/DesignSystem/Components/MailView.swift (1)
50-71: Add@MainActorand@unchecked Sendableto fix concurrency safety.The
nonisolateddelegate method capturesparentCopy(containing a non-Sendable@Binding) and passes it across isolation boundaries into a@MainActorTask. This will trigger Swift 6 strict concurrency warnings.Since SwiftUI coordinators are always accessed on the main thread, marking the class
@MainActorwith@unchecked Sendableis the idiomatic fix. This also removes the need for the intermediateparentCopyvariable.♻️ Suggested refactor
- public class Coordinator: NSObject, MFMailComposeViewControllerDelegate { + @MainActor + public class Coordinator: NSObject, MFMailComposeViewControllerDelegate, @unchecked Sendable { var parent: MailView init(_ parent: MailView) { self.parent = parent } nonisolated public func mailComposeController( _ controller: MFMailComposeViewController, didFinishWith result: MFMailComposeResult, error: Error?, ) { - let parentCopy = parent Task { @MainActor in controller.dismiss(animated: true) if let error { - parentCopy.result = .failure(error) + self.parent.result = .failure(error) } else { - parentCopy.result = .success(result) + self.parent.result = .success(result) } } } }As per coding guidelines, Sendable conformance should be ensured for thread safety in Swift files.
App/WhatsNewCoordinator.swift (1)
39-43: Consider the fallback behaviour for missing version.When
CFBundleShortVersionStringis missing, the fallback to"0"means the WhatsNew screen will be shown once (since"0"differs from any real version) and then not shown again until the next minor release. This is likely acceptable, but verify this edge case is intentional.DesignSystem/Sources/DesignSystem/Components/ThumbnailView.swift (1)
12-14: Consider adding@MainActorannotation to the view.Per coding guidelines, UI code in
{Features,DesignSystem}/**/*.swiftshould apply@MainActorannotation. While the asyncloadThumbnailfunction correctly usesMainActor.runfor state updates, annotating the struct ensures all UI operations are main-actor-isolated.♻️ Suggested change
+@MainActor public struct ThumbnailView: View { let url: URL? let isEnabled: BoolFeatures/Comments/Sources/Comments/CommentRow.swift (2)
191-231: Cache grows unboundedly with comment IDs.
StyledCommentTextCachestores entries keyed by(commentID, textScaling)without eviction. As users browse many posts, this cache can accumulate significant memory. Consider usingNSCacheor implementing an LRU eviction policy.♻️ NSCache-based alternative
@MainActor private enum StyledCommentTextCache { private final class CacheEntry { let base: AttributedString let styled: AttributedString init(base: AttributedString, styled: AttributedString) { self.base = base self.styled = styled } } private static let cache = NSCache<NSString, CacheEntry>() static func text(commentID: Int, textScaling: CGFloat, baseText: AttributedString) -> AttributedString { let key = "\(commentID)-\(textScaling)" as NSString if let cached = cache.object(forKey: key), cached.base == baseText { return cached.styled } // ... styling logic ... cache.setObject(CacheEntry(base: baseText, styled: attributed), forKey: key) return attributed } }
15-21: Consider adding@MainActorannotation.Per coding guidelines for
{Features,DesignSystem}/**/*.swift, UI code should apply@MainActorannotation. This ensures all view operations are main-actor-isolated.♻️ Suggested change
+@MainActor struct CommentRow: View { @Environment(\.textScaling) private var textScaling let post: PostFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swift (1)
11-18: Consider adding@MainActorannotation.Per coding guidelines for
{Features,DesignSystem}/**/*.swift, UI code should apply@MainActorannotation to ensure main-actor isolation.♻️ Suggested change
+@MainActor public struct WhatsNewView: View { private let whatsNewData: WhatsNewData private let onDismiss: () -> VoidDesignSystem/Sources/DesignSystem/Components/PostDisplayView+Pills.swift (1)
269-287: URL string manipulation for unvote URL synthesis is fragile.The logic handles both encoded (
how%3Dup) and unencoded (how=up) query parameters, but string replacement could produce invalid URLs if the pattern appears elsewhere in the URL or if the API changes its format.Consider using
URLComponentsfor safer query parameter manipulation:♻️ Suggested refactor using URLComponents
func derivedVoteLinks(afterUpvoteFrom voteLinks: VoteLinks?) -> VoteLinks? { guard let voteLinks else { return nil } if voteLinks.unvote != nil { return voteLinks } guard let upvoteURL = voteLinks.upvote else { return voteLinks } - let absolute = upvoteURL.absoluteString - if absolute.contains("how=up"), - let unvoteURL = URL(string: absolute.replacingOccurrences(of: "how=up", with: "how=un")) { - return VoteLinks(upvote: upvoteURL, unvote: unvoteURL) - } - if absolute.contains("how%3Dup"), - let unvoteURL = URL(string: absolute.replacingOccurrences(of: "how%3Dup", with: "how%3Dun")) { - return VoteLinks(upvote: upvoteURL, unvote: unvoteURL) - } - return voteLinks + guard var components = URLComponents(url: upvoteURL, resolvingAgainstBaseURL: false) else { + return voteLinks + } + guard var queryItems = components.queryItems else { + return voteLinks + } + guard let howIndex = queryItems.firstIndex(where: { $0.name == "how" && $0.value == "up" }) else { + return voteLinks + } + queryItems[howIndex] = URLQueryItem(name: "how", value: "un") + components.queryItems = queryItems + guard let unvoteURL = components.url else { + return voteLinks + } + return VoteLinks(upvote: upvoteURL, unvote: unvoteURL) }Features/Comments/Sources/Comments/CommentsComponents.swift (1)
99-102: DuplicateshouldShowVoteActionslogic.The same condition appears twice—once for
Postand once forComment. Consider extracting to a shared helper or protocol extension onVotableto reduce duplication.♻️ Potential refactor
// In a shared location or extension on Votable extension Votable { var shouldShowVoteActions: Bool { (voteLinks?.upvote != nil && !upvoted) || (voteLinks?.unvote != nil && upvoted) } }Also applies to: 242-245
App/EmbeddedWebView.swift (2)
189-199: Consider handling navigation stream errors more explicitly.The empty catch block silently ignores all navigation stream errors. While the comment explains the intent, logging unexpected errors could aid debugging.
} catch { - // Ignore navigation stream errors; state updates happen on successful events. + // Navigation stream errors are expected when the view is dismissed. + // Log unexpected errors for debugging if needed. }
377-389: PreferTask.sleepoverDispatchQueue.main.asyncAfterfor consistency.Using
DispatchQueue.main.asyncAfterworks but mixing dispatch queue patterns with Swift concurrency can be confusing. Consider usingTask.sleepfor consistency with the rest of the codebase.♻️ Suggested refactor
.onChange(of: isExpanded) { _, newValue in if newValue { showsExpandedToolbar = false - DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { - guard isExpanded else { return } - withAnimation(.easeInOut(duration: 0.2)) { - showsExpandedToolbar = true + Task { @MainActor in + try? await Task.sleep(for: .milliseconds(150)) + guard isExpanded else { return } + withAnimation(.easeInOut(duration: 0.2)) { + showsExpandedToolbar = true + } } - } } else { showsExpandedToolbar = false } }Features/Settings/Tests/SettingsTests/SettingsViewTests.swift (1)
87-89: Section renamed from Onboarding to WhatsNew.The comment marker update reflects the PR's renaming of the Onboarding module to WhatsNew. The section currently has no tests—consider adding tests for the WhatsNew functionality.
Would you like me to help generate test cases for the WhatsNew functionality?
Domain/Sources/Domain/SettingsUseCase.swift (1)
50-54: Consider moving the enum definition before the protocol.The
LinkBrowserModeenum is referenced bySettingsUseCaseprotocol on line 40, but defined afterwards. While Swift allows forward references, placing type definitions before their usage improves readability.The enum itself is well-designed with
Sendableconformance for thread safety andCaseIterablefor UI enumeration.Features/Feed/Sources/Feed/FeedViewModel.swift (1)
330-333: Redundant@MainActorannotation on Task.Since
FeedViewModelis already annotated with@MainActor, the explicit@MainActoron the nested Task is unnecessary. The Task will inherit the actor context.Suggested simplification
- Task { @MainActor [weak self] in + Task { [weak self] in guard let self else { return } await self.handleBookmarksUpdate(postId: postId, isBookmarked: isBookmarked) }Data/Tests/DataTests/WhatsNewRepositoryTests.swift (1)
116-124: Good test isolation pattern for UserDefaults.The UUID-based suite name ensures test isolation. Note that
synchronize()is deprecated as of iOS 12, though it still functions. Modern UserDefaults writes are automatically persisted.Consider removing deprecated synchronize call
defaults.removePersistentDomain(forName: suiteName) - defaults.synchronize() return defaultsDomain/Tests/DomainTests/UseCaseTests.swift (1)
186-194: Test setslinkBrowserModeto its default value.The test assigns
.customBrowserwhich is already the default value inInMemoryUserDefaults. Consider testing mutation to a different value (e.g.,.systemBrowseror.inAppBrowser) to verify the setter works correctly.Suggested test improvement
settingsUseCase.safariReaderMode = true - settingsUseCase.linkBrowserMode = .customBrowser + settingsUseCase.linkBrowserMode = .systemBrowser settingsUseCase.showThumbnails = false settingsUseCase.rememberFeedCategory = true settingsUseCase.lastFeedCategory = .ask settingsUseCase.textSize = .large #expect(settingsUseCase.safariReaderMode == true, "actual: \(settingsUseCase.safariReaderMode)") - #expect(settingsUseCase.linkBrowserMode == .customBrowser, "actual: \(settingsUseCase.linkBrowserMode)") + #expect(settingsUseCase.linkBrowserMode == .systemBrowser, "actual: \(settingsUseCase.linkBrowserMode)")Features/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift (1)
16-16: Consider the coupling between test assertion and hardcoded version string.The test asserts against
"What's New in Hackers 5.2.1", which matches the current implementation inWhatsNewData.currentWhatsNew(). However, this creates tight coupling — when the version changes, both the implementation and test must be updated together.This is acceptable for verifying the exact content, but be aware that the PR description mentions a version bump to 5.3.0, so you may need to update this title in both places.
Features/Settings/Sources/Settings/SettingsViewModel.swift (2)
62-99: Multiple overloads work but could be consolidated.The three
propagateChangesIfNeededoverloads each contain a switch with one meaningful case. This is functional but somewhat verbose. An alternative would be a single generic method or a more direct approach in thedidSetobservers.That said, the current approach is explicit and easy to follow, so this is optional.
♻️ Alternative: Direct propagation in didSet
- public var linkBrowserMode: LinkBrowserMode = .inAppBrowser { - didSet { propagateChangesIfNeeded(\.linkBrowserMode, linkBrowserMode) } + public var linkBrowserMode: LinkBrowserMode = .inAppBrowser { + didSet { + guard hasLoadedSettings else { return } + settingsUseCase.linkBrowserMode = linkBrowserMode + } }This eliminates the need for the overload, though it trades off consistency with other properties.
108-113: RedundantMainActor.runwithin@MainActorclass.Since
SettingsViewModelis already annotated with@MainActor, theawait MainActor.runcall on line 112 is unnecessary — you're already on the main actor.♻️ Proposed fix
public func refreshCacheUsage() { Task { [weak self] in guard let self else { return } let bytes = await settingsUseCase.cacheUsageBytes() let formatted = ByteCountFormatter.string(fromByteCount: bytes, countStyle: .file) - await MainActor.run { self.cacheUsageText = formatted } + self.cacheUsageText = formatted } }Features/Feed/Sources/Feed/PostRowView.swift (1)
43-84: Consider extracting the duplicatePostDisplayViewconfiguration.The
PostDisplayViewis configured identically in both branches (lines 46-60 and 67-81), differing only in the outerButtonwrapper. Consider extracting the shared configuration to reduce duplication.♻️ Suggested refactor
+ @ViewBuilder + private var postDisplayView: some View { + PostDisplayView( + post: post, + votingState: votingViewModel.votingState(for: post), + showPostText: false, + showThumbnails: showThumbnails, + compactMode: compactMode, + onThumbnailTap: onLinkTap, + onUpvoteTap: { await handleUpvoteTap() }, + onUnvoteTap: { await handleUnvoteTap() }, + onBookmarkTap: { + guard let onBookmarkToggle else { return post.isBookmarked } + return await onBookmarkToggle() + }, + onCommentsTap: onCommentsTap + ) + .contentShape(Rectangle()) + } + var body: some View { if let onCommentsTap { Button(action: onCommentsTap) { - PostDisplayView(...) - .contentShape(Rectangle()) + postDisplayView } .buttonStyle(.plain) .accessibilityAddTraits(.isButton) .accessibilityHint("Open comments") } else { - PostDisplayView(...) - .contentShape(Rectangle()) + postDisplayView } }Features/Comments/Sources/Comments/CommentsView.swift (1)
184-199: Consider extracting shared link-handling logic.The
handleLinkTaplogic (lines 189-198) closely mirrorsFeedView.handleLinkTap(checkinglinkBrowserMode, device idiom, and callingnavigationStore.showPostLink). Consider extracting this into a shared helper to reduce duplication and ensure consistent behaviour.DesignSystem/Sources/DesignSystem/Components/PostDisplayView.swift (2)
13-13: UnusedcolorSchemeenvironment variable.The
colorSchemeenvironment variable is declared but does not appear to be used anywhere in the visible code. If it's intended for future use, consider removing it until needed to avoid confusion.
248-251: Duplicate helper function across files.The
isHackerNewsItemURLfunction is duplicated here and inFeatures/Feed/Sources/Feed/FeedView.swift(lines 389-392). Consider extracting this to a shared utility in theSharedmodule to avoid duplication.App/ContentView.swift (1)
165-165: Consider using@Stateinitialisation pattern for consistency.The
settingsViewModelis marked with@State varbut isn't initialised via theState(initialValue:)pattern in an init. SinceAdaptiveSplitViewreceivessettingsViewModelas a parameter, this should likely be aletconstant instead, or the view should own its own instance.Suggested change
- @State var settingsViewModel: SettingsViewModel + let settingsViewModel: SettingsViewModel
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (69)
.gitignoreApp/AppDelegate.swiftApp/ContentView.swiftApp/EmbeddedWebView.swiftApp/NavigationStore.swiftApp/OnboardingCoordinator.swiftApp/WhatsNewCoordinator.swiftData/Sources/Data/PostRepository+Parsing.swiftData/Sources/Data/PostRepository+Voting.swiftData/Sources/Data/PostRepository.swiftData/Sources/Data/SettingsRepository.swiftData/Sources/Data/UserDefaultsExtensions.swiftData/Sources/Data/WhatsNew/WhatsNewRepository.swiftData/Tests/DataTests/OnboardingRepositoryTests.swiftData/Tests/DataTests/SettingsRepositoryTests.swiftData/Tests/DataTests/WhatsNewRepositoryTests.swiftDesignSystem/Sources/DesignSystem/Components/AppStateViews.swiftDesignSystem/Sources/DesignSystem/Components/AppTextField.swiftDesignSystem/Sources/DesignSystem/Components/MailView.swiftDesignSystem/Sources/DesignSystem/Components/PostDisplayView+Pills.swiftDesignSystem/Sources/DesignSystem/Components/PostDisplayView.swiftDesignSystem/Sources/DesignSystem/Components/ThumbnailView.swiftDesignSystem/Sources/DesignSystem/Components/VoteButton.swiftDomain/Package.swiftDomain/Sources/Domain/Models.swiftDomain/Sources/Domain/OnboardingUseCase.swiftDomain/Sources/Domain/SettingsUseCase.swiftDomain/Sources/Domain/WhatsNewUseCase.swiftDomain/Tests/DomainTests/UseCaseTests.swiftExtensions/OpenInViewController.swiftFeatures/Authentication/Sources/Authentication/LoginView.swiftFeatures/Comments/Sources/Comments/CommentRow.swiftFeatures/Comments/Sources/Comments/CommentsComponents.swiftFeatures/Comments/Sources/Comments/CommentsView.swiftFeatures/Comments/Sources/Comments/CommentsViewModel.swiftFeatures/Comments/Tests/CommentsTests/CommentsViewModelTests.swiftFeatures/Feed/Sources/Feed/FeedView.swiftFeatures/Feed/Sources/Feed/FeedViewModel.swiftFeatures/Feed/Sources/Feed/PostRowView.swiftFeatures/Feed/Tests/FeedTests/FeedViewModelTests.swiftFeatures/Feed/Tests/FeedTests/FeedViewTests.swiftFeatures/Onboarding/Sources/Onboarding/OnboardingService.swiftFeatures/Settings/Package.swiftFeatures/Settings/Sources/Settings/SettingsView.swiftFeatures/Settings/Sources/Settings/SettingsViewModel.swiftFeatures/Settings/Sources/Settings/SupportView.swiftFeatures/Settings/Sources/Settings/SupportViewModel.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewModelTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewTests.swiftFeatures/WhatsNew/Package.resolvedFeatures/WhatsNew/Package.swiftFeatures/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swiftFeatures/WhatsNew/Sources/WhatsNew/Models/WhatsNewItem.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewItemView.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swiftFeatures/WhatsNew/Sources/WhatsNew/WhatsNew.swiftFeatures/WhatsNew/Sources/WhatsNew/WhatsNewService.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swiftHackers.xcodeproj/project.pbxprojNetworking/Sources/Networking/NetworkManager.swiftShared/Sources/Shared/DependencyInjection/DependencyContainer.swiftShared/Sources/Shared/NavigationStoreProtocol.swiftShared/Sources/Shared/Services/LinkOpener.swiftShared/Sources/Shared/Services/ReviewPromptController.swiftShared/Sources/Shared/Session/SessionService.swiftShared/Tests/SharedTests/DependencyContainerTests.swiftShared/Tests/SharedTests/LinkOpenerTests.swiftShared/Tests/SharedTests/VotingViewModelTests.swiftrun_tests.sh
💤 Files with no reviewable changes (4)
- Domain/Sources/Domain/OnboardingUseCase.swift
- Data/Tests/DataTests/OnboardingRepositoryTests.swift
- Features/Onboarding/Sources/Onboarding/OnboardingService.swift
- App/OnboardingCoordinator.swift
🧰 Additional context used
📓 Path-based instructions (5)
**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/*.swift: Use Swift concurrency with async/await in Swift files
Ensure Sendable conformance for thread safety in Swift files
Files:
Features/WhatsNew/Sources/WhatsNew/WhatsNew.swiftFeatures/WhatsNew/Sources/WhatsNew/Models/WhatsNewItem.swiftFeatures/Settings/Package.swiftFeatures/Comments/Tests/CommentsTests/CommentsViewModelTests.swiftFeatures/Authentication/Sources/Authentication/LoginView.swiftFeatures/Feed/Sources/Feed/FeedViewModel.swiftData/Sources/Data/PostRepository.swiftApp/WhatsNewCoordinator.swiftFeatures/Feed/Tests/FeedTests/FeedViewTests.swiftShared/Sources/Shared/NavigationStoreProtocol.swiftShared/Tests/SharedTests/LinkOpenerTests.swiftFeatures/WhatsNew/Package.swiftData/Tests/DataTests/WhatsNewRepositoryTests.swiftNetworking/Sources/Networking/NetworkManager.swiftFeatures/Settings/Sources/Settings/SupportView.swiftFeatures/Feed/Sources/Feed/PostRowView.swiftShared/Tests/SharedTests/VotingViewModelTests.swiftFeatures/Feed/Tests/FeedTests/FeedViewModelTests.swiftFeatures/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swiftData/Sources/Data/SettingsRepository.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewTests.swiftShared/Sources/Shared/Services/LinkOpener.swiftDomain/Sources/Domain/WhatsNewUseCase.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewModelTests.swiftDesignSystem/Sources/DesignSystem/Components/PostDisplayView+Pills.swiftDomain/Package.swiftApp/NavigationStore.swiftData/Sources/Data/UserDefaultsExtensions.swiftDesignSystem/Sources/DesignSystem/Components/ThumbnailView.swiftDesignSystem/Sources/DesignSystem/Components/PostDisplayView.swiftShared/Sources/Shared/Session/SessionService.swiftApp/EmbeddedWebView.swiftFeatures/WhatsNew/Sources/WhatsNew/WhatsNewService.swiftDesignSystem/Sources/DesignSystem/Components/MailView.swiftFeatures/Comments/Sources/Comments/CommentRow.swiftData/Sources/Data/PostRepository+Parsing.swiftData/Tests/DataTests/SettingsRepositoryTests.swiftDesignSystem/Sources/DesignSystem/Components/VoteButton.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewItemView.swiftDomain/Sources/Domain/Models.swiftApp/AppDelegate.swiftFeatures/Settings/Sources/Settings/SettingsViewModel.swiftShared/Tests/SharedTests/DependencyContainerTests.swiftFeatures/Settings/Sources/Settings/SettingsView.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swiftData/Sources/Data/PostRepository+Voting.swiftShared/Sources/Shared/DependencyInjection/DependencyContainer.swiftDomain/Sources/Domain/SettingsUseCase.swiftData/Sources/Data/WhatsNew/WhatsNewRepository.swiftFeatures/Feed/Sources/Feed/FeedView.swiftFeatures/Settings/Sources/Settings/SupportViewModel.swiftExtensions/OpenInViewController.swiftDesignSystem/Sources/DesignSystem/Components/AppTextField.swiftDesignSystem/Sources/DesignSystem/Components/AppStateViews.swiftDomain/Tests/DomainTests/UseCaseTests.swiftFeatures/Comments/Sources/Comments/CommentsComponents.swiftShared/Sources/Shared/Services/ReviewPromptController.swiftFeatures/Comments/Sources/Comments/CommentsView.swiftFeatures/Comments/Sources/Comments/CommentsViewModel.swiftApp/ContentView.swift
{Features,DesignSystem}/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
{Features,DesignSystem}/**/*.swift: Apply @mainactor annotation to UI code in Swift files
Use Combine for reactive bindings with @StateObject for view-owned ViewModels and @EnvironmentObject for navigation/session state in SwiftUI
Files:
Features/WhatsNew/Sources/WhatsNew/WhatsNew.swiftFeatures/WhatsNew/Sources/WhatsNew/Models/WhatsNewItem.swiftFeatures/Settings/Package.swiftFeatures/Comments/Tests/CommentsTests/CommentsViewModelTests.swiftFeatures/Authentication/Sources/Authentication/LoginView.swiftFeatures/Feed/Sources/Feed/FeedViewModel.swiftFeatures/Feed/Tests/FeedTests/FeedViewTests.swiftFeatures/WhatsNew/Package.swiftFeatures/Settings/Sources/Settings/SupportView.swiftFeatures/Feed/Sources/Feed/PostRowView.swiftFeatures/Feed/Tests/FeedTests/FeedViewModelTests.swiftFeatures/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewModelTests.swiftDesignSystem/Sources/DesignSystem/Components/PostDisplayView+Pills.swiftDesignSystem/Sources/DesignSystem/Components/ThumbnailView.swiftDesignSystem/Sources/DesignSystem/Components/PostDisplayView.swiftFeatures/WhatsNew/Sources/WhatsNew/WhatsNewService.swiftDesignSystem/Sources/DesignSystem/Components/MailView.swiftFeatures/Comments/Sources/Comments/CommentRow.swiftDesignSystem/Sources/DesignSystem/Components/VoteButton.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewItemView.swiftFeatures/Settings/Sources/Settings/SettingsViewModel.swiftFeatures/Settings/Sources/Settings/SettingsView.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swiftFeatures/Feed/Sources/Feed/FeedView.swiftFeatures/Settings/Sources/Settings/SupportViewModel.swiftDesignSystem/Sources/DesignSystem/Components/AppTextField.swiftDesignSystem/Sources/DesignSystem/Components/AppStateViews.swiftFeatures/Comments/Sources/Comments/CommentsComponents.swiftFeatures/Comments/Sources/Comments/CommentsView.swiftFeatures/Comments/Sources/Comments/CommentsViewModel.swift
{Features,Shared}/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Inject dependencies into ViewModels via protocols and use DependencyContainer.shared to provide all dependencies
Files:
Features/WhatsNew/Sources/WhatsNew/WhatsNew.swiftFeatures/WhatsNew/Sources/WhatsNew/Models/WhatsNewItem.swiftFeatures/Settings/Package.swiftFeatures/Comments/Tests/CommentsTests/CommentsViewModelTests.swiftFeatures/Authentication/Sources/Authentication/LoginView.swiftFeatures/Feed/Sources/Feed/FeedViewModel.swiftFeatures/Feed/Tests/FeedTests/FeedViewTests.swiftShared/Sources/Shared/NavigationStoreProtocol.swiftShared/Tests/SharedTests/LinkOpenerTests.swiftFeatures/WhatsNew/Package.swiftFeatures/Settings/Sources/Settings/SupportView.swiftFeatures/Feed/Sources/Feed/PostRowView.swiftShared/Tests/SharedTests/VotingViewModelTests.swiftFeatures/Feed/Tests/FeedTests/FeedViewModelTests.swiftFeatures/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewTests.swiftShared/Sources/Shared/Services/LinkOpener.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewModelTests.swiftShared/Sources/Shared/Session/SessionService.swiftFeatures/WhatsNew/Sources/WhatsNew/WhatsNewService.swiftFeatures/Comments/Sources/Comments/CommentRow.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewItemView.swiftFeatures/Settings/Sources/Settings/SettingsViewModel.swiftShared/Tests/SharedTests/DependencyContainerTests.swiftFeatures/Settings/Sources/Settings/SettingsView.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swiftShared/Sources/Shared/DependencyInjection/DependencyContainer.swiftFeatures/Feed/Sources/Feed/FeedView.swiftFeatures/Settings/Sources/Settings/SupportViewModel.swiftFeatures/Comments/Sources/Comments/CommentsComponents.swiftShared/Sources/Shared/Services/ReviewPromptController.swiftFeatures/Comments/Sources/Comments/CommentsView.swiftFeatures/Comments/Sources/Comments/CommentsViewModel.swift
**/Tests/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/Tests/**/*.swift: Use Swift Testing framework with @suite and @test attributes for tests
Mock dependencies with protocols in test files
Files:
Features/Comments/Tests/CommentsTests/CommentsViewModelTests.swiftFeatures/Feed/Tests/FeedTests/FeedViewTests.swiftShared/Tests/SharedTests/LinkOpenerTests.swiftData/Tests/DataTests/WhatsNewRepositoryTests.swiftShared/Tests/SharedTests/VotingViewModelTests.swiftFeatures/Feed/Tests/FeedTests/FeedViewModelTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewModelTests.swiftData/Tests/DataTests/SettingsRepositoryTests.swiftShared/Tests/SharedTests/DependencyContainerTests.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swiftDomain/Tests/DomainTests/UseCaseTests.swift
Features/*/Tests/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Test ViewModels, not Views in the Swift Testing framework
Files:
Features/Comments/Tests/CommentsTests/CommentsViewModelTests.swiftFeatures/Feed/Tests/FeedTests/FeedViewTests.swiftFeatures/Feed/Tests/FeedTests/FeedViewModelTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewModelTests.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift
🧠 Learnings (10)
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to Features/*/Tests/**/*.swift : Test ViewModels, not Views in the Swift Testing framework
Applied to files:
Features/Comments/Tests/CommentsTests/CommentsViewModelTests.swiftFeatures/Authentication/Sources/Authentication/LoginView.swiftFeatures/Feed/Tests/FeedTests/FeedViewTests.swiftShared/Tests/SharedTests/VotingViewModelTests.swiftFeatures/Feed/Tests/FeedTests/FeedViewModelTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewModelTests.swiftShared/Tests/SharedTests/DependencyContainerTests.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swiftDomain/Tests/DomainTests/UseCaseTests.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to **/Tests/**/*.swift : Mock dependencies with protocols in test files
Applied to files:
Features/Comments/Tests/CommentsTests/CommentsViewModelTests.swiftFeatures/Feed/Tests/FeedTests/FeedViewTests.swiftData/Tests/DataTests/WhatsNewRepositoryTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewModelTests.swiftDomain/Package.swiftShared/Tests/SharedTests/DependencyContainerTests.swiftDomain/Tests/DomainTests/UseCaseTests.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to {Features,DesignSystem}/**/*.swift : Use Combine for reactive bindings with StateObject for view-owned ViewModels and EnvironmentObject for navigation/session state in SwiftUI
Applied to files:
Features/Authentication/Sources/Authentication/LoginView.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swiftData/Sources/Data/SettingsRepository.swiftDesignSystem/Sources/DesignSystem/Components/MailView.swiftFeatures/Settings/Sources/Settings/SettingsView.swiftFeatures/Feed/Sources/Feed/FeedView.swiftDesignSystem/Sources/DesignSystem/Components/AppTextField.swiftDesignSystem/Sources/DesignSystem/Components/AppStateViews.swiftFeatures/Comments/Sources/Comments/CommentsComponents.swiftFeatures/Comments/Sources/Comments/CommentsView.swiftApp/ContentView.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: In Features layer, organize each feature as a separate Swift Package with MVVM pattern: ViewModels as ObservableObject with Published properties and SwiftUI views with EnvironmentObject navigation
Applied to files:
Features/Authentication/Sources/Authentication/LoginView.swiftFeatures/Feed/Sources/Feed/FeedViewModel.swiftFeatures/Feed/Sources/Feed/PostRowView.swiftFeatures/Feed/Sources/Feed/FeedView.swiftHackers.xcodeproj/project.pbxprojFeatures/Comments/Sources/Comments/CommentsComponents.swiftFeatures/Comments/Sources/Comments/CommentsView.swiftApp/ContentView.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to **/Tests/**/*.swift : Use Swift Testing framework with Suite and Test attributes for tests
Applied to files:
Data/Tests/DataTests/WhatsNewRepositoryTests.swiftFeatures/Settings/Tests/SettingsTests/SettingsViewTests.swiftShared/Tests/SharedTests/DependencyContainerTests.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swiftDomain/Tests/DomainTests/UseCaseTests.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: In Data layer, implement Domain protocols (e.g., PostRepository implementing PostUseCase) and use protocol-based UserDefaults for testability
Applied to files:
Data/Tests/DataTests/WhatsNewRepositoryTests.swiftData/Sources/Data/SettingsRepository.swiftDomain/Sources/Domain/WhatsNewUseCase.swiftData/Sources/Data/UserDefaultsExtensions.swiftShared/Tests/SharedTests/DependencyContainerTests.swiftData/Sources/Data/WhatsNew/WhatsNewRepository.swiftDomain/Tests/DomainTests/UseCaseTests.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: In Domain layer, define Models (Post, Comment, User, TextSize), Use Cases (PostUseCase, CommentUseCase, SettingsUseCase, VoteUseCase), and VotingStateProvider protocol and implementation
Applied to files:
Features/Feed/Sources/Feed/PostRowView.swiftFeatures/Comments/Sources/Comments/CommentRow.swiftDomain/Sources/Domain/Models.swiftShared/Sources/Shared/DependencyInjection/DependencyContainer.swiftFeatures/Feed/Sources/Feed/FeedView.swiftFeatures/Comments/Sources/Comments/CommentsComponents.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Target iOS 26+ and use Swift 6.2
Applied to files:
Features/Settings/Tests/SettingsTests/SettingsViewTests.swiftShared/Tests/SharedTests/DependencyContainerTests.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to {Features,DesignSystem}/**/*.swift : Apply MainActor annotation to UI code in Swift files
Applied to files:
Features/WhatsNew/Sources/WhatsNew/WhatsNewService.swiftDesignSystem/Sources/DesignSystem/Components/AppStateViews.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to {Features,Shared}/**/*.swift : Inject dependencies into ViewModels via protocols and use DependencyContainer.shared to provide all dependencies
Applied to files:
Shared/Tests/SharedTests/DependencyContainerTests.swiftShared/Sources/Shared/DependencyInjection/DependencyContainer.swiftApp/ContentView.swift
🧬 Code graph analysis (24)
Features/Feed/Sources/Feed/FeedViewModel.swift (2)
Shared/Sources/Shared/LoadingStateManager.swift (1)
setLoadFunction(38-44)Shared/Sources/Shared/Services/BookmarksController.swift (1)
isBookmarked(47-50)
App/WhatsNewCoordinator.swift (3)
Data/Sources/Data/WhatsNew/WhatsNewRepository.swift (2)
shouldShowWhatsNew(55-68)markWhatsNewShown(70-72)Shared/Tests/SharedTests/DependencyContainerTests.swift (2)
shouldShowWhatsNew(167-169)markWhatsNewShown(171-173)Features/WhatsNew/Sources/WhatsNew/WhatsNewService.swift (1)
createWhatsNewView(12-17)
Features/Feed/Tests/FeedTests/FeedViewTests.swift (1)
App/NavigationStore.swift (1)
showPostLink(62-78)
Shared/Sources/Shared/NavigationStoreProtocol.swift (4)
Features/Settings/Tests/SettingsTests/SettingsViewTests.swift (1)
showPostLink(32-35)App/NavigationStore.swift (1)
showPostLink(62-78)Features/Feed/Tests/FeedTests/FeedViewTests.swift (1)
showPostLink(33-35)Shared/Tests/SharedTests/VotingViewModelTests.swift (2)
showPostLink(85-88)showPostLink(316-319)
Data/Tests/DataTests/WhatsNewRepositoryTests.swift (3)
Data/Sources/Data/WhatsNew/WhatsNewRepository.swift (4)
lastShownVersion(34-36)shouldShowWhatsNew(55-68)markWhatsNewShown(70-72)save(38-40)App/WhatsNewCoordinator.swift (2)
shouldShowWhatsNew(23-25)markWhatsNewShown(27-29)Shared/Tests/SharedTests/DependencyContainerTests.swift (2)
shouldShowWhatsNew(167-169)markWhatsNewShown(171-173)
Features/Settings/Sources/Settings/SupportView.swift (2)
Features/Settings/Sources/Settings/SupportViewModel.swift (1)
purchase(76-94)Data/Sources/Data/SupportPurchaseRepository.swift (2)
purchase(50-72)product(121-134)
Shared/Tests/SharedTests/VotingViewModelTests.swift (3)
Features/Settings/Tests/SettingsTests/SettingsViewTests.swift (1)
showPostLink(32-35)App/NavigationStore.swift (1)
showPostLink(62-78)Features/Feed/Tests/FeedTests/FeedViewTests.swift (1)
showPostLink(33-35)
Features/Settings/Tests/SettingsTests/SettingsViewTests.swift (3)
App/NavigationStore.swift (1)
showPostLink(62-78)Features/Feed/Tests/FeedTests/FeedViewTests.swift (1)
showPostLink(33-35)Shared/Tests/SharedTests/VotingViewModelTests.swift (2)
showPostLink(85-88)showPostLink(316-319)
Shared/Sources/Shared/Services/LinkOpener.swift (1)
Extensions/OpenInViewController.swift (1)
openURL(49-63)
Domain/Sources/Domain/WhatsNewUseCase.swift (3)
App/WhatsNewCoordinator.swift (2)
shouldShowWhatsNew(23-25)markWhatsNewShown(27-29)Data/Sources/Data/WhatsNew/WhatsNewRepository.swift (2)
shouldShowWhatsNew(55-68)markWhatsNewShown(70-72)Shared/Tests/SharedTests/DependencyContainerTests.swift (2)
shouldShowWhatsNew(167-169)markWhatsNewShown(171-173)
DesignSystem/Sources/DesignSystem/Components/PostDisplayView+Pills.swift (4)
DesignSystem/Sources/DesignSystem/Theme/TextScaling.swift (1)
scaledFont(89-91)DesignSystem/Sources/DesignSystem/Theme/AppColors.swift (3)
background(173-175)pillForeground(55-68)pillBackground(40-53)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (2)
canVote(183-185)canUnvote(187-189)Shared/Sources/Shared/Services/BookmarksController.swift (1)
isBookmarked(47-50)
App/NavigationStore.swift (3)
Shared/Sources/Shared/NavigationStoreProtocol.swift (1)
openURLInPrimaryContext(28-30)Shared/Sources/Shared/Services/LinkOpener.swift (1)
openURL(35-58)Extensions/OpenInViewController.swift (1)
openURL(49-63)
Data/Sources/Data/UserDefaultsExtensions.swift (3)
Data/Tests/DataTests/SettingsRepositoryTests.swift (4)
integer(38-41)set(48-51)set(53-56)set(58-61)Domain/Tests/DomainTests/UseCaseTests.swift (4)
integer(282-284)set(290-292)set(294-296)set(298-300)Data/Sources/Data/SettingsRepository.swift (1)
registerDefaults(31-40)
DesignSystem/Sources/DesignSystem/Components/PostDisplayView.swift (1)
Features/Feed/Sources/Feed/FeedView.swift (1)
isHackerNewsItemURL(390-393)
Features/WhatsNew/Sources/WhatsNew/WhatsNewService.swift (1)
Features/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swift (1)
currentWhatsNew(19-38)
Features/Comments/Sources/Comments/CommentRow.swift (8)
Data/Sources/Data/PostRepository+Parsing.swift (1)
post(54-108)Domain/Sources/Domain/CommentHTMLParser.swift (1)
parseHTMLText(95-102)DesignSystem/Sources/DesignSystem/Theme/TextScaling.swift (1)
scaledFont(89-91)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (2)
canVote(183-185)canUnvote(187-189)Data/Sources/Data/PostRepository+Voting.swift (5)
voteLinks(97-141)upvote(21-38)upvote(58-75)unvote(40-56)unvote(77-93)DesignSystem/Sources/DesignSystem/Components/VotingContextMenuItems.swift (1)
commentVotingMenuItems(40-62)Shared/Sources/Shared/Services/ContentSharePresenter.swift (1)
shareComment(33-38)DesignSystem/Sources/DesignSystem/Components/VoteButton.swift (1)
foregroundColor(117-125)
Data/Tests/DataTests/SettingsRepositoryTests.swift (1)
Domain/Tests/DomainTests/UseCaseTests.swift (3)
object(274-276)integer(282-284)bool(278-280)
DesignSystem/Sources/DesignSystem/Components/VoteButton.swift (2)
Shared/Sources/Shared/ViewModels/VotingViewModel.swift (3)
votingState(171-181)canUnvote(187-189)canVote(183-185)Domain/Sources/Domain/VotingStateProvider.swift (1)
votingState(27-36)
Shared/Tests/SharedTests/DependencyContainerTests.swift (3)
Shared/Sources/Shared/DependencyInjection/DependencyContainer.swift (1)
getWhatsNewUseCase(108-110)App/WhatsNewCoordinator.swift (2)
shouldShowWhatsNew(23-25)markWhatsNewShown(27-29)Data/Sources/Data/WhatsNew/WhatsNewRepository.swift (2)
shouldShowWhatsNew(55-68)markWhatsNewShown(70-72)
Features/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift (1)
Features/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swift (1)
currentWhatsNew(19-38)
Data/Sources/Data/WhatsNew/WhatsNewRepository.swift (3)
Data/Tests/DataTests/WhatsNewRepositoryTests.swift (1)
lastShownVersion(107-109)App/WhatsNewCoordinator.swift (2)
shouldShowWhatsNew(23-25)markWhatsNewShown(27-29)Shared/Tests/SharedTests/DependencyContainerTests.swift (2)
shouldShowWhatsNew(167-169)markWhatsNewShown(171-173)
Features/Comments/Sources/Comments/CommentsView.swift (5)
Shared/Sources/Shared/Services/LinkOpener.swift (1)
openURL(35-58)Features/Feed/Sources/Feed/FeedView.swift (1)
handleLinkTap(367-388)App/NavigationStore.swift (2)
openURLInPrimaryContext(112-130)showPostLink(62-78)Shared/Sources/Shared/NavigationStoreProtocol.swift (1)
openURLInPrimaryContext(28-30)Shared/Sources/Shared/DependencyInjection/DependencyContainer.swift (1)
getSettingsUseCase(71-73)
Features/Comments/Sources/Comments/CommentsViewModel.swift (1)
Data/Sources/Data/PostRepository+Parsing.swift (2)
comments(110-113)comments(115-125)
App/ContentView.swift (1)
App/WhatsNewCoordinator.swift (2)
makeWhatsNewView(31-36)shouldShowWhatsNew(23-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: test
| ZStack(alignment: .top) { | ||
| NavigationView { | ||
| CommentsView<NavigationStore>( | ||
| postID: viewModel.postID, | ||
| initialPost: viewModel.post, | ||
| showsPostHeader: isExpanded, | ||
| allowsRefresh: false, | ||
| isAtTop: $isScrollAtTop, | ||
| onPostLinkTap: collapseSheet, | ||
| viewModel: viewModel, | ||
| votingViewModel: votingViewModel | ||
| ) | ||
| .scrollDisabled(!isExpanded) | ||
| .toolbar { | ||
| ToolbarItem(placement: .navigationBarLeading) { | ||
| Button { | ||
| Task { @MainActor in onDismiss() } | ||
| } label: { | ||
| Label("Back", systemImage: "chevron.left") | ||
| } | ||
| .accessibilityLabel("Back") | ||
| } | ||
| } | ||
| .toolbar(showsExpandedToolbar ? .visible : .hidden, for: .navigationBar) | ||
| } | ||
| .navigationViewStyle(.stack) | ||
| .padding(.top, isExpanded ? Self.navigationBarTopSpacing : 0) | ||
| .opacity(isExpanded ? 1 : 0) | ||
| .allowsHitTesting(isExpanded) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
NavigationView deprecated iOS 16 NavigationStack replacement
💡 Result:
Short answer: NavigationView was deprecated with SwiftUI 4 / iOS 16 — replace it with NavigationStack for single‑column navigation (and NavigationSplitView for multi‑column UIs). Use NavigationLink(value:) / navigationDestination(for:_:), or NavigationPath for programmatic/value‑based navigation. See Apple’s migration guide and examples. [1][2]
Sources
- Apple: “Migrating to new navigation types” (SwiftUI). [1]
- Overview / examples (NavigationStack / NavigationSplitView). [2]
🏁 Script executed:
# Check the app's deployment target
find . -type f \( -name "*.swift" -o -name "*.pbxproj" -o -name "Info.plist" -o -name "*.json" \) | head -20Repository: weiran/Hackers
Length of output: 870
🏁 Script executed:
# Check Package.swift for platform requirements
cat -n ./Data/Package.swift | head -50Repository: weiran/Hackers
Length of output: 1129
🏁 Script executed:
# Check the main app structure for deployment targets
cat -n ./App/HackersApp.swift | head -30Repository: weiran/Hackers
Length of output: 1160
🏁 Script executed:
# Search for iOS deployment target configuration
rg -i "ios.*1[6-9]|platform.*ios|deployment.*target" --type swiftRepository: weiran/Hackers
Length of output: 323
🏁 Script executed:
# Check CommentsView for Sendable conformance and async/await usage
rg -A 10 "struct CommentsView" --type swift | head -30Repository: weiran/Hackers
Length of output: 2263
🏁 Script executed:
# Check NavigationStore for Sendable conformance
cat -n ./App/NavigationStore.swiftRepository: weiran/Hackers
Length of output: 6519
🏁 Script executed:
# Search for any additional issues in EmbeddedWebView around the flagged lines
sed -n '420,460p' ./App/EmbeddedWebView.swiftRepository: weiran/Hackers
Length of output: 1794
Replace deprecated NavigationView with NavigationStack.
NavigationView with .navigationViewStyle(.stack) is deprecated in favour of NavigationStack. With the app targeting iOS 26+, NavigationStack should be used instead.
🤖 Prompt for AI Agents
In @App/EmbeddedWebView.swift around lines 428 - 456, Replace the deprecated
NavigationView usage with NavigationStack: swap the NavigationView { ... }
wrapper around CommentsView to NavigationStack { ... }, remove the
.navigationViewStyle(.stack) modifier, and keep the existing modifiers
(.scrollDisabled, .toolbar, .toolbar(showsExpandedToolbar ? .visible : .hidden,
for: .navigationBar), .padding(.top, ...), .opacity(...),
.allowsHitTesting(...)) applied to the NavigationStack so toolbar items
(ToolbarItem with placement: .navigationBarLeading and the Back button) and
visibility behavior remain unchanged; verify the Back button action (onDismiss)
and bindings (isScrollAtTop, isExpanded) are passed through unchanged.
| func linkWithIDPrefix(_ prefix: String, in links: [Element]) throws -> Element? { | ||
| for link in links { | ||
| if try link.attr("id").starts(with: prefix) { | ||
| return link | ||
| } | ||
| for link in links where try link.attr("id").starts(with: prefix) { | ||
| return link | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Throwing expression in for-where clause will not compile.
Swift's for-where clause does not support throwing expressions. The try link.attr("id") in the where clause is invalid syntax and will result in a compilation error.
🐛 Proposed fix: move the try expression inside the loop body
func linkWithIDPrefix(_ prefix: String, in links: [Element]) throws -> Element? {
- for link in links where try link.attr("id").starts(with: prefix) {
- return link
+ for link in links {
+ if try link.attr("id").starts(with: prefix) {
+ return link
+ }
}
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func linkWithIDPrefix(_ prefix: String, in links: [Element]) throws -> Element? { | |
| for link in links { | |
| if try link.attr("id").starts(with: prefix) { | |
| return link | |
| } | |
| for link in links where try link.attr("id").starts(with: prefix) { | |
| return link | |
| } | |
| return nil | |
| } | |
| func linkWithIDPrefix(_ prefix: String, in links: [Element]) throws -> Element? { | |
| for link in links { | |
| if try link.attr("id").starts(with: prefix) { | |
| return link | |
| } | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In @Data/Sources/Data/PostRepository+Voting.swift around lines 105 - 110, The
for-where clause uses a throwing expression (try link.attr("id")) which won't
compile; change linkWithIDPrefix(_:, in:) to perform the try inside the loop
body instead: iterate with "for link in links { ... }", call let id = try
link.attr("id") (or use a do-catch if you want to ignore individual errors),
then check if id.starts(with: prefix) and return link if so, otherwise continue;
keep the function throwing so callers still receive propagated errors.
| func linkWithExactText(_ text: String, in links: [Element]) throws -> Element? { | ||
| for link in links { | ||
| if try link.text().localizedCaseInsensitiveCompare(text) == .orderedSame { | ||
| return link | ||
| } | ||
| for link in links where try link.text().localizedCaseInsensitiveCompare(text) == .orderedSame { | ||
| return link | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Same issue: throwing expression in for-where clause.
The try link.text() call in the where clause has the same problem as above—it will not compile.
🐛 Proposed fix
func linkWithExactText(_ text: String, in links: [Element]) throws -> Element? {
- for link in links where try link.text().localizedCaseInsensitiveCompare(text) == .orderedSame {
- return link
+ for link in links {
+ if try link.text().localizedCaseInsensitiveCompare(text) == .orderedSame {
+ return link
+ }
}
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func linkWithExactText(_ text: String, in links: [Element]) throws -> Element? { | |
| for link in links { | |
| if try link.text().localizedCaseInsensitiveCompare(text) == .orderedSame { | |
| return link | |
| } | |
| for link in links where try link.text().localizedCaseInsensitiveCompare(text) == .orderedSame { | |
| return link | |
| } | |
| return nil | |
| } | |
| func linkWithExactText(_ text: String, in links: [Element]) throws -> Element? { | |
| for link in links { | |
| if try link.text().localizedCaseInsensitiveCompare(text) == .orderedSame { | |
| return link | |
| } | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In @Data/Sources/Data/PostRepository+Voting.swift around lines 112 - 117, The
throwing expression in the for-where clause of linkWithExactText(_:, in:) causes
a compile error; move the try out of the where and perform the throwing call
inside the loop body: iterate over links, call let linkText = try link.text()
(inside the loop), then compare linkText.localizedCaseInsensitiveCompare(text)
== .orderedSame and return link when it matches; keep the function's throws
signature so the thrown error propagates.
| var linkBrowserMode: LinkBrowserMode { | ||
| LinkBrowserMode(rawValue: integer(forKey: UserDefaultsKeys.linkBrowserMode.rawValue)) ?? .inAppBrowser | ||
| } |
There was a problem hiding this comment.
Default value inconsistency between getter fallback and registered default.
The computed property linkBrowserMode falls back to .inAppBrowser (line 22) when the stored integer doesn't map to a valid enum case, but registerDefaults sets the default to .customBrowser (line 32).
If registerDefaults() is reliably called before any access, this is fine. However, if the getter is accessed before registration, or if an invalid integer is stored, the fallback would unexpectedly return .inAppBrowser instead of .customBrowser.
Consider aligning the fallback:
🐛 Proposed fix
var linkBrowserMode: LinkBrowserMode {
- LinkBrowserMode(rawValue: integer(forKey: UserDefaultsKeys.linkBrowserMode.rawValue)) ?? .inAppBrowser
+ LinkBrowserMode(rawValue: integer(forKey: UserDefaultsKeys.linkBrowserMode.rawValue)) ?? .customBrowser
}Also applies to: 32-32
| private var baseCommentText: AttributedString { | ||
| if let cached = comment.parsedText { | ||
| return cached | ||
| } | ||
|
|
||
| let parsed = CommentHTMLParser.parseHTMLText(comment.text) | ||
| comment.parsedText = parsed | ||
| return parsed | ||
| } |
There was a problem hiding this comment.
Mutating state in computed property may cause SwiftUI issues.
The baseCommentText computed property mutates comment.parsedText during body evaluation. This side effect during render can cause unexpected behaviour with SwiftUI's view diffing. Consider moving the parsing to .task or .onAppear instead.
🔧 Alternative approach
- private var baseCommentText: AttributedString {
- if let cached = comment.parsedText {
- return cached
- }
-
- let parsed = CommentHTMLParser.parseHTMLText(comment.text)
- comment.parsedText = parsed
- return parsed
- }
+ private var baseCommentText: AttributedString {
+ comment.parsedText ?? CommentHTMLParser.parseHTMLText(comment.text)
+ }Then cache in .task:
.task(id: comment.id) {
if comment.parsedText == nil {
comment.parsedText = CommentHTMLParser.parseHTMLText(comment.text)
}
}🤖 Prompt for AI Agents
In @Features/Comments/Sources/Comments/CommentRow.swift around lines 23 - 31,
The computed property baseCommentText mutates comment.parsedText during view
evaluation, which can break SwiftUI; remove mutation from baseCommentText and
make it purely a getter returning comment.parsedText ??
CommentHTMLParser.parseHTMLText(comment.text) without assigning, then perform
the caching assignment elsewhere (e.g., in the view’s .task(id: comment.id) or
.onAppear) by checking if comment.parsedText == nil and setting
comment.parsedText = CommentHTMLParser.parseHTMLText(comment.text); ensure
references to baseCommentText, comment.parsedText, and
CommentHTMLParser.parseHTMLText are updated accordingly.
| func showPostLink(_ post: Post) { | ||
| selectedPostId = post.id | ||
| } |
There was a problem hiding this comment.
Inconsistent mock implementation compared to other test files.
This implementation only sets selectedPostId, but other mock implementations (in SettingsViewTests.swift and VotingViewModelTests.swift) also set selectedPost. This inconsistency could cause issues if tests are added that rely on selectedPost being populated.
🔧 Suggested fix for consistency
func showPostLink(_ post: Post) {
+ selectedPost = post
selectedPostId = post.id
}🤖 Prompt for AI Agents
In @Features/Feed/Tests/FeedTests/FeedViewTests.swift around lines 33 - 35, The
mock function showPostLink(_ post: Post) only assigns selectedPostId; update it
to also assign selectedPost = post so it matches the other test mocks (e.g., in
SettingsViewTests.swift and VotingViewModelTests.swift) and ensures tests that
rely on selectedPost being populated will behave consistently; locate the mock
implementation of showPostLink and add the selectedPost assignment alongside
selectedPostId.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @App/EmbeddedWebView.swift:
- Around line 74-86: The init currently wraps an externally-provided
BrowserController in StateObject which can confuse ownership; change the
property handling so that when an external controller is passed you use
@ObservedObject for that instance and only use @StateObject when you create/own
the controller internally: detect nil controller in init and assign a new
BrowserController to the StateObject ownership path, otherwise keep the passed
instance as an observed object; update references to the controller property
accordingly and document the ownership semantics in the init parameter comment
(mention BrowserController, StateObject, ObservedObject, and init).
- Around line 248-282: InteractivePopGestureEnabler currently unconditionally
replaces navigationController.interactivePopGestureRecognizer?.delegate with its
Coordinator, which can permanently override any existing delegate; modify the
implementation to capture and hold the original delegate (e.g., add
originalDelegate: UIGestureRecognizerDelegate? on Coordinator or a stored
property), only replace the delegate if it differs in updateUIViewController,
and restore the saved original delegate in dismantleUIViewController (or
coordinator deinit/cleanup) when the representable is removed so existing
navigation controller gesture behavior is preserved.
- Around line 295-296: CommentsViewModel and VotingViewModel are
ObservableObject types but are declared with @State, so their @Published changes
won't update the view; change the property wrappers to @StateObject for both
viewModel and votingViewModel and update the view's initializer to accept and
assign those models using the _viewModel and _votingViewModel StateObject
initializers (or initialize them inside the view with StateObject(wrappedValue:
...)) so the objects are owned and observed correctly by the view.
- Around line 556-576: The safeInsetPaddingLeft and safeInsetPaddingRight access
PresentationContextProvider.shared.windowScene?.windows.first directly which is
unreliable; update them to first use the GeometryProxy values (via
resolvedSafeAreaInsets(for:) or proxy.safeAreaInsets) and only fall back to
PresentationContextProvider.shared.windowScene?.windows.first?.safeAreaInsets if
the proxy reports zero insets, mirroring the logic in resolvedSafeAreaInsets and
resolvedScreenSize; locate and modify the safeInsetPaddingLeft and
safeInsetPaddingRight properties to call resolvedSafeAreaInsets(for: proxy) or
accept a GeometryProxy parameter so they prefer proxy-derived insets and only
use the windowScene fallback as a last resort.
🧹 Nitpick comments (8)
Domain/Sources/Domain/SettingsUseCase.swift (1)
50-54: Consider adding adisplayNameproperty for UI consistency.The
TextSizeenum (lines 10-36) includes adisplayNamecomputed property for presenting human-readable text in the UI. For consistency,LinkBrowserModemay benefit from a similar property if it needs to be displayed in settings.💡 Optional enhancement
public enum LinkBrowserMode: Int, CaseIterable, Sendable { case inAppBrowser = 0 case customBrowser = 1 case systemBrowser = 2 + + public var displayName: String { + switch self { + case .inAppBrowser: "In-App Browser" + case .customBrowser: "Embedded Browser" + case .systemBrowser: "System Browser" + } + } }Features/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swift (1)
19-37: Consider externalising the version number.The version string "5.3" is hard-coded in the title. If this value is used elsewhere (e.g., in build configuration), consider deriving it to avoid manual updates on each release.
Features/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift (1)
11-11: Add@Suiteattribute per Swift Testing framework conventions.Based on learnings, tests should use the Swift Testing framework with
@Suiteand@Testattributes. The struct is missing the@Suiteannotation.♻️ Proposed fix
-struct WhatsNewDataTests { +@Suite("WhatsNewData Tests") +struct WhatsNewDataTests {Features/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swift (2)
13-13: Add@MainActorannotation to the View.Per coding guidelines, UI code in Features should have
@MainActorannotation applied for thread safety.♻️ Proposed fix
+@MainActor public struct WhatsNewView: View {
103-108: Consider the user flow when enabling embedded browser.The function enables the embedded browser, updates local state, then immediately dismisses. This is a valid flow, but the user might not notice the setting was applied before dismissal. Consider if brief visual feedback (e.g., a checkmark animation) before dismissal would improve UX.
Features/Feed/Sources/Feed/FeedView.swift (1)
404-407: Consider extractingshouldShowVoteActionsto a shared utility.This helper duplicates logic from
Shared/Comments/CommentsComponents.swift(lines 98-101). While the duplication is minor, extracting it to a shared location (e.g., an extension onPostor a utility inShared) would improve maintainability.App/EmbeddedWebView.swift (2)
200-209: Silent error suppression in navigation monitoring.The
catchblock silently ignores all errors from the navigation stream. Consider logging errors for debugging purposes, even if the UI doesn't need to react to them.♻️ Proposed enhancement
} catch { - // Ignore navigation stream errors; state updates happen on successful events. + // Navigation stream ended or failed; typically benign when view is dismissed. + #if DEBUG + print("[EmbeddedWebView] Navigation stream error: \(error)") + #endif }
388-399: Prefer Task.sleep over DispatchQueue.main.asyncAfter for Swift concurrency consistency.Using
DispatchQueue.main.asyncAfterworks but doesn't integrate with Swift's structured concurrency. Consider usingTaskwithtry await Task.sleepfor consistency with the rest of the codebase.♻️ Proposed refactor
.onChange(of: isExpanded) { _, newValue in if newValue { showsExpandedToolbar = false - DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { - guard isExpanded else { return } - withAnimation(.easeInOut(duration: 0.2)) { - showsExpandedToolbar = true - } - } + Task { @MainActor in + try? await Task.sleep(for: .milliseconds(150)) + guard isExpanded else { return } + withAnimation(.easeInOut(duration: 0.2)) { + showsExpandedToolbar = true + } + } } else { showsExpandedToolbar = false } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
App/EmbeddedWebView.swiftDomain/Sources/Domain/SettingsUseCase.swiftFeatures/Feed/Sources/Feed/FeedView.swiftFeatures/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift
🧰 Additional context used
📓 Path-based instructions (5)
**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/*.swift: Use Swift concurrency with async/await in Swift files
Ensure Sendable conformance for thread safety in Swift files
Files:
Domain/Sources/Domain/SettingsUseCase.swiftFeatures/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swiftApp/EmbeddedWebView.swiftFeatures/Feed/Sources/Feed/FeedView.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift
{Features,DesignSystem}/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
{Features,DesignSystem}/**/*.swift: Apply @mainactor annotation to UI code in Swift files
Use Combine for reactive bindings with @StateObject for view-owned ViewModels and @EnvironmentObject for navigation/session state in SwiftUI
Files:
Features/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swiftFeatures/Feed/Sources/Feed/FeedView.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift
{Features,Shared}/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Inject dependencies into ViewModels via protocols and use DependencyContainer.shared to provide all dependencies
Files:
Features/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swiftFeatures/Feed/Sources/Feed/FeedView.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swiftFeatures/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift
**/Tests/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/Tests/**/*.swift: Use Swift Testing framework with @suite and @test attributes for tests
Mock dependencies with protocols in test files
Files:
Features/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift
Features/*/Tests/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Test ViewModels, not Views in the Swift Testing framework
Files:
Features/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift
🧠 Learnings (6)
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to {Features,DesignSystem}/**/*.swift : Use Combine for reactive bindings with StateObject for view-owned ViewModels and EnvironmentObject for navigation/session state in SwiftUI
Applied to files:
App/EmbeddedWebView.swiftFeatures/Feed/Sources/Feed/FeedView.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: In Features layer, organize each feature as a separate Swift Package with MVVM pattern: ViewModels as ObservableObject with Published properties and SwiftUI views with EnvironmentObject navigation
Applied to files:
Features/Feed/Sources/Feed/FeedView.swiftFeatures/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: In Domain layer, define Models (Post, Comment, User, TextSize), Use Cases (PostUseCase, CommentUseCase, SettingsUseCase, VoteUseCase), and VotingStateProvider protocol and implementation
Applied to files:
Features/Feed/Sources/Feed/FeedView.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to **/Tests/**/*.swift : Use Swift Testing framework with Suite and Test attributes for tests
Applied to files:
Features/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to Features/*/Tests/**/*.swift : Test ViewModels, not Views in the Swift Testing framework
Applied to files:
Features/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to **/Tests/**/*.swift : Mock dependencies with protocols in test files
Applied to files:
Features/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift
🧬 Code graph analysis (4)
App/EmbeddedWebView.swift (3)
Shared/Sources/Shared/Services/ContentSharePresenter.swift (1)
shareURL(23-31)Shared/Sources/Shared/Services/LinkOpener.swift (1)
openURL(35-58)Shared/Sources/Shared/ViewModels/VotingViewModel.swift (2)
canVote(183-185)canUnvote(187-189)
Features/Feed/Sources/Feed/FeedView.swift (3)
App/NavigationStore.swift (3)
showPostLink(62-78)showPost(50-60)showPost(80-89)DesignSystem/Sources/DesignSystem/Components/PostDisplayView.swift (1)
isHackerNewsItemURL(248-251)Shared/Sources/Shared/Services/LinkOpener.swift (1)
openURL(35-58)
Features/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swift (2)
Shared/Sources/Shared/DependencyInjection/DependencyContainer.swift (1)
getSettingsUseCase(71-73)DesignSystem/Sources/DesignSystem/Theme/TextScaling.swift (1)
scaledFont(89-91)
Features/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift (1)
Features/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swift (1)
currentWhatsNew(19-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (15)
Domain/Sources/Domain/SettingsUseCase.swift (1)
38-48: LGTM! Protocol changes are well-structured.The addition of
AnyObjectconformance alongsideSendableproperly enables reference semantics for the mutable settings state whilst maintaining thread safety guarantees. ThelinkBrowserModeproperty correctly replaces the boolean with the new enum type.Features/WhatsNew/Sources/WhatsNew/Models/WhatsNewData.swift (1)
10-17: LGTM! Clean data model with proper Sendable conformance.The struct is appropriately simple and immutable with public access modifiers correctly applied. The
Sendableconformance ensures thread-safe usage across the codebase.Features/WhatsNew/Tests/WhatsNewTests/WhatsNewDataTests.swift (2)
12-23: LGTM! Test correctly validates the factory output.The test appropriately verifies the structure and content of
currentWhatsNew()using#expectassertions. Testing data models directly is acceptable here sinceWhatsNewDatais a pure data structure without ViewModel logic.
25-37: LGTM! WhatsNewItem initialisation test is well-structured.The test validates all properties including the auto-generated UUID, which confirms
Identifiableconformance works correctly.Features/WhatsNew/Sources/WhatsNew/Views/WhatsNewView.swift (2)
19-28: Dependency injection pattern looks good.The initialiser correctly uses
DependencyContainer.shared.getSettingsUseCase()as the default whilst allowing injection for testing. This aligns with the coding guidelines for dependency injection via protocols.
115-146: LGTM! Action button helper is well-structured.The helper function cleanly encapsulates the button styling logic with appropriate use of
@ViewBuilderand maintains consistency between primary and secondary styles.Features/Feed/Sources/Feed/FeedView.swift (5)
11-16: LGTM: Environment-based navigation store integrationThe use of
@Environment(Store.self)for the navigation store and theisSidebarproperty aligns well with SwiftUI patterns and the coding guidelines for using EnvironmentObject for navigation/session state.
70-76: LGTM: Reactive post update handlingThe onChange handler correctly observes
navigationStore.selectedPostchanges and updates the feed viareplacePost. The guard-let pattern safely handles the optional.
97-128: LGTM: Well-organised helper properties in private extensionThe extraction of
primaryPostTypes,secondaryPostTypes,shouldShowBookmarksEmptyState, andselectionBindinginto a private extension improves readability and separation of concerns.
367-374: LGTM: Browser mode routing logicThe
handlePostTapmethod correctly routes toshowPostLinkfor custom browser mode and falls back toshowPostotherwise. Dependency access viaDependencyContainer.shared.getSettingsUseCase()follows the coding guidelines.
382-396: Confirm iPad handling asymmetry is intentional or reconcile the routing logic.The asymmetry is confirmed:
handlePostTapcallsshowPostLinkwithout iPad exclusion, whereashandleLinkTapexplicitly guards against iPad withUIDevice.current.userInterfaceIdiom != .pad. However,showPostLinkitself has built-in iPad handling that routes toopenURLInPrimaryContextorLinkOpener. The explicit guard inhandleLinkTapbypassesshowPostLinkentirely for iPad, forcing iPad links throughopenURLInPrimaryContextinstead. If both methods should behave consistently,handlePostTapmay need the same guard. If the different routing is intentional, add clarifying comments explaining why link taps and post taps take different iPad paths.App/EmbeddedWebView.swift (4)
17-65: LGTM: Well-structured BrowserController with MainActor annotationThe
BrowserControllerclass correctly uses@MainActorfor thread safety and@Publishedproperties for reactive state updates. The navigation methods (goBack,goForward,reload,stopLoading) appropriately delegate to the underlyingWebPageand callupdateState().
638-772: LGTM: BrowserControlsView with glass-styled controlsThe
BrowserControlsViewprovides a clean, accessible control layout with proper labels. The separation of navigation controls, share controls, and the close button is well-organised. ThecontrolLabel(for:)helper ensures accessibility labels are consistent.
795-879: LGTM: CollapsedPostHeaderView with proper voting state handlingThe collapsed header correctly derives voting state from either
votingStateor falls back to the post's properties. The use ofPostPillViewfor consistent styling and proper accessibility labels is good.
774-786: No changes required —.glassEffectAPI is appropriate for the app's deployment target.The
.glassEffectmodifier requires iOS 26.0+, and the app is configured with a minimum deployment target of iOS 26.0. The code does not require an@availablecheck or fallback implementation.Likely an incorrect or invalid review comment.
| init( | ||
| url: URL, | ||
| onDismiss: @MainActor @escaping () -> Void, | ||
| showsCloseButton: Bool, | ||
| showsToolbar: Bool = true, | ||
| controller: BrowserController? = nil | ||
| ) { | ||
| self.url = url | ||
| self.onDismiss = onDismiss | ||
| self.showsCloseButton = showsCloseButton | ||
| self.showsToolbar = showsToolbar | ||
| _controller = StateObject(wrappedValue: controller ?? BrowserController()) | ||
| } |
There was a problem hiding this comment.
Potential issue with StateObject initialisation for externally-provided controller.
When a BrowserController is passed in from outside, wrapping it in StateObject may cause unexpected behaviour. StateObject is designed for view-owned objects and ignores subsequent initialisations, but the external caller may expect to observe the same instance. Consider using @ObservedObject when a controller is provided externally, or document that the caller should not retain a reference.
🔧 Alternative approach using conditional property wrapper
If the controller is always owned by a parent view when provided, consider either:
- Always creating the controller internally and exposing state via callbacks
- Using a factory pattern that clearly indicates ownership
// Option: Document ownership clearly
/// - Parameter controller: If provided, the view observes but does not own this controller.
/// Pass `nil` to have the view create and own its own controller.🤖 Prompt for AI Agents
In @App/EmbeddedWebView.swift around lines 74 - 86, The init currently wraps an
externally-provided BrowserController in StateObject which can confuse
ownership; change the property handling so that when an external controller is
passed you use @ObservedObject for that instance and only use @StateObject when
you create/own the controller internally: detect nil controller in init and
assign a new BrowserController to the StateObject ownership path, otherwise keep
the passed instance as an observed object; update references to the controller
property accordingly and document the ownership semantics in the init parameter
comment (mention BrowserController, StateObject, ObservedObject, and init).
| private struct InteractivePopGestureEnabler: UIViewControllerRepresentable { | ||
| func makeUIViewController(context: Context) -> UIViewController { | ||
| let controller = UIViewController() | ||
| controller.view.backgroundColor = .clear | ||
| return controller | ||
| } | ||
|
|
||
| override init() { | ||
| let configuration = WKWebViewConfiguration() | ||
| self.webView = WKWebView(frame: .zero, configuration: configuration) | ||
| super.init() | ||
| webView.navigationDelegate = self | ||
| func updateUIViewController(_ uiViewController: UIViewController, context: Context) { | ||
| guard let navigationController = uiViewController.navigationController else { return } | ||
| context.coordinator.navigationController = navigationController | ||
| if navigationController.interactivePopGestureRecognizer?.delegate !== context.coordinator { | ||
| navigationController.interactivePopGestureRecognizer?.delegate = context.coordinator | ||
| } | ||
| navigationController.interactivePopGestureRecognizer?.isEnabled = true | ||
| } | ||
|
|
||
| func load(_ target: URL) { | ||
| guard currentURL != target else { return } | ||
| currentURL = target | ||
| webView.load(URLRequest(url: target)) | ||
| updateState() | ||
| func makeCoordinator() -> Coordinator { | ||
| Coordinator() | ||
| } | ||
|
|
||
| func reload() { | ||
| webView.reload() | ||
| updateState() | ||
| final class Coordinator: NSObject, UIGestureRecognizerDelegate { | ||
| weak var navigationController: UINavigationController? | ||
|
|
||
| func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool { | ||
| (navigationController?.viewControllers.count ?? 0) > 1 | ||
| } | ||
|
|
||
| func gestureRecognizer( | ||
| _ gestureRecognizer: UIGestureRecognizer, | ||
| shouldRecognizeSimultaneouslyWith otherGestureRecognizer: UIGestureRecognizer | ||
| ) -> Bool { | ||
| true | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
InteractivePopGestureEnabler may override existing gesture delegate permanently.
Setting interactivePopGestureRecognizer?.delegate to the coordinator replaces any existing delegate without restoring it. If the navigation controller has custom pop gesture behaviour, this could cause issues when the view disappears.
🔧 Suggested fix: Store and restore original delegate
final class Coordinator: NSObject, UIGestureRecognizerDelegate {
weak var navigationController: UINavigationController?
+ weak var originalDelegate: UIGestureRecognizerDelegate?
func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool {
(navigationController?.viewControllers.count ?? 0) > 1
}
// ...
}
func updateUIViewController(_ uiViewController: UIViewController, context: Context) {
guard let navigationController = uiViewController.navigationController else { return }
context.coordinator.navigationController = navigationController
if navigationController.interactivePopGestureRecognizer?.delegate !== context.coordinator {
+ context.coordinator.originalDelegate = navigationController.interactivePopGestureRecognizer?.delegate
navigationController.interactivePopGestureRecognizer?.delegate = context.coordinator
}
// ...
}You would also need to restore the delegate in dismantleUIViewController or a cleanup path.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private struct InteractivePopGestureEnabler: UIViewControllerRepresentable { | |
| func makeUIViewController(context: Context) -> UIViewController { | |
| let controller = UIViewController() | |
| controller.view.backgroundColor = .clear | |
| return controller | |
| } | |
| override init() { | |
| let configuration = WKWebViewConfiguration() | |
| self.webView = WKWebView(frame: .zero, configuration: configuration) | |
| super.init() | |
| webView.navigationDelegate = self | |
| func updateUIViewController(_ uiViewController: UIViewController, context: Context) { | |
| guard let navigationController = uiViewController.navigationController else { return } | |
| context.coordinator.navigationController = navigationController | |
| if navigationController.interactivePopGestureRecognizer?.delegate !== context.coordinator { | |
| navigationController.interactivePopGestureRecognizer?.delegate = context.coordinator | |
| } | |
| navigationController.interactivePopGestureRecognizer?.isEnabled = true | |
| } | |
| func load(_ target: URL) { | |
| guard currentURL != target else { return } | |
| currentURL = target | |
| webView.load(URLRequest(url: target)) | |
| updateState() | |
| func makeCoordinator() -> Coordinator { | |
| Coordinator() | |
| } | |
| func reload() { | |
| webView.reload() | |
| updateState() | |
| final class Coordinator: NSObject, UIGestureRecognizerDelegate { | |
| weak var navigationController: UINavigationController? | |
| func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool { | |
| (navigationController?.viewControllers.count ?? 0) > 1 | |
| } | |
| func gestureRecognizer( | |
| _ gestureRecognizer: UIGestureRecognizer, | |
| shouldRecognizeSimultaneouslyWith otherGestureRecognizer: UIGestureRecognizer | |
| ) -> Bool { | |
| true | |
| } | |
| } | |
| } | |
| private struct InteractivePopGestureEnabler: UIViewControllerRepresentable { | |
| func makeUIViewController(context: Context) -> UIViewController { | |
| let controller = UIViewController() | |
| controller.view.backgroundColor = .clear | |
| return controller | |
| } | |
| func updateUIViewController(_ uiViewController: UIViewController, context: Context) { | |
| guard let navigationController = uiViewController.navigationController else { return } | |
| context.coordinator.navigationController = navigationController | |
| if navigationController.interactivePopGestureRecognizer?.delegate !== context.coordinator { | |
| context.coordinator.originalDelegate = navigationController.interactivePopGestureRecognizer?.delegate | |
| navigationController.interactivePopGestureRecognizer?.delegate = context.coordinator | |
| } | |
| navigationController.interactivePopGestureRecognizer?.isEnabled = true | |
| } | |
| func makeCoordinator() -> Coordinator { | |
| Coordinator() | |
| } | |
| final class Coordinator: NSObject, UIGestureRecognizerDelegate { | |
| weak var navigationController: UINavigationController? | |
| weak var originalDelegate: UIGestureRecognizerDelegate? | |
| func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool { | |
| (navigationController?.viewControllers.count ?? 0) > 1 | |
| } | |
| func gestureRecognizer( | |
| _ gestureRecognizer: UIGestureRecognizer, | |
| shouldRecognizeSimultaneouslyWith otherGestureRecognizer: UIGestureRecognizer | |
| ) -> Bool { | |
| true | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In @App/EmbeddedWebView.swift around lines 248 - 282,
InteractivePopGestureEnabler currently unconditionally replaces
navigationController.interactivePopGestureRecognizer?.delegate with its
Coordinator, which can permanently override any existing delegate; modify the
implementation to capture and hold the original delegate (e.g., add
originalDelegate: UIGestureRecognizerDelegate? on Coordinator or a stored
property), only replace the delegate if it differs in updateUIViewController,
and restore the saved original delegate in dismantleUIViewController (or
coordinator deinit/cleanup) when the representable is removed so existing
navigation controller gesture behavior is preserved.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @App/EmbeddedWebView.swift:
- Around line 295-296: The view is using @State for reference-type ViewModels so
SwiftUI won't observe their @Published changes; change @State to @StateObject
for both CommentsViewModel and VotingViewModel declarations and adjust the view
initializer to accept and assign these as StateObject-wrapped instances
(instantiate them inside the view or accept existing ObservableObject instances
and assign to the @StateObject properties) so SwiftUI properly subscribes to
their updates.
🧹 Nitpick comments (3)
App/EmbeddedWebView.swift (3)
268-281: Consider marking Coordinator with@MainActorfor Swift 6 concurrency compliance.The
Coordinatorclass accesses UIKit components (navigationController) but lacks explicit main actor isolation. While UIKit calls these delegate methods on the main thread, adding@MainActorwould make this explicit and avoid potential Swift 6 strict concurrency warnings.Suggested change
- final class Coordinator: NSObject, UIGestureRecognizerDelegate { + @MainActor + final class Coordinator: NSObject, UIGestureRecognizerDelegate {
439-463: Consider migrating from deprecatedNavigationViewtoNavigationStack.
NavigationViewhas been deprecated since iOS 16. If the minimum deployment target supports it, consider migrating toNavigationStackfor improved navigation state management.
707-717: Minor inconsistency in Task wrapper usage.The share button (line 708) wraps its action in
Task { @MainActor in }while the Safari button (line 715) does not. Since both are called from aButtonaction (already on main actor), neither strictly needs theTaskwrapper. Consider making this consistent.Option 1: Remove unnecessary Task wrapper
controlButton(systemName: "square.and.arrow.up") { - Task { @MainActor in - let targetURL = controller.currentURL ?? fallbackURL - ContentSharePresenter.shared.shareURL(targetURL, title: controller.currentTitle) - } + let targetURL = controller.currentURL ?? fallbackURL + ContentSharePresenter.shared.shareURL(targetURL, title: controller.currentTitle) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
App/EmbeddedWebView.swiftShared/Sources/Shared/Services/PresentationContextProvider.swift
🧰 Additional context used
📓 Path-based instructions (2)
**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/*.swift: Use Swift concurrency with async/await in Swift files
Ensure Sendable conformance for thread safety in Swift files
Files:
Shared/Sources/Shared/Services/PresentationContextProvider.swiftApp/EmbeddedWebView.swift
{Features,Shared}/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Inject dependencies into ViewModels via protocols and use DependencyContainer.shared to provide all dependencies
Files:
Shared/Sources/Shared/Services/PresentationContextProvider.swift
🧠 Learnings (4)
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to {Features,DesignSystem}/**/*.swift : Use Combine for reactive bindings with StateObject for view-owned ViewModels and EnvironmentObject for navigation/session state in SwiftUI
Applied to files:
App/EmbeddedWebView.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: In Domain layer, define Models (Post, Comment, User, TextSize), Use Cases (PostUseCase, CommentUseCase, SettingsUseCase, VoteUseCase), and VotingStateProvider protocol and implementation
Applied to files:
App/EmbeddedWebView.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: In Features layer, organize each feature as a separate Swift Package with MVVM pattern: ViewModels as ObservableObject with Published properties and SwiftUI views with EnvironmentObject navigation
Applied to files:
App/EmbeddedWebView.swift
📚 Learning: 2025-12-03T07:05:57.476Z
Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T07:05:57.476Z
Learning: Applies to Features/*/Tests/**/*.swift : Test ViewModels, not Views in the Swift Testing framework
Applied to files:
App/EmbeddedWebView.swift
🧬 Code graph analysis (1)
App/EmbeddedWebView.swift (5)
Shared/Sources/Shared/Services/ContentSharePresenter.swift (1)
shareURL(23-31)Shared/Sources/Shared/DependencyInjection/DependencyContainer.swift (2)
getVotingStateProvider(87-89)getCommentVotingStateProvider(91-102)Shared/Sources/Shared/Services/LinkOpener.swift (1)
openURL(35-58)Shared/Tests/SharedTests/VotingViewModelTests.swift (3)
votingState(33-39)unvote(46-46)upvote(41-44)Data/Sources/Data/PostRepository+Voting.swift (1)
voteLinks(97-141)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (9)
Shared/Sources/Shared/Services/PresentationContextProvider.swift (3)
16-22: LGTM — robust scene lookup logic.The approach of preferring the foreground active scene before falling back to any connected scene is correct and handles multi-scene environments (e.g., iPad multitasking) gracefully.
24-27: LGTM — good key window resolution strategy.The fallback to
windowScene.windows.firstensures a window is returned even during transitions when no window is marked as key. This is a sensible defensive approach.
29-31: LGTM — cleaner derivation via keyWindow.Deriving
rootViewControllerfromkeyWindowis the correct approach and leverages the improved key window lookup logic.App/EmbeddedWebView.swift (6)
17-65: LGTM! Well-structured browser controller.The
@MainActorannotation provides thread safety by confining all access to the main actor, which is appropriate for this UI-driven controller. The separation of state management and navigation actions is clean.
88-91: Good use of Swift concurrency with task modifiers.The
.task(id: url)pattern correctly reloads when the URL changes, and the separate.taskfor monitoring navigations follows modern async/await practices.
212-246: LGTM! Clean composition of embedded browser with comments sheet.The shared
browserControllerbetweenEmbeddedWebViewandPostCommentsSheetensures synchronised navigation state.
581-633: Well-implemented drag gestures with momentum-based snapping.The use of
predictedEndTranslationfor determining final sheet state and the proper cleanup of tracking state inonEndedhandlers demonstrates good UX consideration.
798-882: Good implementation of the collapsed header with proper accessibility support.The voting state handling correctly falls back through optional chain (
votingState?.isUpvoted ?? post.upvoted), and accessibility labels are appropriately provided for interactive elements.
777-789: No action required—.glassEffectAPI is fully supported by the project's deployment target.The project's minimum deployment target is iOS 26.0, which matches the
.glassEffectAPI requirement. Availability guards are unnecessary.Likely an incorrect or invalid review comment.
| @State private var viewModel: CommentsViewModel | ||
| @State private var votingViewModel: VotingViewModel |
There was a problem hiding this comment.
Use @StateObject instead of @State for ObservableObject ViewModels.
@State is designed for value types. Using it with reference types (CommentsViewModel, VotingViewModel) means SwiftUI won't observe changes to their @Published properties, causing the UI to not update when ViewModel state changes.
Suggested fix
- @State private var viewModel: CommentsViewModel
- @State private var votingViewModel: VotingViewModel
+ @StateObject private var viewModel: CommentsViewModel
+ @StateObject private var votingViewModel: VotingViewModelAnd update the initialiser:
- _viewModel = State(initialValue: CommentsViewModel(post: post))
+ _viewModel = StateObject(wrappedValue: CommentsViewModel(post: post))
let container = DependencyContainer.shared
- _votingViewModel = State(initialValue: VotingViewModel(
+ _votingViewModel = StateObject(wrappedValue: VotingViewModel(
votingStateProvider: container.getVotingStateProvider(),
commentVotingStateProvider: container.getCommentVotingStateProvider(),
authenticationUseCase: container.getAuthenticationUseCase()
))Based on learnings, ViewModels should be ObservableObject with @Published properties, and view-owned ViewModels should use @StateObject.
🤖 Prompt for AI Agents
In @App/EmbeddedWebView.swift around lines 295 - 296, The view is using @State
for reference-type ViewModels so SwiftUI won't observe their @Published changes;
change @State to @StateObject for both CommentsViewModel and VotingViewModel
declarations and adjust the view initializer to accept and assign these as
StateObject-wrapped instances (instantiate them inside the view or accept
existing ObservableObject instances and assign to the @StateObject properties)
so SwiftUI properly subscribes to their updates.
1 participant
Summary
Testing
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.