Skip to content

Fix: Detail page auto-dismissal caused by nested navigation #79

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

Merged
merged 2 commits into from
Dec 1, 2025
Merged

Fix: Detail page auto-dismissal caused by nested navigation #79

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e4fbb5
fix: resolve detail page auto-dismissal caused by nested navigation
graycreate Dec 1, 2025
33f5183
fix: improve UI test reliability by replacing sleep with proper wait
graycreate Dec 1, 2025
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
10 changes: 5 additions & 5 deletions V2er/View/Feed/FeedItemView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,11 @@ import SwiftUI

struct FeedItemView<Data: FeedItemProtocol>: View {
let data: Data

var body: some View {
VStack(spacing: 0) {
HStack(alignment: .top) {
NavigationLink(destination: UserDetailPage(userId: data.userName ?? .empty)) {
AvatarView(url: data.avatar)
}
AvatarView(url: data.avatar)
VStack(alignment: .leading, spacing: 2) {
Text(data.userName.safe)
.font(.footnote)
Expand All @@ -26,7 +24,9 @@ struct FeedItemView<Data: FeedItemProtocol>: View {
.lineLimit(1)
.foregroundColor(Color.tintColor)
Spacer()
NodeView(id: data.nodeId.safe, name: data.nodeName.safe)
Text(data.nodeName.safe)
.font(.footnote)
.foregroundColor(.gray)
Comment on lines +27 to +29
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Replacing NodeView with plain Text removes the navigation functionality to TagDetailPage. Users can no longer tap on node names to navigate to tag details from feed items. Consider restoring this functionality with proper NavigationStack-compatible navigation.

Copilot uses AI. Check for mistakes.
}
Text(data.title.safe)
// .fontWeight(.medium)
Expand Down
6 changes: 1 addition & 5 deletions V2er/View/Feed/FeedPage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ struct FeedPage: BaseHomePageView {
var selecedTab: TabId

var isSelected: Bool {
let selected = selecedTab == .feed
if selected && !state.hasLoadedOnce {
dispatch(FeedActions.FetchData.Start(autoLoad: true))
}
return selected
selecedTab == .feed
}
Comment on lines 18 to 20
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

The removed logic in isSelected that dispatched FetchData.Start when the tab was selected is now duplicated in the onAppear handler (lines 24-28). However, isSelected is a computed property that may be called multiple times. Consider if the original pattern was intentional to trigger fetches on tab selection separately from view appearance. Check if ExplorePage and MessagePage follow similar patterns and ensure consistency.

Copilot uses AI. Check for mistakes.

var body: some View {
Expand Down
46 changes: 14 additions & 32 deletions V2er/View/FeedDetail/FeedDetailPage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,9 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
return state.showProgressView
|| (!isContentEmpty && !self.rendered)
}

var body: some View {
contentView
.navigatable()
.sheet(isPresented: $showingSafari) {
if let url = safariURL {
SafariView(url: url)
Expand All @@ -70,7 +69,6 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
@ViewBuilder
private var contentView: some View {
VStack (spacing: 0) {
// TODO: improve here
VStack(spacing: 0) {
AuthorInfoView(initData: initData, data: state.model.headerInfo)
if !isContentEmpty {
Expand All @@ -89,7 +87,6 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
withAnimation {
hideTitleViews = !(scrollY <= -100)
}
// replyIsFocused = false
}
.onTapGesture {
replyIsFocused = false
Expand All @@ -111,16 +108,15 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
dispatch(FeedDetailActions.FetchData.Start(id: instanceId, feedId: initData?.id, autoLoad: !state.hasLoadedOnce))
}
.onDisappear {
if !isPresented {
log("onPageClosed----->")
let data: FeedInfo.Item?
if state.model.headerInfo != nil {
data = state.model.headerInfo?.toFeedItemInfo()
} else {
data = initData
}
dispatch(MyRecentActions.RecordAction(data: data))
guard !isPresented else { return }
log("onPageClosed----->")
let data: FeedInfo.Item?
if state.model.headerInfo != nil {
data = state.model.headerInfo?.toFeedItemInfo()
} else {
data = initData
}
dispatch(MyRecentActions.RecordAction(data: data))
}
}

Expand Down Expand Up @@ -150,18 +146,14 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
}
.background(Color.lightGray)
.clipShape(RoundedRectangle(cornerRadius: 12))
// if isKeyboardVisiable {
// actionBar
// .transition(.opacity)
// }
}
.padding(.bottom, isKeyboardVisiable ? 0 : topSafeAreaInset().bottom * 0.9)
.padding(.top, 10)
.padding(.horizontal, 10)
.background(Color.itemBg)
}
}

@ViewBuilder
private var actionBar: some View {
HStack (spacing: 10) {
Expand All @@ -182,7 +174,7 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
.padding(.vertical, 10)
.padding(.horizontal, 16)
}

@ViewBuilder
private var navBar: some View {
NavbarHostView(paddingH: 0) {
Expand All @@ -197,10 +189,7 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
.foregroundColor(.tintColor)
}
Group {
// FIXME: use real value
NavigationLink(destination: UserDetailPage(userId: initData?.id ?? .empty)) {
AvatarView(url: state.model.headerInfo?.avatar ?? .empty, size: 32)
}
AvatarView(url: state.model.headerInfo?.avatar ?? .empty, size: 32)
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Removing the NavigationLink to UserDetailPage removes the ability to navigate to user profiles from the detail page. This functionality should be restored once navigation issues are resolved, or alternative navigation should be provided (e.g., using NavigationStack's navigation state).

Copilot uses AI. Check for mistakes.
VStack(alignment: .leading) {
Text("话题")
.font(.headline)
Expand Down Expand Up @@ -271,7 +260,7 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
}
.visualBlur()
}

@ViewBuilder
private var replayListView: some View {
LazyVStack(spacing: 0) {
Expand All @@ -280,12 +269,5 @@ struct FeedDetailPage: StateView, KeyboardReadable, InstanceIdentifiable {
}
}
}

}

//struct NewsDetailPage_Previews: PreviewProvider {
// static var previews: some View {
// FeedDetailPage(id: .empty)
// .environmentObject(Store.shared)
// }
//}
}
2 changes: 1 addition & 1 deletion V2er/View/MainPage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ struct MainPage: StateView {
}

var body: some View {
NavigationView {
NavigationStack {
ZStack {
TabView(selection: tabSelection) {
// Feed Tab
Expand Down
1 change: 0 additions & 1 deletion V2er/View/Me/MyFavoritePage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ struct MyFavoritePage: StateView {

var body: some View {
contentView
.navigatable()
}

@ViewBuilder
Expand Down
8 changes: 4 additions & 4 deletions V2er/View/Me/MyRecentPage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ struct RecentItemView<Data: FeedItemProtocol>: View {
var body: some View {
VStack(spacing: 0) {
HStack(alignment: .top) {
NavigationLink(destination: UserDetailPage(userId: data.userName.safe)) {
AvatarView(url: data.avatar)
}
AvatarView(url: data.avatar)
VStack(alignment: .leading, spacing: 5) {
Text(data.userName.safe)
.lineLimit(1)
Expand All @@ -57,7 +55,9 @@ struct RecentItemView<Data: FeedItemProtocol>: View {
.foregroundColor(Color.tintColor)
}
Spacer()
NodeView(id: data.nodeId.safe, name: data.nodeName.safe)
Text(data.nodeName.safe)
.font(.footnote)
.foregroundColor(.gray)
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Replacing NodeView with plain Text removes the navigation functionality to TagDetailPage. This breaks the ability to navigate to tag details from recent items. If this is intentional due to navigation conflicts, consider restoring this functionality once the navigation architecture is stable, or document why this navigation was removed.

Suggested change
.foregroundColor(.gray)
.foregroundColor(.gray)
.to {
TagDetailPage(nodeName: data.nodeName.safe)
}

Copilot uses AI. Check for mistakes.
}
Text(data.title.safe)
.greedyWidth(.leading)
Expand Down
1 change: 0 additions & 1 deletion V2er/View/Me/UserDetailPage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ struct UserDetailPage: StateView {

var body: some View {
contentView
.navigatable()
.statusBarStyle(statusBarStyle, original: .darkContent)
}

Expand Down
1 change: 0 additions & 1 deletion V2er/View/Tag/TagDetailPage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ struct TagDetailPage: StateView, InstanceIdentifiable {

var body: some View {
contentView
.navigatable()
.statusBarStyle(statusBarStyle, original: .darkContent)
}

Expand Down
26 changes: 11 additions & 15 deletions V2er/View/Widget/NavbarHostView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,22 +110,18 @@ struct NavBarModifier: ViewModifier {
let title: String

func body(content: Content) -> some View {
NavigationView {
content
.safeAreaInset(edge: .top, spacing: 0) {
NavbarTitleView {
Text(title)
.font(.headline)
} onBackPressed: {
dismiss()
}
content
.safeAreaInset(edge: .top, spacing: 0) {
NavbarTitleView {
Text(title)
.font(.headline)
} onBackPressed: {
dismiss()
}
.background(Color.bgColor)
.navigationBarHidden(true)
.ignoresSafeArea(.all)
}
.navigationBarHidden(true)
.ignoresSafeArea(.all)
}
.background(Color.bgColor)
.navigationBarHidden(true)
.ignoresSafeArea(.container)
}
}

Expand Down
41 changes: 41 additions & 0 deletions V2erUITests/V2erUITests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// In UI tests it is usually best to stop immediately when a failure occurs.
continueAfterFailure = false

// In UI tests it’s important to set the initial state - such as interface orientation - required for your tests before they run. The setUp method is a good place to do this.

Check failure on line 19 in V2erUITests/V2erUITests.swift

View workflow job for this annotation

GitHub Actions / SwiftLint 

Line Length Violation: Line should be 120 characters or less; currently it has 182 characters (line_length)
}

override func tearDownWithError() throws {
Expand All @@ -32,6 +32,47 @@
// Use XCTAssert and related functions to verify your tests produce the correct results.
}

func testFeedNavigationStaysOpen() throws {
let app = XCUIApplication()
app.launch()

// Wait for feed to load
sleep(6)

// Find a feed item by looking for text that says "评论" (comment count)
let commentLabels = app.staticTexts.matching(NSPredicate(format: "label CONTAINS '评论'"))
guard commentLabels.count > 0 else {
XCTFail("No feed items found (no comment labels)")
return
}

// Tap on the first comment label's parent area
let firstCommentLabel = commentLabels.element(boundBy: 0)
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Using sleep() in UI tests is fragile and increases test execution time. Replace with XCTest's waitForExistence(timeout:) method. For example, wait for a feed item to appear: XCTAssertTrue(commentLabels.firstMatch.waitForExistence(timeout: 10), \"Feed should load\").

Suggested change
// Wait for feed to load
sleep(6)
// Find a feed item by looking for text that says "评论" (comment count)
let commentLabels = app.staticTexts.matching(NSPredicate(format: "label CONTAINS '评论'"))
guard commentLabels.count > 0 else {
XCTFail("No feed items found (no comment labels)")
return
}
// Tap on the first comment label's parent area
let firstCommentLabel = commentLabels.element(boundBy: 0)
// Wait for feed to load by waiting for the first comment label to appear
let commentLabels = app.staticTexts.matching(NSPredicate(format: "label CONTAINS '评论'"))
let firstCommentLabel = commentLabels.element(boundBy: 0)
XCTAssertTrue(firstCommentLabel.waitForExistence(timeout: 10), "Feed should load and show at least one comment label")
// Find a feed item by looking for text that says "评论" (comment count)
guard commentLabels.count > 0 else {
XCTFail("No feed items found (no comment labels)")
return
}
// Tap on the first comment label's parent area

Copilot uses AI. Check for mistakes.
XCTAssertTrue(firstCommentLabel.waitForExistence(timeout: 10), "Comment label should exist")
firstCommentLabel.tap()

// Wait for navigation animation
sleep(3)

// Verify we're on detail page - look for "话题" text or "发表回复" placeholder
let topicLabel = app.staticTexts["话题"]
let replyPlaceholder = app.textViews.firstMatch

Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Replace sleep(3) with waitForExistence(timeout:) on the expected detail page element to make the test more reliable and faster.

Suggested change
// Wait for navigation animation
sleep(3)
// Verify we're on detail page - look for "话题" text or "发表回复" placeholder
let topicLabel = app.staticTexts["话题"]
let replyPlaceholder = app.textViews.firstMatch
// Wait for navigation animation by waiting for detail page element to appear
let topicLabel = app.staticTexts["话题"]
let replyPlaceholder = app.textViews.firstMatch
let appeared = topicLabel.waitForExistence(timeout: 5) || replyPlaceholder.waitForExistence(timeout: 5)
XCTAssertTrue(appeared, "Detail page did not appear after tapping comment label")
// Verify we're on detail page - look for "话题" text or "发表回复" placeholder

Copilot uses AI. Check for mistakes.
let onDetailPage = topicLabel.exists || replyPlaceholder.exists

XCTAssertTrue(onDetailPage, "Should be on detail page after navigation")

// Wait longer to verify page stays open (the original bug was page dismissing)
sleep(3)
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Replace sleep(3) with a proper assertion mechanism. If testing that the page stays open, consider using XCTWaiter to wait a specific time then assert, or verify the element remains accessible.

Suggested change
sleep(3)
let expectation = XCTestExpectation(description: "Wait 3 seconds to verify detail page stays open")
let result = XCTWaiter.wait(for: [expectation], timeout: 3.0)

Copilot uses AI. Check for mistakes.

// Verify still on detail page
let stillOnDetail = topicLabel.exists || replyPlaceholder.exists
XCTAssertTrue(stillOnDetail, "Detail page should remain open after 3 seconds")

// Leave app open for visual inspection
sleep(10)
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Remove this sleep(10) for visual inspection. UI tests should be automated and not require manual inspection during execution. If manual testing is needed, do it separately from automated tests.

Suggested change
sleep(10)
// Removed sleep(10) for automation; UI tests should not require manual inspection.

Copilot uses AI. Check for mistakes.
}

func testLaunchPerformance() throws {
if #available(macOS 10.15, iOS 13.0, tvOS 13.0, *) {
// This measures how long it takes to launch your application.
Expand Down
Loading