Skip to content
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.

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

Fix refresh behavior #220

Merged
merged 2 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 11 additions & 4 deletions Source/Turbo Navigator/TurboNavigationHierarchyController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -168,21 +168,28 @@ class TurboNavigationHierarchyController {
if navigationController.presentedViewController != nil {
if modalNavigationController.viewControllers.count == 1 {
navigationController.dismiss(animated: proposal.animated)
delegate.refresh(navigationStack: .main)
refreshIfTopViewControllerIsVisitable(from: .main)
} else {
modalNavigationController.popViewController(animated: proposal.animated)
delegate.refresh(navigationStack: .modal)
refreshIfTopViewControllerIsVisitable(from: .modal)
}
} else {
navigationController.popViewController(animated: proposal.animated)
delegate.refresh(navigationStack: .main)
refreshIfTopViewControllerIsVisitable(from: .main)
}
}

private func refreshIfTopViewControllerIsVisitable(from stack: NavigationStackType) {
if let navControllerTopmostVisitable = navController(for: stack).topViewController as? Visitable {
Copy link
Member Author

Choose a reason for hiding this comment

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

We should discuss the best way to pass this refresh message to a non-Visitable view controller.

Copy link
Contributor

Choose a reason for hiding this comment

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

As you mentioned on the call, leveraging the PathConfigurationIdentifiable protocol is an option here.

delegate.refreshVisitable(navigationStack: stack,
newTopmostVisitable: navControllerTopmostVisitable)
}
}

private func clearAll(via proposal: VisitProposal) {
navigationController.dismiss(animated: proposal.animated)
navigationController.popToRootViewController(animated: proposal.animated)
delegate.refresh(navigationStack: .main)
refreshIfTopViewControllerIsVisitable(from: .main)
}

private func replaceRoot(with controller: UIViewController, via proposal: VisitProposal) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
import SafariServices
import WebKit

/// Implement to be notified when certain navigations are performed
/// or to render a native controller instead of a Turbo web visit.
protocol TurboNavigationHierarchyControllerDelegate: AnyObject {
func visit(_ : Visitable, on: TurboNavigationHierarchyController.NavigationStackType, with: VisitOptions)
func refresh(navigationStack: TurboNavigationHierarchyController.NavigationStackType)

/// Once the navigation hierarchy is modified, begin a visit on a navigation controller.
///
/// - Parameters:
/// - _: the Visitable destination
/// - on: the navigation controller that was modified
/// - with: the visit options
func visit(_ : Visitable,
on: TurboNavigationHierarchyController.NavigationStackType,
with: VisitOptions)

/// A refresh will pop (or dismiss) then ask the session to refresh the previous (or underlying) Visitable.
///
/// - Parameters:
/// - navigationStack: the stack where the refresh is happening
/// - newTopmostVisitable: the visitable to be refreshed
func refreshVisitable(navigationStack: TurboNavigationHierarchyController.NavigationStackType,
newTopmostVisitable: Visitable)
}
9 changes: 6 additions & 3 deletions Source/Turbo Navigator/TurboNavigator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,13 @@ extension TurboNavigator: TurboNavigationHierarchyControllerDelegate {
}
}

func refresh(navigationStack: TurboNavigationHierarchyController.NavigationStackType) {
func refreshVisitable(navigationStack: TurboNavigationHierarchyController.NavigationStackType,
newTopmostVisitable: any Visitable) {
switch navigationStack {
case .main: session.reload()
Copy link
Member Author

@olivaresf olivaresf Jul 4, 2024

Choose a reason for hiding this comment

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

We can't use session.reload() because topmostVisitable does not change when a view controller is popped (or dismissed) programmatically. For example:

  1. Propose visit to URL "/one"
  2. Visit completes, session.topmostVisitable = "one"
  3. Propose visit to URL "/two"
  4. Visit completes, session.topmostVisitable = "two"
  5. Propose visit to URL "/refresh_historical_location" with presentation: refresh
  6. Visitable with visitableURL = "/two" is popped.
  7. session.reload() is called, which reloads the topmost visitable ("/two" set during step 4).

case .modal: modalSession.reload()
case .main:
session.visit(newTopmostVisitable, action: .restore)
case .modal:
modalSession.visit(newTopmostVisitable, action: .restore)
}
}
}
Expand Down
57 changes: 56 additions & 1 deletion Tests/Turbo Navigator/TurboNavigatorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,61 @@ final class TurboNavigationHierarchyControllerTests: XCTestCase {
XCTAssert(navigationController.viewControllers.last is VisitableViewController)
assertVisited(url: proposal.url, on: .main)
}

func test_default_default_refresh_refreshesPreviousController() {
navigator.route(oneURL)
XCTAssertEqual(navigationController.viewControllers.count, 1)

navigator.route(twoURL)
XCTAssertEqual(navigator.rootViewController.viewControllers.count, 2)

/// Refreshing should pop the view controller and refresh the underlying controller.
let proposal = VisitProposal(presentation: .refresh)
navigator.route(proposal)

let visitable = navigator.session.activeVisitable as! VisitableViewController
XCTAssertEqual(visitable.visitableURL, oneURL)
XCTAssertEqual(navigator.rootViewController.viewControllers.count, 1)
}

func test_default_modal_refresh_refreshesPreviousController() {
navigationController.pushViewController(UIViewController(), animated: false)
XCTAssertEqual(navigationController.viewControllers.count, 1)

let oneURLProposal = VisitProposal(path: "/one", context: .modal)
navigator.route(oneURLProposal)

let twoURLProposal = VisitProposal(path: "/two", context: .modal)
navigator.route(twoURLProposal)
XCTAssertEqual(modalNavigationController.viewControllers.count, 2)

/// Refreshing should pop the view controller and refresh the underlying controller.
let proposal = VisitProposal(presentation: .refresh)
navigator.route(proposal)

let visitable = navigator.modalSession.activeVisitable as! VisitableViewController
XCTAssertEqual(visitable.visitableURL, oneURL)
XCTAssertEqual(modalNavigationController.viewControllers.count, 1)
}

func test_default_modal_refresh_dismissesAndRefreshesMainStackTopViewController() {
navigator.route(oneURL)
XCTAssertEqual(navigationController.viewControllers.count, 1)

let twoURLProposal = VisitProposal(path: "/two", context: .modal)
navigator.route(twoURLProposal)
XCTAssertEqual(modalNavigationController.viewControllers.count, 1)

/// Refreshing should dismiss the view controller and refresh the underlying controller.
let proposal = VisitProposal(context: .modal, presentation: .refresh)
navigator.route(proposal)

let visitable = navigator.session.activeVisitable as! VisitableViewController
XCTAssertEqual(visitable.visitableURL, oneURL)

XCTAssertNil(navigationController.presentedViewController)
XCTAssertEqual(navigator.rootViewController.viewControllers.count, 1)
}

func test_default_modal_default_presentsModal() {
navigationController.pushViewController(UIViewController(), animated: false)
Expand Down Expand Up @@ -309,7 +364,7 @@ final class TurboNavigationHierarchyControllerTests: XCTestCase {

private class EmptyNavigationDelegate: TurboNavigationHierarchyControllerDelegate {
func visit(_: Visitable, on: TurboNavigationHierarchyController.NavigationStackType, with: VisitOptions) {}
func refresh(navigationStack: TurboNavigationHierarchyController.NavigationStackType) {}
func refreshVisitable(navigationStack: TurboNavigationHierarchyController.NavigationStackType, newTopmostVisitable: any Visitable) { }
}

// MARK: - VisitProposal extension
Expand Down