Fixes scroll position lost when navigating back with keyboard open

Source/Turbo/Session/RestorationState.swift

@@ -0,0 +1,11 @@
+import Foundation
+
+class RestorationState {
+    var identifier: String?
+    var scrollPosition: CGPoint?
+
+    init(identifier: String? = nil, scrollPosition: CGPoint? = nil) {
+        self.identifier = identifier
+        self.scrollPosition = scrollPosition
+    }
+}

Source/Turbo/Session/Session.swift

@@ -73,7 +73,7 @@ public class Session: NSObject {
 
     private func makeVisit(for visitable: Visitable, options: VisitOptions) -> Visit {
         if initialized {
-            return JavaScriptVisit(visitable: visitable, options: options, bridge: bridge, restorationIdentifier: restorationIdentifier(for: visitable))
+            return JavaScriptVisit(visitable: visitable, options: options, bridge: bridge, restorationIdentifier: restorationState(for: visitable).identifier)
         } else {
             return ColdBootVisit(visitable: visitable, options: options, bridge: bridge)
         }
@@ -107,15 +107,21 @@ public class Session: NSObject {
 
     private var activatedVisitable: Visitable?
 
-    private func activateVisitable(_ visitable: Visitable) {
+    internal func activateVisitable(_ visitable: Visitable) {
         guard !isActivatedVisitable(visitable) else { return }
 
         deactivateActivatedVisitable()
         visitable.activateVisitableWebView(webView)
         activatedVisitable = visitable
+
+        if isRevealedByPop(visitable), let position = restorationState(for: visitable).scrollPosition {
+            webView.scrollView.contentOffset = position
+        } else {
+            webView.scrollView.contentOffset = CGPoint(x: 0, y: -webView.scrollView.adjustedContentInset.top)
+        }
     }
 
-    private func deactivateActivatedVisitable() {
+    internal func deactivateActivatedVisitable() {
         guard let visitable = activatedVisitable else { return }
         deactivateVisitable(visitable, showScreenshot: true)
     }
@@ -128,6 +134,10 @@ public class Session: NSObject {
             visitable.showVisitableScreenshot()
         }
 
+        if !visitable.visitableViewController.isMovingFromParent {
+            restorationState(for: visitable).scrollPosition = webView.scrollView.contentOffset
+        }
+
         visitable.deactivateVisitableWebView()
         activatedVisitable = nil
     }
@@ -136,16 +146,24 @@ public class Session: NSObject {
         return visitable === activatedVisitable
     }
 
-    // MARK: Restoration Identifiers
+    // MARK: Restoration state management
 
-    private var visitableRestorationIdentifiers = NSMapTable<UIViewController, NSString>(keyOptions: NSPointerFunctions.Options.weakMemory, valueOptions: [])
+    private var visitableRestorationStates = NSMapTable<UIViewController, RestorationState>(keyOptions: .weakMemory, valueOptions: .strongMemory)
 
-    private func restorationIdentifier(for visitable: Visitable) -> String? {
-        return visitableRestorationIdentifiers.object(forKey: visitable.visitableViewController) as String?
+    private func isRevealedByPop(_ visitable: Visitable?) -> Bool {
+        (visitable?.visitableViewController as? VisitableViewController)?.appearReason == .revealedByPop
     }
 
-    private func storeRestorationIdentifier(_ restorationIdentifier: String, forVisitable visitable: Visitable) {
-        visitableRestorationIdentifiers.setObject(restorationIdentifier as NSString, forKey: visitable.visitableViewController)
+    private func restorationState(for visitable: Visitable) -> RestorationState {
+        let viewController = visitable.visitableViewController
+
+        if let existingState = visitableRestorationStates.object(forKey: viewController) {
+            return existingState
+        }
+
+        let newState = RestorationState()
+        visitableRestorationStates.setObject(newState, forKey: viewController)
+        return newState
     }
 
     // MARK: - Navigation
@@ -155,6 +173,12 @@ public class Session: NSObject {
 
         topmostVisit = visit
     }
+
+    private func restoreScrollPosition(for visitable: Visitable) {
+        if visitable === activatedVisitable, let position = restorationState(for: visitable).scrollPosition {
+            webView.scrollView.contentOffset = position
+        }
+    }
 }
 
 extension Session: VisitDelegate {
@@ -195,14 +219,16 @@ extension Session: VisitDelegate {
     }
 
     func visitDidRender(_ visit: Visit) {
-        visit.visitable.hideVisitableScreenshot()
+        if !isRevealedByPop(visit.visitable) {
+            visit.visitable.hideVisitableScreenshot()
+            visit.visitable.visitableDidRender()
+        }
         visit.visitable.hideVisitableActivityIndicator()
-        visit.visitable.visitableDidRender()
     }
 
     func visitDidComplete(_ visit: Visit) {
         guard let restorationIdentifier = visit.restorationIdentifier else { return }
-        storeRestorationIdentifier(restorationIdentifier, forVisitable: visit.visitable)
+        restorationState(for: visit.visitable).identifier = restorationIdentifier
     }
 
     func visitDidFail(_ visit: Visit) {
@@ -282,13 +308,27 @@ extension Session: VisitableDelegate {
             return
         }
 
-        // Navigating backward from a native to a web view screen.
         if visitable === previousVisit?.visitable {
             visit(visitable, action: .restore)
         }
     }
 
     public func visitableViewDidAppear(_ visitable: Visitable) {
+        if isRevealedByPop(visitable) {
+            // Appearing after back navigation
+            if let coordinator = visitable.visitableViewController.transitionCoordinator {
+                coordinator.animate(alongsideTransition: nil) { [weak self] _ in
+                    self?.restoreScrollPosition(for: visitable)
+                    visitable.hideVisitableScreenshot()
+                    visitable.visitableDidRender()
+                }
+            } else {
+                restoreScrollPosition(for: visitable)
+                visitable.hideVisitableScreenshot()
+                visitable.visitableDidRender()
+            }
+        }
+
         if let currentVisit = currentVisit, visitable === currentVisit.visitable {
             // Appearing after successful navigation
             completeNavigationForCurrentVisit()
@@ -303,6 +343,8 @@ extension Session: VisitableDelegate {
 
     public func visitableViewWillDisappear(_ visitable: Visitable) {
         previousVisit = topmostVisit
+        visitable.updateVisitableScreenshot()
+        visitable.showVisitableScreenshot()
     }
 
     public func visitableViewDidDisappear(_ visitable: Visitable) {

Tests/Turbo/ScrollRestorationTests.swift

@@ -0,0 +1,88 @@
+import WebKit
+import XCTest
+
+@testable import HotwireNative
+
+class ScrollRestorationTests: XCTestCase {
+    private var session: Session!
+    private var webView: WKWebView!
+
+    @MainActor
+    override func setUp() async throws {
+        session = Session()
+        webView = session.webView
+    }
+
+    override func tearDown() {
+        session = nil
+        webView = nil
+    }
+
+    @MainActor
+    func test_deactivateWebView_preservesScrollPosition() async throws {
+        let visitable1 = VisitableViewController(url: URL(string: "http://example.com/page1")!)
+        let visitable2 = VisitableViewController(url: URL(string: "http://example.com/page2")!)
+
+        activate(visitable1)
+        webView.scrollView.contentOffset = CGPoint(x: 0, y: 500)
+
+        deactivate(visitable1)
+        activate(visitable2)
+
+        visitable1.appearReason = .revealedByPop
+        deactivate(visitable2)
+        activate(visitable1)
+
+        XCTAssertEqual(webView.scrollView.contentOffset.y, 500, accuracy: 1.0)
+    }
+
+    @MainActor
+    func test_activateWebView_startsAtTop() async throws {
+        let visitable1 = VisitableViewController(url: URL(string: "http://example.com/page1")!)
+        let visitable2 = VisitableViewController(url: URL(string: "http://example.com/page2")!)
+
+        activate(visitable1)
+        webView.scrollView.contentOffset = CGPoint(x: 0, y: 300)
+        webView.scrollView.contentInset = UIEdgeInsets(top: 0, left: 0, bottom: 350, right: 0)
+
+        deactivate(visitable1)
+        activate(visitable2)
+
+        XCTAssertEqual(webView.scrollView.contentOffset.y, 0, accuracy: 1.0)
+    }
+
+    @MainActor
+    func test_scrollRestoration_unaffectedByKeyboardOnSubsequentPage() async throws {
+        let visitable1 = VisitableViewController(url: URL(string: "http://example.com/page1")!)
+        let visitable2 = VisitableViewController(url: URL(string: "http://example.com/page2")!)
+
+        activate(visitable1)
+        webView.scrollView.contentOffset = CGPoint(x: 0, y: 500)
+
+        deactivate(visitable1)
+        activate(visitable2)
+
+        // Simulate keyboard appearance on page 2
+        webView.scrollView.contentInset = UIEdgeInsets(top: 0, left: 0, bottom: 350, right: 0)
+        webView.scrollView.contentOffset = CGPoint(x: 0, y: 100)
+
+        visitable1.appearReason = .revealedByPop
+        deactivate(visitable2)
+        activate(visitable1)
+
+        XCTAssertEqual(webView.scrollView.contentOffset.y, 500, accuracy: 10.0)
+    }
+
+    // MARK: - Helpers
+
+    @MainActor
+    private func activate(_ visitable: VisitableViewController) {
+        visitable.visitableDelegate = session
+        session.activateVisitable(visitable)
+    }
+
+    @MainActor
+    private func deactivate(_ visitable: VisitableViewController) {
+        session.deactivateActivatedVisitable()
+    }
+}