macOS: Remove hidden titlebar safe area and pad document height in SurfaceScrollView (#9241)
Fixes #9233. The issue was that the content view was effectively truncated by `SurfaceScrollView`s safe area insets covering the titlebar (I suppose the scroll view doesn't inherit the `ignoresSafeArea` modifier on `TerminalSplitTreeView` because it's not actually a node in that tree, it just contains a subview that is). I think it's OK to zero these insets unconditionally, as I don't think the frame size passed down from `SurfaceWrapper` will ever overlap with a visible titlebar. This PR also fixes a somewhat related issue I discovered along the way. In many cases, the mouse wouldn't work on the first row of text, whether trying to select text or interact with TUI elements like neovim bufferlines/tablines. When looking at the inspector while moving the mouse around, you'd see the mouse position jump discontinuously from a y-coordinate of 10-20 pixels to -3, i.e., no longer within the surface. The problem turned out to be that the height of the document view only included the text lines, not the window padding, while the content view is sized to the surface including padding. If you imagine the metaphorical sliding of the viewport up and down the document, and you require the text to snap to a fixed grid on the viewport, it's clear that the document must have the same top and bottom padding as the viewport, otherwise the math breaks at the ends. Sure enough, adding this padding fixed the problem. To properly reproduce these issues and see the effect of the fixes, it's helpful to change the system settings to always show scroll bars (Appearance -> Show scroll bars -> Always). That's what connecting an external mouse was all about. You should also remove the debug banner or use a release build, otherwise the banner takes the place of the hidden titlebar and conceals the issue.pull/9252/head
commit
a2e95ce7b5
|
|
@ -118,7 +118,12 @@ class SurfaceScrollView: NSView {
|
||||||
deinit {
|
deinit {
|
||||||
observers.forEach { NotificationCenter.default.removeObserver($0) }
|
observers.forEach { NotificationCenter.default.removeObserver($0) }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The entire bounds is a safe area, so we override any default
|
||||||
|
// insets. This is necessary for the content view to match the
|
||||||
|
// surface view if we have the "hidden" titlebar style.
|
||||||
|
override var safeAreaInsets: NSEdgeInsets { return NSEdgeInsetsZero }
|
||||||
|
|
||||||
override func setFrameSize(_ newSize: NSSize) {
|
override func setFrameSize(_ newSize: NSSize) {
|
||||||
super.setFrameSize(newSize)
|
super.setFrameSize(newSize)
|
||||||
|
|
||||||
|
|
@ -221,11 +226,17 @@ class SurfaceScrollView: NSView {
|
||||||
// Convert row units to pixels using cell height, ignore zero height.
|
// Convert row units to pixels using cell height, ignore zero height.
|
||||||
let cellHeight = surfaceView.cellSize.height
|
let cellHeight = surfaceView.cellSize.height
|
||||||
guard cellHeight > 0 else { return }
|
guard cellHeight > 0 else { return }
|
||||||
|
|
||||||
|
// The full document height must include the vertical padding around the cell
|
||||||
|
// grid, otherwise the content view ends up misaligned with the surface.
|
||||||
|
let documentGridHeight = CGFloat(scrollbar.total) * cellHeight
|
||||||
|
let gridHeight = CGFloat(scrollbar.len) * cellHeight
|
||||||
|
let padding = scrollView.contentSize.height - gridHeight
|
||||||
|
let documentHeight = documentGridHeight + padding
|
||||||
|
|
||||||
// Our width should be the content width to account for visible scrollers.
|
// Our width should be the content width to account for visible scrollers.
|
||||||
// We don't do horizontal scrolling in terminals.
|
// We don't do horizontal scrolling in terminals.
|
||||||
let totalHeight = CGFloat(scrollbar.total) * cellHeight
|
let newSize = CGSize(width: scrollView.contentSize.width, height: documentHeight)
|
||||||
let newSize = CGSize(width: scrollView.contentSize.width, height: totalHeight)
|
|
||||||
documentView.setFrameSize(newSize)
|
documentView.setFrameSize(newSize)
|
||||||
|
|
||||||
// Only update our actual scroll position if we're not actively scrolling.
|
// Only update our actual scroll position if we're not actively scrolling.
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue