Show loading state when switching category

[weiran]

Summary by CodeRabbit

• Improvements
  • Sidebar selection now resets when switching post types to avoid stale selections.
  • Loading indicator refined: shown only during active loading when no posts exist and not in bookmarks view.
  • Feed list identity updated so the list refreshes correctly when changing between post types, ensuring the view clears and reloads as expected.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Reset selectedPostId when selectedPostType changes, adjust loading-state rendering to show AppLoadingStateView only when loading and posts are empty (excluding bookmarks), add .id(selectedPostType) to the feed list, and change ViewModel reset to accept clearPosts with changePostType clearing posts before reload.

Changes

Cohort / File(s)	Summary
Feed view: selection, loading & identity Features/Feed/Sources/Feed/FeedView.swift	- Added onChange for selectedPostType to clear selectedPostId. - Changed loading conditional: show AppLoadingStateView only when isLoading is true, posts are empty, and selectedPostType ≠ bookmarks. - Applied .id(selectedPostType) to feedListView to change view identity when the type changes.
Feed view model: explicit clear on type change Features/Feed/Sources/Feed/FeedViewModel.swift	- reset signature updated to private func reset(clearPosts: Bool = false) and clears feedLoader.data when clearPosts is true. - changePostType(_:) now calls reset(clearPosts: true) before reloading, replacing the prior refreshFeed() call. - refresh retains prior behaviour by calling reset() (without clearing posts) then feedLoader.refresh().

Sequence Diagram(s)

sequenceDiagram
    participant UI as FeedView
    participant VM as FeedViewModel
    participant List as feedListView

    UI->>VM: User selects new selectedPostType
    VM-->>UI: selectedPostType updated
    Note over UI: onChange clears selectedPostId
    UI->>UI: selectedPostId = nil
    UI->>List: feedListView re-evaluated (.id(selectedPostType))
    Note right of VM: changePostType -> reset(clearPosts: true) -> feedLoader.refresh()
    alt isLoading && posts.isEmpty && selectedPostType != bookmarks
        UI->>UI: render AppLoadingStateView
    else
        UI->>UI: render feedListView content
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify reset(clearPosts: true) reliably clears in-flight data and that feedLoader.data clearing has no unexpected side effects.
• Confirm .id(selectedPostType) does not unintentionally discard desired view state (selection, scroll position) beyond selectedPostId.
• Test loading UI for bookmark type to ensure no missing loading indicators or regressions.
• Check concurrency/async interactions around changePostType and feedLoader.refresh().

Poem

🐰
A hop, a swap, the type now new,
Old id drops and views renew,
Loading waits for non-bookmark streams,
Feed clears clean to chase new dreams,
A rabbit twitches, softly pleased.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check	❓ Inconclusive	The title "Add loading state and scroll to top on category switch" refers to aspects present in the changeset. The loading state addition is clearly reflected in the second bullet point, which describes modified loading state rendering to show AppLoadingStateView under specific conditions. The category switch aspect is addressed through the onChange handler for selectedPostType and the id modifier on feedListView. However, the summary does not explicitly mention scroll-to-top functionality being implemented; whilst the view identity change may produce this effect, it is not clearly stated as an intentional outcome, creating ambiguity about whether the title accurately represents the implemented behaviour.	To resolve this, clarify whether the id modifier using selectedPostType as the view identity explicitly implements a scroll-to-top behaviour, or if it merely affects view refresh and identity. If scroll-to-top is not explicitly implemented, consider revising the title to more accurately reflect the actual changes, such as "Reset post selection and add loading state on post type change" or "Add loading state for post type switching."

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 15c4425 and 478cd20.

📒 Files selected for processing (1)

• Features/Feed/Sources/Feed/FeedViewModel.swift (1 hunks)

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ba34fb and d8e154d.

📒 Files selected for processing (1)

• Features/Feed/Sources/Feed/FeedView.swift (2 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

Features/*/Sources/**/*.swift

📄 CodeRabbit inference engine (AGENTS.md)

Features/*/Sources/**/*.swift: SwiftUI views in Features should use @EnvironmentObject for navigation
Annotate UI code with @mainactor

Files:

• Features/Feed/Sources/Feed/FeedView.swift

Features/*/Sources/**/*View.swift

📄 CodeRabbit inference engine (AGENTS.md)

Use @StateObject for view-owned ViewModels in SwiftUI Views

Files:

• Features/Feed/Sources/Feed/FeedView.swift

**/*.swift

📄 CodeRabbit inference engine (AGENTS.md)

**/*.swift: Use Swift concurrency (async/await) instead of completion handlers where appropriate
Adopt Sendable where needed to ensure thread safety across concurrency boundaries

Files:

• Features/Feed/Sources/Feed/FeedView.swift

🧠 Learnings (2)

📚 Learning: 2025-09-14T17:42:10.615Z

Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T17:42:10.615Z
Learning: Applies to Features/*/Sources/**/*View.swift : Use StateObject for view-owned ViewModels in SwiftUI Views

Applied to files:

• Features/Feed/Sources/Feed/FeedView.swift

📚 Learning: 2025-09-14T17:42:10.615Z

Learnt from: CR
Repo: weiran/Hackers PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T17:42:10.615Z
Learning: Applies to Features/*/Sources/**/*ViewModel.swift : Feature ViewModels must be ObservableObject and expose state via Published properties

Applied to files:

• Features/Feed/Sources/Feed/FeedView.swift

🧬 Code graph analysis (1)

Features/Feed/Sources/Feed/FeedView.swift (2)

Features/Feed/Sources/Feed/FeedViewModel.swift (4)

• isLoadingMore (13-154)
• replacePost (148-153)
• changePostType (129-135)
• reset (137-144)

Features/Feed/Tests/FeedTests/FeedViewTests.swift (1)

• selectedPost (17-51)

⏰ 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). (1)

• GitHub Check: test

🔇 Additional comments (1)

Features/Feed/Sources/Feed/FeedView.swift (1)

124-124: LGTM: Bookmarks correctly excluded from loading indicator.

The condition properly excludes bookmarks from displaying the loading state, which is consistent with the bookmark-specific empty state handling. When bookmarks are loading and empty, the view falls through to display the feed list (which will be empty), avoiding a potentially confusing loading indicator for what is likely an instant local operation.