fix(datagrid): overlay the inline cell editor exactly on the drawn cell

Author: shreeve

CHANGELOG.md

@@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Inline cell editor opening taller than the row and shifting a single-line value instead of overlaying it.
 - Parse error on any MongoDB filter written in shell syntax, such as `db.orders.find({status: 1})`.
 - MongoDB `.sort()` and `.projection()` silently ignored when written with unquoted keys.
 - Compare & Sync unable to drop an overloaded PostgreSQL routine, or any trigger.

TablePro/Views/Results/CellOverlayBase.swift

@@ -86,22 +86,56 @@ class CellOverlayBase: NSObject {
         onRemove?()
     }
 
+    static let maximumOverlayHeight: CGFloat = 120
+
+    /// A single-line value gets exactly the cell it is editing, which is what keeps the
+    /// glyphs from moving when the overlay opens. Only a value that actually breaks into
+    /// lines grows, and its height budget comes from the same geometry the text view is
+    /// configured with: `textContainerInset` is symmetric, so the content pays the top
+    /// inset twice.
     static func overlayFrame(for cellFrame: NSRect, value: String) -> NSRect {
-        let lineHeight = ThemeEngine.shared.dataGridFonts.regular.boundingRectForFont.height + 4
-        var newlineCount = 0
-        for scalar in value.unicodeScalars where scalar == "\n" {
-            newlineCount += 1
-        }
-        let lineCount = CGFloat(newlineCount + 1)
-        let contentHeight = max(lineCount * lineHeight + 8, cellFrame.height)
-        let height = min(max(contentHeight, cellFrame.height), 120)
+        let breaks = lineBreakCount(in: value)
+        guard breaks > 0 else { return cellFrame }
+
+        let font = ThemeEngine.shared.valueFont
+        let inset = DataGridCellTextGeometry.textContainerTopInset(
+            rowHeight: cellFrame.height, font: font
+        )
+        let lineCount = CGFloat(breaks + 1)
+        let contentHeight = lineCount * DataGridCellTextGeometry.lineHeight(for: font) + 2 * inset
+        let height = min(max(contentHeight, cellFrame.height), maximumOverlayHeight)
         return NSRect(x: cellFrame.origin.x, y: cellFrame.origin.y, width: cellFrame.width, height: height)
     }
 
