Skip to content

Conversation

@graycreate
Copy link
Member

Summary

  • Remove page-level overlay ProgressView from FeedDetailPage
  • Keep only RichContentView's internal loading indicator
  • Simplify NewsContentView by removing unused rendered binding

This fixes the issue where two loading spinners were showing simultaneously on the topic detail page.

Test plan

  • Open a topic detail page and verify only one loading spinner appears
  • Verify content loads correctly after loading completes
  • Check pull-to-refresh still works properly

🤖 Generated with Claude Code

Remove page-level overlay ProgressView, keeping only RichContentView's
internal loading indicator to avoid showing two loading spinners.

Changes:
- Remove showProgressView computed property and rendered state
- Remove .overlay with ProgressView from FeedDetailPage
- Simplify NewsContentView by removing rendered binding

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings December 21, 2025 01:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the duplicate loading spinner issue on topic detail pages by removing the page-level ProgressView overlay and relying solely on RichContentView's internal loading indicator. The changes simplify state management by removing the rendered binding that tracked RichContentView's rendering completion.

Key changes:

  • Removed page-level ProgressView overlay that was causing duplicate loading indicators
  • Simplified NewsContentView by removing unused rendered binding parameter and render completion callbacks
  • Streamlined loading state management to use only RichContentView's built-in loading indicator

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
V2er/View/FeedDetail/FeedDetailPage.swift Removed rendered state variable, showProgressView computed property, and the page-level ProgressView overlay
V2er/View/FeedDetail/NewsContentView.swift Removed rendered binding parameter from initializer and removed onRenderCompleted/onRenderFailed callbacks that were only used to update that binding
Comments suppressed due to low confidence (1)

V2er/View/FeedDetail/FeedDetailPage.swift:157

  • The removal of the page-level progress view overlay eliminates all loading feedback during initial data fetch. When the page first loads, the condition if !isContentEmpty prevents NewsContentView (and thus RichContentView) from rendering until data arrives. This means users will see no loading indicator during the network request for initial content.

Consider either:

  1. Restoring a page-level loading indicator that shows when state.showProgressView is true AND isContentEmpty (for initial loads only)
  2. Or showing a placeholder/skeleton view in the content section during initial load

The current implementation only addresses the duplicate spinner issue when content is being rendered, but introduces a regression for the initial load experience.

            if !isContentEmpty {
                NewsContentView(state.model.contentInfo)
                    .padding(.horizontal, 10)
                    .listRowInsets(EdgeInsets())
                    .listRowSeparator(.hidden)
                    .listRowBackground(Color.itemBg)
            }

@github-actions
Copy link

Code Coverage Report ❌

Current coverage: 30.94%

@graycreate graycreate merged commit 9ec2ef6 into main Dec 21, 2025
12 checks passed
@graycreate graycreate deleted the bugfix/remove-duplicate-loading-view branch December 21, 2025 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants