Skip to content

feat: optimize FeedDetailPage list to reduce jumping #136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
graycreate wants to merge 1 commit into from
Closed

feat: optimize FeedDetailPage list to reduce jumping #136

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 32 additions & 25 deletions V2er/View/FeedDetail/FeedDetailPage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
@State var isKeyboardVisiable = false
@State private var isLoadingMore = false
@State private var contentReady = false
@State private var repliesReady = false
Comment on lines 46 to +47
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

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

The state variables contentReady and repliesReady are not reset when new data is loaded. When a user pulls to refresh or when data is reloaded, these states remain true, preventing the staged appearance animation from running again. Consider adding an onChange handler that resets these states when state.model changes or when a refresh begins.

Copilot uses AI. Check for mistakes.
@FocusState private var replyIsFocused: Bool
var initData: FeedInfo.Item? = nil
var id: String
Expand Down Expand Up @@ -206,8 +207,12 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
// Content Section
if !isContentEmpty {
NewsContentView(state.model.contentInfo) {
withAnimation {
contentReady = true
contentReady = true
// Show replies after a short delay for smoother transition
DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) {
withAnimation(.easeInOut(duration: 0.2)) {
repliesReady = true
}
}
Comment on lines +212 to 216
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

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

The DispatchQueue.main.asyncAfter closure captures self implicitly, which could lead to a retain cycle if the view is dismissed before the delay completes. Additionally, if the view is dismissed or refreshed before the 0.15 second delay completes, the delayed closure will still execute and set repliesReady = true on a potentially stale view. Consider using a weak self capture or canceling the delayed work when appropriate.

Copilot uses AI. Check for mistakes.
}
.padding(.horizontal, 10)
Expand All @@ -216,31 +221,31 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
.listRowBackground(Color.itemBg)
}

// Show postscripts and replies only after content is ready
if contentReady || isContentEmpty {
// Postscripts Section (附言)
ForEach(state.model.postscripts) { postscript in
PostscriptItemView(postscript: postscript)
.listRowInsets(EdgeInsets())
.listRowSeparator(.hidden)
.listRowBackground(Color.itemBg)
}
// Postscripts Section (附言) - always in layout, fade in when ready
ForEach(state.model.postscripts) { postscript in
PostscriptItemView(postscript: postscript)
.listRowInsets(EdgeInsets())
.listRowSeparator(.hidden)
.listRowBackground(Color.itemBg)
.opacity(contentReady || isContentEmpty ? 1 : 0)
}

// Reply Section Header with Sort Toggle
if !state.model.replyInfo.items.isEmpty {
replySectionHeader
.listRowInsets(EdgeInsets())
.listRowSeparator(.hidden)
.listRowBackground(Color.itemBg)
}
// Reply Section Header with Sort Toggle
if !state.model.replyInfo.items.isEmpty {
replySectionHeader
.listRowInsets(EdgeInsets())
.listRowSeparator(.hidden)
.listRowBackground(Color.itemBg)
.opacity(repliesReady || isContentEmpty ? 1 : 0)
}

// Reply Section
ForEach(sortedReplies, id: \.floor) { item in
ReplyItemView(info: item, topicId: id)
.listRowInsets(EdgeInsets())
.listRowSeparator(.hidden)
.listRowBackground(Color.itemBg)
}
// Reply Section - always in layout, fade in when ready
ForEach(sortedReplies, id: \.floor) { item in
ReplyItemView(info: item, topicId: id)
.listRowInsets(EdgeInsets())
.listRowSeparator(.hidden)
.listRowBackground(Color.itemBg)
.opacity(repliesReady || isContentEmpty ? 1 : 0)
}

// Load More Indicator
Expand Down Expand Up @@ -272,6 +277,8 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
.scrollContentBackground(.hidden)
.background(Color.itemBg)
.environment(\.defaultMinListRowHeight, 1)
.animation(.easeIn(duration: 0.15), value: contentReady)
.animation(.easeIn(duration: 0.15), value: repliesReady)
Comment on lines +280 to +281
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

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

The animation modifier is applied to the entire List based on contentReady and repliesReady state changes. This causes all changes to the List to be animated with easeInOut, not just the appearance of content and replies. This could lead to unintended animations for other state changes, such as when loading more replies or when the reply section header appears. Consider applying the animation only to the specific views that should be animated, or use a more targeted approach.

Suggested change
.animation(.easeIn(duration: 0.15), value: contentReady)
.animation(.easeIn(duration: 0.15), value: repliesReady)

Copilot uses AI. Check for mistakes.
.refreshable {
await run(action: FeedDetailActions.FetchData.Start(id: instanceId, feedId: initData?.id))
}
Expand Down
Loading