+    /// Counts the breaks TextKit lays out, not just LF: a lone CR, NEL, or a Unicode line or
+    /// paragraph separator each start a new line fragment, and CRLF is one break. Counting
+    /// only "\n" classified a "line1\rline2" value as single-line, which sized the overlay
+    /// to one row and hid the second line behind it.
+    static func lineBreakCount(in value: String) -> Int {
+        var count = 0
+        var previousWasCarriageReturn = false
+        for scalar in value.unicodeScalars {
+            switch scalar.value {
+            case 0x0A:
+                if !previousWasCarriageReturn { count += 1 }
+                previousWasCarriageReturn = false
+            case 0x0D:
+                count += 1
+                previousWasCarriageReturn = true
+            case 0x85, 0x2028, 0x2029:
+                count += 1
+                previousWasCarriageReturn = false
+            default:
+                previousWasCarriageReturn = false
+            }
+        }
+        return count
+    }
+
     static func makeContainer(frame: NSRect) -> CellOverlayContainerView {
         let container = CellOverlayContainerView(frame: frame)
         container.wantsLayer = true
-        container.layer?.borderWidth = 2
+        container.layer?.borderWidth = 1
         container.layer?.cornerRadius = 2
         container.layer?.masksToBounds = true
         container.applyLayerColors()
@@ -130,18 +164,32 @@ class CellOverlayBase: NSObject {
         textView.textContainer?.containerSize = unbounded
     }
 
-    static func makeScrollView(in container: NSView) -> NSScrollView {
+    /// A row-height overlay holding a font taller than the row would otherwise show a
+    /// vertical scroller and scroll its own descenders; a single-line value has nothing to
+    /// scroll to, so the vertical axis is shut off entirely.
+    static func makeScrollView(in container: NSView, scrollsVertically: Bool) -> NSScrollView {
         let scrollView = NSScrollView(frame: container.bounds)
         scrollView.autoresizingMask = [.width, .height]
-        scrollView.hasVerticalScroller = true
+        scrollView.hasVerticalScroller = scrollsVertically
         scrollView.hasHorizontalScroller = false
         scrollView.autohidesScrollers = true
         scrollView.borderType = .noBorder
         scrollView.drawsBackground = true
         scrollView.backgroundColor = .textBackgroundColor
+        if !scrollsVertically {
+            scrollView.verticalScrollElasticity = .none
+        }
         return scrollView
     }
 
+    static func configureCellTextGeometry(of textView: NSTextView, rowHeight: CGFloat, font: NSFont) {
+        textView.textContainer?.lineFragmentPadding = DataGridMetrics.cellHorizontalInset
+        textView.textContainerInset = NSSize(
+            width: 0,
+            height: DataGridCellTextGeometry.textContainerTopInset(rowHeight: rowHeight, font: font)
+        )
+    }
+
     private func installDismissObservers() {
         guard let hostTableView else { return }
 

TablePro/Views/Results/CellOverlayEditor.swift

@@ -8,6 +8,8 @@ import AppKit
 @MainActor
 final class CellOverlayEditor: CellOverlayBase, NSTextViewDelegate {
     private var editorTextView: OverlayTextView?
+    private var editorScrollView: NSScrollView?
+    private var editedCellFrame: NSRect = .zero
     private var initialValue: String = ""
 
     var onCommit: ((_ row: Int, _ columnIndex: Int, _ newValue: String) -> Void)?
@@ -27,19 +29,23 @@ final class CellOverlayEditor: CellOverlayBase, NSTextViewDelegate {
         guard let window = tableView.window else { return }
 
         let frame = Self.overlayFrame(for: cellFrame, value: value)
+        let font = ThemeEngine.shared.valueFont
         let containerView = Self.makeContainer(frame: frame)
-        let scrollView = Self.makeScrollView(in: containerView)
+        let scrollView = Self.makeScrollView(
+            in: containerView, scrollsVertically: frame.height > cellFrame.height
+        )
 
         let textView = OverlayTextView(frame: scrollView.bounds)
         textView.overlayEditor = self
         textView.isEditable = true
         textView.isRichText = false
         textView.allowsUndo = true
-        textView.font = ThemeEngine.shared.valueFont
+        textView.font = font
         textView.textColor = .labelColor
         textView.backgroundColor = .textBackgroundColor
         textView.focusRingType = .none
         Self.applyCellTextLayout(to: textView)
+        Self.configureCellTextGeometry(of: textView, rowHeight: cellFrame.height, font: font)
         textView.delegate = self
         textView.string = value
         textView.selectAll(nil)
@@ -49,6 +55,8 @@ final class CellOverlayEditor: CellOverlayBase, NSTextViewDelegate {
 
         initialValue = value
         editorTextView = textView
+        editorScrollView = scrollView
+        editedCellFrame = cellFrame
 
         install(in: tableView, row: row, column: column, columnIndex: columnIndex, container: containerView)
         window.makeFirstResponder(textView)
@@ -66,6 +74,8 @@ final class CellOverlayEditor: CellOverlayBase, NSTextViewDelegate {
         let dismissColumnIndex = columnIndex
 
         editorTextView = nil
+        editorScrollView = nil
+        editedCellFrame = .zero
         initialValue = ""
         removeOverlay()
 
@@ -74,6 +84,20 @@ final class CellOverlayEditor: CellOverlayBase, NSTextViewDelegate {
         }
     }
 
+    /// Option+Return and pasted text can turn a single-line edit into a multiline one after
+    /// the overlay opened, and the row-height overlay would clip the new lines with no
+    /// affordance that they exist. The frame follows the text, exactly as it would have been
+    /// framed had the value arrived that way.
+    func textDidChange(_ notification: Notification) {
+        guard let textView = editorTextView, let container = containerView else { return }
+        let frame = Self.overlayFrame(for: editedCellFrame, value: textView.string)
+        guard frame != container.frame else { return }
+        container.frame = frame
+        let grew = frame.height > editedCellFrame.height
+        editorScrollView?.hasVerticalScroller = grew
+        editorScrollView?.verticalScrollElasticity = grew ? .automatic : .none
+    }
+
     func textView(_ textView: NSTextView, doCommandBy commandSelector: Selector) -> Bool {
         if commandSelector == #selector(NSResponder.insertNewline(_:)) {
             if NSApp.currentEvent?.modifierFlags.contains(.option) == true {

TablePro/Views/Results/CellOverlayViewer.swift

@@ -21,17 +21,21 @@ final class CellOverlayViewer: CellOverlayBase, NSTextViewDelegate {
         guard let window = tableView.window else { return }
 
         let frame = Self.overlayFrame(for: cellFrame, value: value)
+        let font = ThemeEngine.shared.valueFont
         let containerView = Self.makeContainer(frame: frame)
-        let scrollView = Self.makeScrollView(in: containerView)
+        let scrollView = Self.makeScrollView(
+            in: containerView, scrollsVertically: frame.height > cellFrame.height
+        )
 
         let textView = NSTextView(frame: scrollView.bounds)
         textView.isEditable = false
         textView.isSelectable = true
         textView.isRichText = false
-        textView.font = ThemeEngine.shared.valueFont
+        textView.font = font
         textView.textColor = .labelColor
         textView.backgroundColor = .textBackgroundColor
         Self.applyCellTextLayout(to: textView)
+        Self.configureCellTextGeometry(of: textView, rowHeight: cellFrame.height, font: font)
         textView.delegate = self
         textView.string = value
         textView.selectAll(nil)

TablePro/Views/Results/Cells/DataGridCellRenderer.swift

@@ -74,8 +74,9 @@ final class DataGridCellRenderer {
             ? (CTLineCreateTruncatedLine(fullLine, Double(availableWidth), .end, ellipsis) ?? ellipsis)
             : fullLine
 
-        let font = appearance.font
-        let baselineOffset = (rect.height - font.ascender + font.descender - font.leading) / 2 + font.ascender
+        let baselineOffset = DataGridCellTextGeometry.baselineY(
+            rowHeight: rect.height, font: appearance.font
+        )
 
         context.saveGState()
         context.textMatrix = CGAffineTransform(scaleX: 1, y: -1)

TablePro/Views/Results/Cells/DataGridCellTextGeometry.swift

@@ -0,0 +1,39 @@
+//
+//  DataGridCellTextGeometry.swift
+//  TablePro
+//
+//  The one owner of where a cell's glyphs sit, shared by the CoreText draw path and the
+//  overlay editor so the two cannot disagree. Before it existed each side had its own
+//  numbers and an inline edit visibly shifted the value it was editing.
+//
+
+import AppKit
+
+@MainActor
+enum DataGridCellTextGeometry {
+    /// Baseline questions go through a detached layout manager, never a text view's
+    /// `layoutManager` property: one read of that property downgrades a TextKit 2 view to
+    /// TextKit 1, which reverts the overlay's no-wrap layout fix (#2381). Measured: the
+    /// detached answer equals the TextKit 2 first-fragment glyph origin.
+    private static let baselineMeasurer = NSLayoutManager()
+
+    /// The centered baseline the renderer draws at, floored to a whole point because that
+    /// is where TextKit puts it: measured at 1x and 2x backing, TextKit floors a rendered
+    /// baseline to integral points while `CTLineDraw` honors fractions. Flooring the shared
+    /// target is what lets the editor land on the drawn glyphs exactly at every scale.
+    static func baselineY(rowHeight: CGFloat, font: NSFont) -> CGFloat {
+        ((rowHeight - font.ascender + font.descender - font.leading) / 2 + font.ascender)
+            .rounded(.down)
+    }
+
+    /// The symmetric `textContainerInset.height` that puts an overlay text view's first
+    /// baseline on `baselineY`. Negative when the font outgrows the row; AppKit accepts a
+    /// negative inset and parity holds.
+    static func textContainerTopInset(rowHeight: CGFloat, font: NSFont) -> CGFloat {
+        baselineY(rowHeight: rowHeight, font: font) - baselineMeasurer.defaultBaselineOffset(for: font)
+    }
+
+    static func lineHeight(for font: NSFont) -> CGFloat {
+        baselineMeasurer.defaultLineHeight(for: font)
+    }
+}