Avoid scrolling whole selections to keep undo responsive

Sources/STTextViewAppKit/STTextView+Scrolling.swift

@@ -6,27 +6,42 @@ import AppKit
 
 extension STTextView {
 
+    private func adjustedScrollRect(_ rect: CGRect) -> CGRect {
+        var adjustedRect = rect
+
+        if adjustedRect.width.isZero {
+            // add padding around the point to ensure the visibility the segment
+            // since the width of the segment is 0 for a selection
+            adjustedRect = adjustedRect.inset(by: .init(top: 0, left: -textContainer.lineFragmentPadding, bottom: 0, right: -textContainer.lineFragmentPadding))
+        }
+
+        // scroll to visible IN clip view (ignoring gutter view overlay)
+        // adjust rect to mimick it's size to include gutter overlay
+        adjustedRect.origin.x -= gutterView?.frame.width ?? 0
+        adjustedRect.size.width += gutterView?.frame.width ?? 0
+        return adjustedRect
+    }
+
     override open func scroll(_ point: NSPoint) {
         contentView.scroll(point.applying(.init(translationX: -(gutterView?.frame.width ?? 0), y: 0)))
     }
 
     @discardableResult
     func scrollToVisible(_ textRange: NSTextRange, type: NSTextLayoutManager.SegmentType) -> Bool {
-        guard var rect = textLayoutManager.textSegmentFrame(in: textRange, type: type) else {
+        guard let rect = textLayoutManager.textSegmentFrame(in: textRange, type: type) else {
             return false
         }
 
-        if rect.width.isZero {
-            // add padding around the point to ensure the visibility the segment
-            // since the width of the segment is 0 for a selection
-            rect = rect.inset(by: .init(top: 0, left: -textContainer.lineFragmentPadding, bottom: 0, right: -textContainer.lineFragmentPadding))
+        return contentView.scrollToVisible(adjustedScrollRect(rect))
+    }
+
+    @discardableResult
+    func scrollToVisible(_ textLocation: NSTextLocation, type: NSTextLayoutManager.SegmentType) -> Bool {
+        guard let rect = textLayoutManager.textSegmentFrame(at: textLocation, type: type) else {
+            return false
         }
 
-        // scroll to visible IN clip view (ignoring gutter view overlay)
-        // adjust rect to mimick it's size to include gutter overlay
-        rect.origin.x -= gutterView?.frame.width ?? 0
-        rect.size.width += gutterView?.frame.width ?? 0
-        return contentView.scrollToVisible(rect)
+        return contentView.scrollToVisible(adjustedScrollRect(rect))
     }
 
     override open func centerSelectionInVisibleArea(_ sender: Any?) {

Sources/STTextViewAppKit/STTextView.swift

@@ -1339,8 +1339,10 @@ open class STTextView: NSView, NSTextInput, NSTextContent, STTextViewProtocol {
         super.layout()
         layoutText()
 
-        if needsScrollToSelection, let textRange = textLayoutManager.textSelections.last?.textRanges.last {
-            scrollToVisible(textRange, type: .standard)
+        if needsScrollToSelection,
+           let textRange = textLayoutManager.textSelections.last?.textRanges.last,
+           let textLocation = textLocationForScrollingSelection(toVisible: textRange) {
+            scrollToVisible(textLocation, type: .standard)
         }
 
         needsScrollToSelection = false
@@ -1384,6 +1386,30 @@ open class STTextView: NSView, NSTextInput, NSTextContent, STTextViewProtocol {
         }
     }
 
+    func textLocationForScrollingSelection(toVisible textRange: NSTextRange) -> NSTextLocation? {
+        guard !textRange.isEmpty else {
+            return textRange.location
+        }
+
+        guard let viewportRange = textLayoutManager.textViewportLayoutController.viewportRange else {
+            return textRange.location
+        }
+
+        if textRange.intersects(viewportRange) {
+            return nil
+        }
+
+        let selectionEndsBeforeViewport = textContentManager.offset(
+            from: textRange.endLocation,
+            to: viewportRange.location
+        ) > 0
+        if selectionEndsBeforeViewport {
+            return textRange.endLocation
+        }
+
+        return textRange.location
+    }
+
     private var effectiveVisibleRect: CGRect {
         visibleRect.isInfinite ? bounds : visibleRect
     }

Tests/STTextViewAppKitTests/UndoTests.swift

@@ -1,7 +1,9 @@
 #if os(macOS)
+    import AppKit
     import XCTest
     @testable import STTextViewAppKit
 
+    @MainActor
     final class UndoTests: XCTestCase {
         func testInsertingAtEndAndUndo() {
             let textView = STTextView()
@@ -80,5 +82,123 @@
             textView.redo(nil)
             XCTAssertEqual(textView.text!, "123a789")
         }
+
+        func testSelectionScrollLocationSkipsSelectionsAlreadyInViewport() {
+            let harness = ScrollViewHarness()
+            let textView = harness.textView
+
+            textView.isHorizontallyResizable = false
+            textView.isVerticallyResizable = true
+            textView.text = Array(repeating: "alpha beta gamma delta epsilon zeta eta theta iota kappa", count: 200).joined(separator: "\n")
+
+            harness.flushLayout()
+
+            XCTAssertNil(textView.textLocationForScrollingSelection(toVisible: textView.textLayoutManager.documentRange))
+        }
+
+        func testSelectionScrollLocationUsesNearestSelectionEdgeOutsideViewport() throws {
+            let harness = ScrollViewHarness()
+            let textView = harness.textView
+
+            textView.isHorizontallyResizable = false
+            textView.isVerticallyResizable = true
+            textView.text = Array(repeating: "alpha beta gamma delta epsilon zeta eta theta iota kappa", count: 200).joined(separator: "\n")
+
+            harness.flushLayout()
+
+            guard let initialViewportRange = textView.textLayoutManager.textViewportLayoutController.viewportRange else {
+                return XCTFail("Missing initial viewport range")
+            }
+
+            let documentStart = textView.textLayoutManager.documentRange.location
+            let initialViewportEndOffset = textView.textContentManager.offset(from: documentStart, to: initialViewportRange.endLocation)
+            let afterRange = try XCTUnwrap(
+                NSTextRange(
+                    NSRange(location: min(initialViewportEndOffset + 1, textView.text!.utf16.count - 1), length: 1),
+                    in: textView.textContentManager
+                )
+            )
+            let afterLocation = try XCTUnwrap(textView.textLocationForScrollingSelection(toVisible: afterRange))
+            XCTAssertEqual(
+                textView.textContentManager.offset(from: documentStart, to: afterLocation),
+                NSRange(afterRange, in: textView.textContentManager).location
+            )
+
+            harness.scrollToBottom()
+
+            guard let viewportRange = textView.textLayoutManager.textViewportLayoutController.viewportRange else {
+                return XCTFail("Missing viewport range")
+            }
+
+            let viewportStartOffset = textView.textContentManager.offset(from: documentStart, to: viewportRange.location)
+            let beforeRange = try XCTUnwrap(
+                NSTextRange(
+                    NSRange(location: 0, length: max(1, viewportStartOffset - 1)),
+                    in: textView.textContentManager
+                )
+            )
+            let beforeLocation = try XCTUnwrap(textView.textLocationForScrollingSelection(toVisible: beforeRange))
+
+            XCTAssertEqual(
+                textView.textContentManager.offset(from: documentStart, to: beforeLocation),
+                NSMaxRange(NSRange(beforeRange, in: textView.textContentManager))
+            )
+        }
+    }
+
+    @MainActor
+    private final class ScrollViewHarness {
+        let window: NSWindow
+        let scrollView: NSScrollView
+        let textView: STTextView
+
+        init() {
+            let scrollView = STTextView.scrollableTextView()
+            self.window = NSWindow(
+                contentRect: NSRect(x: 0, y: 0, width: 480, height: 320),
+                styleMask: [.titled, .closable, .resizable],
+                backing: .buffered,
+                defer: false
+            )
+            self.scrollView = scrollView
+            self.textView = scrollView.documentView as! STTextView
+
+            guard let contentView = window.contentView else {
+                fatalError("Missing window content view")
+            }
+
+            scrollView.translatesAutoresizingMaskIntoConstraints = false
+            contentView.addSubview(scrollView)
+            NSLayoutConstraint.activate([
+                scrollView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor),
+                scrollView.trailingAnchor.constraint(equalTo: contentView.trailingAnchor),
+                scrollView.topAnchor.constraint(equalTo: contentView.topAnchor),
+                scrollView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor)
+            ])
+
+            window.makeKeyAndOrderFront(nil)
+        }
+
+        func flushLayout() {
+            window.contentView?.layoutSubtreeIfNeeded()
+            textView.layoutSubtreeIfNeeded()
+
+            RunLoop.current.run(until: Date().addingTimeInterval(0.01))
+
+            window.contentView?.layoutSubtreeIfNeeded()
+            textView.layoutSubtreeIfNeeded()
+        }
+
+        func scrollToBottom() {
+            let documentHeight = textView.frame.height
+            let visibleHeight = scrollView.contentView.bounds.height
+            guard documentHeight > visibleHeight else {
+                return
+            }
+
+            scrollView.contentView.scroll(to: CGPoint(x: 0, y: documentHeight - visibleHeight))
+            scrollView.reflectScrolledClipView(scrollView.contentView)
+            flushLayout()
+        }
     }
 #endif