Merge pull request #1965 from manaflow-ai/task-scrollbar-fix-mainline
Preserve explicit wheel scrollback against passive follow
This commit is contained in:
commit
da1bfedb87
2 changed files with 120 additions and 11 deletions
|
|
@ -6058,6 +6058,7 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
|
|||
}
|
||||
|
||||
override func scrollWheel(with event: NSEvent) {
|
||||
NotificationCenter.default.post(name: .ghosttyDidReceiveWheelScroll, object: self)
|
||||
guard let surface = surface else { return }
|
||||
lastScrollEventTime = CACurrentMediaTime()
|
||||
Self.focusLog("scrollWheel: surface=\(terminalSurface?.id.uuidString ?? "nil") firstResponder=\(String(describing: window?.firstResponder))")
|
||||
|
|
@ -6459,6 +6460,7 @@ enum GhosttyNotificationKey {
|
|||
extension Notification.Name {
|
||||
static let ghosttyDidUpdateScrollbar = Notification.Name("ghosttyDidUpdateScrollbar")
|
||||
static let ghosttyDidUpdateCellSize = Notification.Name("ghosttyDidUpdateCellSize")
|
||||
static let ghosttyDidReceiveWheelScroll = Notification.Name("ghosttyDidReceiveWheelScroll")
|
||||
static let ghosttySearchFocus = Notification.Name("ghosttySearchFocus")
|
||||
static let ghosttyConfigDidReload = Notification.Name("ghosttyConfigDidReload")
|
||||
static let ghosttyDefaultBackgroundDidChange = Notification.Name("ghosttyDefaultBackgroundDidChange")
|
||||
|
|
@ -6592,6 +6594,8 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
/// When true, auto-scroll should be suspended to prevent the "doomscroll" bug
|
||||
/// where the terminal fights the user's scroll position.
|
||||
private var userScrolledAwayFromBottom = false
|
||||
private var pendingExplicitWheelScroll = false
|
||||
private var allowExplicitScrollbarSync = false
|
||||
/// Threshold in points from bottom to consider "at bottom" (allows for minor float drift)
|
||||
private static let scrollToBottomThreshold: CGFloat = 5.0
|
||||
private var isActive = true
|
||||
|
|
@ -7017,6 +7021,14 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
self?.handleScrollbarUpdate(notification)
|
||||
})
|
||||
|
||||
observers.append(NotificationCenter.default.addObserver(
|
||||
forName: .ghosttyDidReceiveWheelScroll,
|
||||
object: surfaceView,
|
||||
queue: .main
|
||||
) { [weak self] _ in
|
||||
self?.pendingExplicitWheelScroll = true
|
||||
})
|
||||
|
||||
observers.append(NotificationCenter.default.addObserver(
|
||||
forName: .ghosttySearchFocus,
|
||||
object: nil,
|
||||
|
|
@ -9034,19 +9046,12 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
userScrolledAwayFromBottom = false
|
||||
}
|
||||
|
||||
// Only auto-scroll if user hasn't manually scrolled away from bottom
|
||||
// or if we're following terminal output (scrollbar shows we're at bottom)
|
||||
let shouldAutoScroll = !userScrolledAwayFromBottom ||
|
||||
(scrollbar.offset + scrollbar.len >= scrollbar.total)
|
||||
// Passive bottom packets should not override an explicit scrollback review,
|
||||
// but the first scrollbar packet caused by the user's own wheel input should
|
||||
// still move the viewport to the requested scrollback position.
|
||||
let shouldAutoScroll = !userScrolledAwayFromBottom || allowExplicitScrollbarSync
|
||||
|
||||
if shouldAutoScroll && !pointApproximatelyEqual(currentOrigin, targetOrigin) {
|
||||
#if DEBUG
|
||||
logDragGeometryChange(
|
||||
event: "scrollOrigin",
|
||||
old: currentOrigin,
|
||||
new: targetOrigin
|
||||
)
|
||||
#endif
|
||||
scrollView.contentView.scroll(to: targetOrigin)
|
||||
didChangeGeometry = true
|
||||
}
|
||||
|
|
@ -9054,6 +9059,8 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
}
|
||||
}
|
||||
|
||||
allowExplicitScrollbarSync = false
|
||||
|
||||
if didChangeGeometry {
|
||||
scrollView.reflectScrolledClipView(scrollView.contentView)
|
||||
}
|
||||
|
|
@ -9089,6 +9096,11 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
guard let scrollbar = notification.userInfo?[GhosttyNotificationKey.scrollbar] as? GhosttyScrollbar else {
|
||||
return
|
||||
}
|
||||
if pendingExplicitWheelScroll {
|
||||
userScrolledAwayFromBottom = scrollbar.offset + scrollbar.len < scrollbar.total
|
||||
allowExplicitScrollbarSync = true
|
||||
pendingExplicitWheelScroll = false
|
||||
}
|
||||
surfaceView.scrollbar = scrollbar
|
||||
synchronizeScrollView()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1683,6 +1683,30 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
private final class ScrollbarPostingSurfaceView: GhosttyNSView {
|
||||
var nextScrollbar: GhosttyScrollbar?
|
||||
|
||||
override func scrollWheel(with event: NSEvent) {
|
||||
super.scrollWheel(with: event)
|
||||
guard let nextScrollbar else { return }
|
||||
NotificationCenter.default.post(
|
||||
name: .ghosttyDidUpdateScrollbar,
|
||||
object: self,
|
||||
userInfo: [GhosttyNotificationKey.scrollbar: nextScrollbar]
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private func makeScrollbar(total: UInt64, offset: UInt64, len: UInt64) -> GhosttyScrollbar {
|
||||
GhosttyScrollbar(
|
||||
c: ghostty_action_scrollbar_s(
|
||||
total: total,
|
||||
offset: offset,
|
||||
len: len
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
private func findEditableTextField(in view: NSView) -> NSTextField? {
|
||||
if let field = view as? NSTextField, field.isEditable {
|
||||
return field
|
||||
|
|
@ -1768,6 +1792,79 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
|
|||
)
|
||||
}
|
||||
|
||||
func testExplicitWheelScrollKeepsScrollbackPinnedAgainstLaterBottomPacket() {
|
||||
let window = NSWindow(
|
||||
contentRect: NSRect(x: 0, y: 0, width: 360, height: 240),
|
||||
styleMask: [.titled, .closable],
|
||||
backing: .buffered,
|
||||
defer: false
|
||||
)
|
||||
defer { window.orderOut(nil) }
|
||||
|
||||
guard let contentView = window.contentView else {
|
||||
XCTFail("Expected content view")
|
||||
return
|
||||
}
|
||||
|
||||
let surfaceView = ScrollbarPostingSurfaceView(frame: NSRect(x: 0, y: 0, width: 160, height: 120))
|
||||
surfaceView.cellSize = CGSize(width: 10, height: 10)
|
||||
let hostedView = GhosttySurfaceScrollView(surfaceView: surfaceView)
|
||||
hostedView.frame = contentView.bounds
|
||||
hostedView.autoresizingMask = [.width, .height]
|
||||
contentView.addSubview(hostedView)
|
||||
|
||||
window.makeKeyAndOrderFront(nil)
|
||||
window.displayIfNeeded()
|
||||
contentView.layoutSubtreeIfNeeded()
|
||||
hostedView.layoutSubtreeIfNeeded()
|
||||
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
|
||||
|
||||
guard let scrollView = hostedView.subviews.first(where: { $0 is NSScrollView }) as? NSScrollView else {
|
||||
XCTFail("Expected hosted terminal scroll view")
|
||||
return
|
||||
}
|
||||
|
||||
NotificationCenter.default.post(
|
||||
name: .ghosttyDidUpdateScrollbar,
|
||||
object: surfaceView,
|
||||
userInfo: [GhosttyNotificationKey.scrollbar: makeScrollbar(total: 100, offset: 90, len: 10)]
|
||||
)
|
||||
RunLoop.current.run(until: Date().addingTimeInterval(0.01))
|
||||
XCTAssertEqual(scrollView.contentView.bounds.origin.y, 0, accuracy: 0.01)
|
||||
|
||||
surfaceView.nextScrollbar = makeScrollbar(total: 100, offset: 40, len: 10)
|
||||
|
||||
guard let cgEvent = CGEvent(
|
||||
scrollWheelEvent2Source: nil,
|
||||
units: .pixel,
|
||||
wheelCount: 2,
|
||||
wheel1: 0,
|
||||
wheel2: -12,
|
||||
wheel3: 0
|
||||
), let scrollEvent = NSEvent(cgEvent: cgEvent) else {
|
||||
XCTFail("Expected scroll wheel event")
|
||||
return
|
||||
}
|
||||
|
||||
scrollView.scrollWheel(with: scrollEvent)
|
||||
RunLoop.current.run(until: Date().addingTimeInterval(0.01))
|
||||
XCTAssertEqual(scrollView.contentView.bounds.origin.y, 500, accuracy: 0.01)
|
||||
|
||||
NotificationCenter.default.post(
|
||||
name: .ghosttyDidUpdateScrollbar,
|
||||
object: surfaceView,
|
||||
userInfo: [GhosttyNotificationKey.scrollbar: makeScrollbar(total: 100, offset: 90, len: 10)]
|
||||
)
|
||||
RunLoop.current.run(until: Date().addingTimeInterval(0.01))
|
||||
|
||||
XCTAssertEqual(
|
||||
scrollView.contentView.bounds.origin.y,
|
||||
500,
|
||||
accuracy: 0.01,
|
||||
"A passive bottom packet should not yank the viewport after an explicit wheel scroll into scrollback"
|
||||
)
|
||||
}
|
||||
|
||||
func testInactiveOverlayVisibilityTracksRequestedState() {
|
||||
let hostedView = GhosttySurfaceScrollView(
|
||||
surfaceView: GhosttyNSView(frame: NSRect(x: 0, y: 0, width: 80, height: 50))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue