From 218df7ec6d8b269dc6716e597ba6534a47fe66bb Mon Sep 17 00:00:00 2001 From: Victor Babenko Date: Sat, 31 Oct 2026 19:59:00 -0700 Subject: [PATCH 1/3] fix(inspector): field resize handle tracks the drag and sizes one field Co-authored-by: Cursor --- .../FieldEditors/FieldEditorContent.swift | 13 +- .../FieldEditors/FieldResizeHandle.swift | 102 +++++++++++++++ .../FieldEditors/GeometryFieldView.swift | 14 ++- .../FieldEditors/JsonEditorView.swift | 13 +- .../FieldEditors/MultiLineEditorView.swift | 5 +- .../ResizableEditorContainer.swift | 37 ++---- .../FieldEditors/ResizableFieldMetrics.swift | 20 ++- .../RowInspector/InspectorFieldListView.swift | 30 ++++- .../RowInspector/InspectorFieldRow.swift | 2 + .../FieldExpansionPolicyTests.swift | 46 +++++++ .../RowInspector/FieldResizeHandleTests.swift | 118 ++++++++++++++++++ .../InspectorValueThemeTests.swift | 4 +- .../ResizableFieldMetricsTests.swift | 24 ++++ 13 files changed, 382 insertions(+), 46 deletions(-) create mode 100644 apple/TablePro/Views/RowInspector/FieldEditors/FieldResizeHandle.swift create mode 100644 apple/TableProTests/Views/RowInspector/FieldResizeHandleTests.swift diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/FieldEditorContent.swift b/apple/TablePro/Views/RowInspector/FieldEditors/FieldEditorContent.swift index b8094dbc0c..6a10a3afe3 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/FieldEditorContent.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/FieldEditorContent.swift @@ -10,6 +10,9 @@ import SwiftUI internal struct FieldEditorContent: View { internal let context: FieldEditorContext internal let kind: FieldEditorKind + /// The height this field was dragged to, nil until it is. Required so no caller drops it: a + /// constant here takes the drag and throws it away on release. + internal let resizedHeight: Binding internal var onSetNull: (() -> Void)? internal var onSetDefault: (() -> Void)? internal var onPopOut: ((String) -> Void)? @@ -64,7 +67,7 @@ internal struct FieldEditorContent: View { private var editor: some View { switch kind { case .json: - JsonEditorView(context: context, onPopOut: onPopOut, isExpanded: isExpanded) + JsonEditorView(context: context, onPopOut: onPopOut, isExpanded: isExpanded, resizedHeight: resizedHeight) case .phpSerialized: PhpSerializedFieldView(context: context, onPopOut: onPopOut, isExpanded: isExpanded) case .binaryValue(let format): @@ -77,6 +80,7 @@ internal struct FieldEditorContent: View { descriptor: descriptor, onPopOut: onPopOut, isExpanded: isExpanded, + resizedHeight: resizedHeight, onTabStopChange: onTabStopChange ) case .blobHex: @@ -98,7 +102,12 @@ internal struct FieldEditorContent: View { case .vector: VectorFieldEditorView(context: context, onSetNull: onSetNull, onSetDefault: onSetDefault) case .multiLine: - MultiLineEditorView(context: context, onPopOut: onPopOut, isExpanded: isExpanded) + MultiLineEditorView( + context: context, + onPopOut: onPopOut, + isExpanded: isExpanded, + resizedHeight: resizedHeight + ) case .singleLine: SingleLineEditorView(context: context) } diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/FieldResizeHandle.swift b/apple/TablePro/Views/RowInspector/FieldEditors/FieldResizeHandle.swift new file mode 100644 index 0000000000..8259ede410 --- /dev/null +++ b/apple/TablePro/Views/RowInspector/FieldEditors/FieldResizeHandle.swift @@ -0,0 +1,102 @@ +// +// FieldResizeHandle.swift +// TablePro +// + +import AppKit +import SwiftUI + +/// The grip under a resizable field, reporting how far it has been dragged downward. +/// +/// AppKit on purpose. The inspector is a SwiftUI `List`, an `NSTableView`, and the table took the +/// mouse from a `DragGesture` in its row a few points into the drag: the gesture was cancelled +/// without ending, so the field snapped back to its stored height. A view that takes `mouseDown` +/// itself receives every drag and the release, as a split view's divider does. +internal struct FieldResizeHandle: NSViewRepresentable { + let onHover: (Bool) -> Void + let onDrag: (Double) -> Void + let onEnd: (Double) -> Void + + func makeNSView(context: Context) -> FieldResizeHandleView { + let view = FieldResizeHandleView() + view.setAccessibilityElement(false) + return view + } + + func updateNSView(_ view: FieldResizeHandleView, context: Context) { + view.onHover = onHover + view.onDrag = onDrag + view.onEnd = onEnd + } +} + +internal final class FieldResizeHandleView: NSView { + var onHover: ((Bool) -> Void)? + var onDrag: ((Double) -> Void)? + var onEnd: ((Double) -> Void)? + + /// Window coordinates, which stay put while the field grows and moves this view down. + private var dragStartY: CGFloat? + private var hoverArea: NSTrackingArea? + private var isHovered = false + + override var mouseDownCanMoveWindow: Bool { false } + + override func acceptsFirstMouse(for event: NSEvent?) -> Bool { true } + + override func resetCursorRects() { + addCursorRect(bounds, cursor: .resizeUpDown) + } + + override func updateTrackingAreas() { + super.updateTrackingAreas() + if let hoverArea { removeTrackingArea(hoverArea) } + let area = NSTrackingArea( + rect: .zero, + options: [.mouseEnteredAndExited, .activeInActiveApp, .inVisibleRect], + owner: self + ) + addTrackingArea(area) + hoverArea = area + } + + override func mouseEntered(with event: NSEvent) { + isHovered = true + onHover?(true) + } + + override func mouseExited(with event: NSEvent) { + isHovered = false + onHover?(false) + } + + /// Expanding from the keyboard removes the handle under a resting pointer, and no exit follows, + /// so the grip stayed lit after collapsing. + override func viewWillMove(toWindow newWindow: NSWindow?) { + super.viewWillMove(toWindow: newWindow) + guard newWindow == nil, isHovered else { return } + isHovered = false + onHover?(false) + } + + override func mouseDown(with event: NSEvent) { + dragStartY = event.locationInWindow.y + } + + override func mouseDragged(with event: NSEvent) { + guard let delta = downwardDelta(to: event) else { return } + NSCursor.resizeUpDown.set() + onDrag?(delta) + } + + override func mouseUp(with event: NSEvent) { + guard let delta = downwardDelta(to: event) else { return } + dragStartY = nil + onEnd?(delta) + } + + private func downwardDelta(to event: NSEvent) -> Double? { + guard let dragStartY else { return nil } + return Double(dragStartY - event.locationInWindow.y) + } +} diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/GeometryFieldView.swift b/apple/TablePro/Views/RowInspector/FieldEditors/GeometryFieldView.swift index ec283a95de..5991aab102 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/GeometryFieldView.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/GeometryFieldView.swift @@ -15,6 +15,7 @@ internal struct GeometryFieldView: View { let descriptor: GeometryFieldDescriptor var onPopOut: ((String) -> Void)? var isExpanded = false + let resizedHeight: Binding /// Told whether Tab can stop here without Full Keyboard Access. A map takes no focus. var onTabStopChange: ((Bool) -> Void)? @@ -22,7 +23,7 @@ internal struct GeometryFieldView: View { @AppStorage(PreferenceKeys.rowInspectorGeometryFieldMode.name, store: AppStorageEnvironment.shared.defaults) private var storedMode = Mode.map.rawValue @AppStorage(PreferenceKeys.rowInspectorGeometryFieldHeight.name, store: AppStorageEnvironment.shared.defaults) - private var mapHeight = ResizableFieldMetrics.defaultGeometryHeight + private var startingMapHeight = ResizableFieldMetrics.defaultGeometryHeight @StateObject private var previews = GeometryFieldPreviewStore() @State private var latch = ModeLatch() @@ -169,9 +170,14 @@ internal struct GeometryFieldView: View { Group { switch descriptor.textEditor { case .multiLine: - MultiLineEditorView(context: context, onPopOut: nil, isExpanded: isExpanded) + MultiLineEditorView( + context: context, + onPopOut: nil, + isExpanded: isExpanded, + resizedHeight: resizedHeight + ) case .json: - JsonEditorView(context: context, onPopOut: nil, isExpanded: isExpanded) + JsonEditorView(context: context, onPopOut: nil, isExpanded: isExpanded, resizedHeight: resizedHeight) case .hex: BlobHexEditorView(context: context) } @@ -182,7 +188,7 @@ internal struct GeometryFieldView: View { private func mapPane(_ preview: GeometryFieldPreview?) -> some View { VStack(alignment: .leading, spacing: 2) { ResizableEditorContainer( - height: $mapHeight, + height: ResizableFieldMetrics.height(resized: resizedHeight, starting: startingMapHeight), range: ResizableFieldMetrics.geometryHeightRange, expandedHeight: isExpanded ? ResizableFieldMetrics.expandedHeight : nil ) { diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/JsonEditorView.swift b/apple/TablePro/Views/RowInspector/FieldEditors/JsonEditorView.swift index a53c05b691..0fee9317c2 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/JsonEditorView.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/JsonEditorView.swift @@ -9,15 +9,22 @@ internal struct JsonEditorView: View { let context: FieldEditorContext var onPopOut: ((String) -> Void)? var isExpanded = false + let resizedHeight: Binding @State private var model: JsonFieldEditingModel - @AppStorage(PreferenceKeys.rowInspectorJsonFieldHeight.name, store: AppStorageEnvironment.shared.defaults) private var fieldHeight = ResizableFieldMetrics + @AppStorage(PreferenceKeys.rowInspectorJsonFieldHeight.name, store: AppStorageEnvironment.shared.defaults) private var startingHeight = ResizableFieldMetrics .defaultJsonHeight - init(context: FieldEditorContext, onPopOut: ((String) -> Void)? = nil, isExpanded: Bool = false) { + init( + context: FieldEditorContext, + onPopOut: ((String) -> Void)? = nil, + isExpanded: Bool = false, + resizedHeight: Binding + ) { self.context = context self.onPopOut = onPopOut self.isExpanded = isExpanded + self.resizedHeight = resizedHeight self._model = State(wrappedValue: JsonFieldEditingModel(storedValue: context.value.wrappedValue)) } @@ -36,7 +43,7 @@ internal struct JsonEditorView: View { var body: some View { ResizableEditorContainer( - height: $fieldHeight, + height: ResizableFieldMetrics.height(resized: resizedHeight, starting: startingHeight), range: ResizableFieldMetrics.jsonHeightRange, expandedHeight: isExpanded ? ResizableFieldMetrics.expandedHeight : nil ) { diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/MultiLineEditorView.swift b/apple/TablePro/Views/RowInspector/FieldEditors/MultiLineEditorView.swift index ab3058311c..0fa20ad931 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/MultiLineEditorView.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/MultiLineEditorView.swift @@ -11,13 +11,14 @@ internal struct MultiLineEditorView: View { let context: FieldEditorContext var onPopOut: ((String) -> Void)? var isExpanded = false + let resizedHeight: Binding @AppStorage(PreferenceKeys.rowInspectorTextFieldHeight.name, store: AppStorageEnvironment.shared.defaults) - private var fieldHeight = ResizableFieldMetrics.defaultTextHeight + private var startingHeight = ResizableFieldMetrics.defaultTextHeight var body: some View { ResizableEditorContainer( - height: $fieldHeight, + height: ResizableFieldMetrics.height(resized: resizedHeight, starting: startingHeight), range: ResizableFieldMetrics.textHeightRange, expandedHeight: isExpanded ? ResizableFieldMetrics.expandedHeight : nil ) { diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/ResizableEditorContainer.swift b/apple/TablePro/Views/RowInspector/FieldEditors/ResizableEditorContainer.swift index 6438601394..22abe7b48b 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/ResizableEditorContainer.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/ResizableEditorContainer.swift @@ -16,7 +16,7 @@ internal struct ResizableEditorContainer: View { @ViewBuilder let content: () -> Content - @GestureState private var liveDelta: Double = 0 + @State private var liveDelta: Double = 0 @State private var isHandleHovered = false private var resolvedHeight: Double { @@ -43,34 +43,15 @@ internal struct ResizableEditorContainer: View { .frame(width: 26, height: 4) .opacity(isHandleHovered ? 1 : 0.5) .frame(maxWidth: .infinity, minHeight: 11) - .contentShape(Rectangle()) - .gesture( - DragGesture(minimumDistance: 1, coordinateSpace: .global) - .updating($liveDelta) { value, state, _ in - state = value.translation.height + .overlay { + FieldResizeHandle( + onHover: { isHandleHovered = $0 }, + onDrag: { liveDelta = $0 }, + onEnd: { delta in + height = ResizableFieldMetrics.resolve(base: height, delta: delta, range: range) + liveDelta = 0 } - .onEnded { value in - height = ResizableFieldMetrics.resolve( - base: height, - delta: value.translation.height, - range: range - ) - } - ) - .onHover { hovering in - guard hovering != isHandleHovered else { return } - isHandleHovered = hovering - if hovering { - NSCursor.resizeUpDown.push() - } else { - NSCursor.pop() - } - } - .onDisappear { - if isHandleHovered { - NSCursor.pop() - isHandleHovered = false - } + ) } .accessibilityHidden(true) } diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/ResizableFieldMetrics.swift b/apple/TablePro/Views/RowInspector/FieldEditors/ResizableFieldMetrics.swift index d3da9331b1..c3d7be33aa 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/ResizableFieldMetrics.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/ResizableFieldMetrics.swift @@ -4,6 +4,7 @@ // import Foundation +import SwiftUI internal enum ResizableFieldMetrics { static let jsonHeightRange: ClosedRange = 80...600 @@ -21,7 +22,24 @@ internal enum ResizableFieldMetrics { /// between this and the full-pane takeover it replaces. static let expandedHeight: Double = 320 + /// The base is clamped before the drag is added. A field's height is shared by editors with + /// different floors (a geometry map and its text pane, or a column whose values switch between + /// JSON and text), and an unclamped base below the floor left the handle dead until the drag + /// had covered the difference. static func resolve(base: Double, delta: Double, range: ClosedRange) -> Double { - min(max(base + delta, range.lowerBound), range.upperBound) + clamp(clamp(base, to: range) + delta, to: range) + } + + private static func clamp(_ value: Double, to range: ClosedRange) -> Double { + min(max(value, range.lowerBound), range.upperBound) + } + + /// A drag sizes its own field only. Writing the per-kind default instead resized every field of + /// that kind at once; the default is where a field starts until it is dragged. + static func height(resized: Binding, starting: Double) -> Binding { + Binding( + get: { resized.wrappedValue ?? starting }, + set: { resized.wrappedValue = $0 } + ) } } diff --git a/apple/TablePro/Views/RowInspector/InspectorFieldListView.swift b/apple/TablePro/Views/RowInspector/InspectorFieldListView.swift index 8328ab0a09..8edfb869ba 100644 --- a/apple/TablePro/Views/RowInspector/InspectorFieldListView.swift +++ b/apple/TablePro/Views/RowInspector/InspectorFieldListView.swift @@ -26,7 +26,9 @@ internal struct InspectorFieldListView: View { @State private var searchText = "" @State private var showsModifiedOnly = false - @State private var expandedFieldID: UUID? + @State private var expandedColumnIndex: Int? + /// By column for the same reason as the expansion: a dragged size holds while rows change. + @State private var resizedHeights: [Int: Double] = [:] @StateObject private var tabStops = InspectorTabStopRegistry() @FocusState private var focusedField: UUID? @@ -45,7 +47,8 @@ internal struct InspectorFieldListView: View { } } .onChange(of: editState.fields.map(\.columnName)) { _ in - expandedFieldID = nil + expandedColumnIndex = nil + resizedHeights = [:] focusedField = nil } } @@ -157,13 +160,18 @@ internal struct InspectorFieldListView: View { isModified: field.hasEdit || field.hasCommittedEdit, isPrimaryKey: field.isPrimaryKey, isForeignKey: field.isForeignKey, - isExpanded: expandedFieldID == field.id, + isExpanded: Self.isExpanded(field, expandedColumnIndex: expandedColumnIndex), + resizedHeight: Self.resizedHeight(of: field, in: $resizedHeights), commands: commands, isPendingExpression: field.isPendingExpression, onSetValue: { editState.setValue($0, at: field.columnIndex) }, onRemoveField: offersFieldRemoval ? { editState.removeField(at: field.columnIndex) } : nil, onToggleExpand: FieldEditorContent.canExpand(kind: kind, state: FieldValueState.resolve(field)) - ? { expandedFieldID = expandedFieldID == field.id ? nil : field.id } + ? { + expandedColumnIndex = Self.isExpanded(field, expandedColumnIndex: expandedColumnIndex) + ? nil + : field.columnIndex + } : nil, onPopOut: { onPopOut?(field, $0, kind) }, onTabStopChange: { [tabStops] in tabStops.record($0, for: field.id) }, @@ -172,6 +180,20 @@ internal struct InspectorFieldListView: View { ) } + /// Keyed by column, not by field id: ids are reissued on every selection change, so a field + /// expanded on one row collapsed the moment the next row was selected. + internal static func isExpanded(_ field: FieldEditState, expandedColumnIndex: Int?) -> Bool { + field.columnIndex == expandedColumnIndex + } + + internal static func resizedHeight(of field: FieldEditState, in heights: Binding<[Int: Double]>) -> Binding { + let column = field.columnIndex + return Binding( + get: { heights.wrappedValue[column] }, + set: { heights.wrappedValue[column] = $0 } + ) + } + private func context( for field: FieldEditState, kind: FieldEditorKind, diff --git a/apple/TablePro/Views/RowInspector/InspectorFieldRow.swift b/apple/TablePro/Views/RowInspector/InspectorFieldRow.swift index 20ca42288f..f8fbb09648 100644 --- a/apple/TablePro/Views/RowInspector/InspectorFieldRow.swift +++ b/apple/TablePro/Views/RowInspector/InspectorFieldRow.swift @@ -15,6 +15,7 @@ internal struct InspectorFieldRow: View { internal let isPrimaryKey: Bool internal let isForeignKey: Bool internal let isExpanded: Bool + internal let resizedHeight: Binding internal let commands: [SetValueCommand] internal var isPendingExpression = false internal let onSetValue: (SetValueCommand) -> Void @@ -169,6 +170,7 @@ internal struct InspectorFieldRow: View { FieldEditorContent( context: context, kind: kind, + resizedHeight: resizedHeight, onSetNull: context.canMutate && commands.contains(.null) ? { onSetValue(.null) } : nil, onSetDefault: context.canMutate && commands.contains(.defaultValue) ? { onSetValue(.defaultValue) } : nil, onPopOut: onPopOut, diff --git a/apple/TableProTests/Views/RowInspector/FieldExpansionPolicyTests.swift b/apple/TableProTests/Views/RowInspector/FieldExpansionPolicyTests.swift index 28caac3e41..a3ea509f90 100644 --- a/apple/TableProTests/Views/RowInspector/FieldExpansionPolicyTests.swift +++ b/apple/TableProTests/Views/RowInspector/FieldExpansionPolicyTests.swift @@ -7,6 +7,7 @@ // import Foundation +import SwiftUI @testable import TablePro import Testing @@ -46,6 +47,51 @@ struct FieldExpansionPolicyTests { #expect(FieldEditorContent.canExpand(kind: .multiLine, state: .multipleValues)) } + // MARK: - Across rows + + private func field(column: Int) -> FieldEditState { + FieldEditState( + columnIndex: column, + columnName: "payload", + columnTypeEnum: .text(rawType: "JSON"), + isLongText: false, + isJson: true, + hasMultipleValues: false, + pendingValue: nil, + isPendingNull: false, + isPendingDefault: false + ) + } + + /// Selecting another row builds its fields afresh, each with a new id. + @Test("An expanded field stays expanded on the next row") + func expansionSurvivesSelectionChange() { + let first = field(column: 2) + let next = field(column: 2) + #expect(first.id != next.id) + #expect(InspectorFieldListView.isExpanded(first, expandedColumnIndex: 2)) + #expect(InspectorFieldListView.isExpanded(next, expandedColumnIndex: 2)) + } + + @Test("Only the expanded column is expanded") + func otherColumnsStayCollapsed() { + #expect(InspectorFieldListView.isExpanded(field(column: 3), expandedColumnIndex: 2) == false) + #expect(InspectorFieldListView.isExpanded(field(column: 2), expandedColumnIndex: nil) == false) + } + + /// Dragging one field once resized every field of its kind, and the size it was given went + /// away with the row. + @Test("A dragged height belongs to its column and outlives the row") + func resizedHeightIsPerColumn() { + var heights: [Int: Double] = [:] + let store = Binding(get: { heights }, set: { heights = $0 }) + + InspectorFieldListView.resizedHeight(of: field(column: 2), in: store).wrappedValue = 300 + + #expect(InspectorFieldListView.resizedHeight(of: field(column: 2), in: store).wrappedValue == 300) + #expect(InspectorFieldListView.resizedHeight(of: field(column: 3), in: store).wrappedValue == nil) + } + // MARK: - Geometry private func geometry(_ editor: GeometryTextEditor, _ source: GeometryFieldSource) -> FieldEditorKind { diff --git a/apple/TableProTests/Views/RowInspector/FieldResizeHandleTests.swift b/apple/TableProTests/Views/RowInspector/FieldResizeHandleTests.swift new file mode 100644 index 0000000000..ffd077c034 --- /dev/null +++ b/apple/TableProTests/Views/RowInspector/FieldResizeHandleTests.swift @@ -0,0 +1,118 @@ +// +// FieldResizeHandleTests.swift +// TableProTests +// +// The handle owns the mouse from press to release, so the drag is measured from the press and +// the release always reports where it ended. +// + +import AppKit +import SwiftUI +@testable import TablePro +import Testing + +@MainActor +struct FieldResizeHandleTests { + private let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 200, height: 400), + styleMask: [.titled], + backing: .buffered, + defer: true + ) + + private func event(_ type: NSEvent.EventType, y: CGFloat) -> NSEvent { + NSEvent.mouseEvent( + with: type, + location: NSPoint(x: 50, y: y), + modifierFlags: [], + timestamp: 0, + windowNumber: window.windowNumber, + context: nil, + eventNumber: 0, + clickCount: 1, + pressure: 1 + )! + } + + @Test("Dragging down reports a growing delta, and the release reports the final one") + func reportsDownwardDrag() { + let view = FieldResizeHandleView() + var drags: [Double] = [] + var ended: Double? + view.onDrag = { drags.append($0) } + view.onEnd = { ended = $0 } + + view.mouseDown(with: event(.leftMouseDown, y: 300)) + view.mouseDragged(with: event(.leftMouseDragged, y: 290)) + view.mouseDragged(with: event(.leftMouseDragged, y: 260)) + view.mouseUp(with: event(.leftMouseUp, y: 250)) + + #expect(drags == [10, 40]) + #expect(ended == 50) + } + + @Test("Dragging up reports a negative delta") + func reportsUpwardDrag() { + let view = FieldResizeHandleView() + var ended: Double? + view.onEnd = { ended = $0 } + + view.mouseDown(with: event(.leftMouseDown, y: 300)) + view.mouseUp(with: event(.leftMouseUp, y: 330)) + + #expect(ended == -30) + } + + /// The press has to land on the AppKit view, not on SwiftUI's hosting view: a gesture there lost + /// the mouse to the inspector's table a few points into the drag. + @Test("A press on the grip lands on the handle view") + func pressLandsOnHandle() throws { + let container = ResizableEditorContainer(height: .constant(120), range: 80...600) { + Color.clear + } + let hosting = NSHostingView(rootView: container) + hosting.frame = NSRect(x: 0, y: 0, width: 200, height: 200) + window.contentView = hosting + hosting.layoutSubtreeIfNeeded() + + let handle = try #require(Self.firstSubview(of: FieldResizeHandleView.self, in: hosting)) + let center = handle.convert(NSPoint(x: handle.bounds.midX, y: handle.bounds.midY), to: nil) + let root = try #require(hosting.superview) + + #expect(root.hitTest(root.convert(center, from: nil)) === handle) + } + + private static func firstSubview(of type: View.Type, in view: NSView) -> View? { + for subview in view.subviews { + if let match = subview as? View ?? firstSubview(of: type, in: subview) { return match } + } + return nil + } + + @Test("A handle removed under the pointer stops reporting a hover") + func removalEndsHover() { + let view = FieldResizeHandleView() + var hovers: [Bool] = [] + view.onHover = { hovers.append($0) } + window.contentView?.addSubview(view) + + view.mouseEntered(with: event(.mouseMoved, y: 10)) + view.removeFromSuperview() + + #expect(hovers == [true, false]) + } + + /// A release with no press on the handle belongs to some other drag. + @Test("A drag that did not start on the handle reports nothing") + func ignoresStrayEvents() { + let view = FieldResizeHandleView() + var reported = false + view.onDrag = { _ in reported = true } + view.onEnd = { _ in reported = true } + + view.mouseDragged(with: event(.leftMouseDragged, y: 200)) + view.mouseUp(with: event(.leftMouseUp, y: 200)) + + #expect(reported == false) + } +} diff --git a/apple/TableProTests/Views/RowInspector/InspectorValueThemeTests.swift b/apple/TableProTests/Views/RowInspector/InspectorValueThemeTests.swift index dae89e5df5..3305a04f4c 100644 --- a/apple/TableProTests/Views/RowInspector/InspectorValueThemeTests.swift +++ b/apple/TableProTests/Views/RowInspector/InspectorValueThemeTests.swift @@ -33,7 +33,7 @@ struct InspectorValueThemeTests { } private func field(_ value: String, kind: FieldEditorKind, state: FieldValueState? = nil) -> FieldEditorContent { - FieldEditorContent(context: context(value, state: state), kind: kind) + FieldEditorContent(context: context(value, state: state), kind: kind, resizedHeight: .constant(nil)) } private func pixel(of view: some View, size: CGSize, x: Int, fromBottom: Int? = nil) throws -> NSColor { @@ -85,7 +85,7 @@ struct InspectorValueThemeTests { valueState: .value(value.wrappedValue), isReadOnly: false ) - return FieldEditorContent(context: context, kind: kind) + return FieldEditorContent(context: context, kind: kind, resizedHeight: .constant(nil)) } private func editableField(_ value: String, kind: FieldEditorKind = .singleLine) -> FieldEditorContent { diff --git a/apple/TableProTests/Views/RowInspector/ResizableFieldMetricsTests.swift b/apple/TableProTests/Views/RowInspector/ResizableFieldMetricsTests.swift index 7ed3e341bf..6d98af9f3d 100644 --- a/apple/TableProTests/Views/RowInspector/ResizableFieldMetricsTests.swift +++ b/apple/TableProTests/Views/RowInspector/ResizableFieldMetricsTests.swift @@ -1,3 +1,4 @@ +import SwiftUI import Testing @testable import TablePro @@ -45,6 +46,29 @@ struct ResizableFieldMetricsTests { #expect(ResizableFieldMetrics.resolve(base: 160, delta: 5_000, range: mapRange) == 600) } + /// A text pane dragged to 60 and then shown as a 100pt map: the drag moves it from 100 at once. + @Test func testABaseBelowTheFloorMovesWithTheFirstPointOfDrag() { + let mapRange = ResizableFieldMetrics.geometryHeightRange + #expect(ResizableFieldMetrics.resolve(base: 60, delta: 30, range: mapRange) == 130) + #expect(ResizableFieldMetrics.resolve(base: 700, delta: -50, range: mapRange) == 550) + } + + /// Every field of a kind once shared one stored height, so dragging one resized them all. + @Test func testDraggingOneFieldLeavesTheOthersAtTheirStartingHeight() { + var resized: [Int: Double] = [:] + func height(for column: Int) -> Binding { + ResizableFieldMetrics.height( + resized: Binding(get: { resized[column] }, set: { resized[column] = $0 }), + starting: 120 + ) + } + + height(for: 1).wrappedValue = 300 + + #expect(height(for: 1).wrappedValue == 300) + #expect(height(for: 2).wrappedValue == 120) + } + @Test func testTextHeightResolvesWithinItsOwnRange() { let textRange = ResizableFieldMetrics.textHeightRange #expect(ResizableFieldMetrics.resolve(base: 110, delta: -500, range: textRange) == textRange.lowerBound) From 93fa0072d16b606e25c3ab456cb39259adf6d9b1 Mon Sep 17 00:00:00 2001 From: Victor Babenko Date: Sat, 31 Oct 2026 19:59:25 -0700 Subject: [PATCH 2/3] chore(changelog): inspector resize and expansion fixes Co-authored-by: Cursor --- apple/changelog.d/3975.fixed.1.md | 1 + apple/changelog.d/3975.fixed.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 apple/changelog.d/3975.fixed.1.md create mode 100644 apple/changelog.d/3975.fixed.md diff --git a/apple/changelog.d/3975.fixed.1.md b/apple/changelog.d/3975.fixed.1.md new file mode 100644 index 0000000000..b4b852148d --- /dev/null +++ b/apple/changelog.d/3975.fixed.1.md @@ -0,0 +1 @@ +An expanded field in the row inspector collapsed when another row was selected. diff --git a/apple/changelog.d/3975.fixed.md b/apple/changelog.d/3975.fixed.md new file mode 100644 index 0000000000..228e200024 --- /dev/null +++ b/apple/changelog.d/3975.fixed.md @@ -0,0 +1 @@ +Dragging a field's resize handle in the row inspector snapped back, or resized every field of that kind. From a859387de663dd52a35aed9e860577974e414368 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 1 Nov 2026 13:25:22 +0700 Subject: [PATCH 3/3] refactor(inspector): keep field heights per column across relaunches and harden the resize handle --- .claude/rules/data-sync-storage.md | 1 + apple/TablePro/AppDelegate.swift | 2 +- .../Core/Storage/AppSettingsManager.swift | 1 + .../Core/Storage/AppSettingsStorage.swift | 27 ++- .../Storage/InspectorFieldHeightStore.swift | 83 +++++++ .../Storage/Preferences/PreferenceKeys.swift | 10 +- .../Models/UI/MultiRowEditState.swift | 8 + .../ResizeCursorSplitViewController.swift | 13 +- .../FieldEditors/FieldEditorContent.swift | 13 +- .../FieldEditors/FieldResizeHandle.swift | 112 ++++++---- .../FieldEditors/GeometryFieldView.swift | 66 ++---- .../FieldEditors/JsonEditorView.swift | 24 +- .../FieldEditors/MultiLineEditorView.swift | 7 +- .../ResizableEditorContainer.swift | 67 +++--- .../FieldEditors/ResizableFieldMetrics.swift | 26 +-- .../RowInspector/InspectorFieldListView.swift | 82 ++----- .../RowInspector/InspectorFieldRow.swift | 2 - .../AppSettingsStorageMigrationTests.swift | 47 ++-- .../AppSettingsStorageResetTests.swift | 10 +- .../InspectorFieldHeightStoreTests.swift | 80 +++++++ .../Storage/PreferenceKeysGuardTests.swift | 4 +- .../UI/MultiRowEditStateExpansionTests.swift | 80 +++++++ .../FieldExpansionPolicyTests.swift | 46 ---- .../RowInspector/FieldResizeHandleTests.swift | 205 +++++++++++++----- .../InspectorValueThemeTests.swift | 4 +- .../ResizableFieldMetricsTests.swift | 23 +- apple/changelog.d/3975.changed.md | 1 + apple/changelog.d/3975.fixed.md | 2 +- 28 files changed, 636 insertions(+), 410 deletions(-) create mode 100644 apple/TablePro/Core/Storage/InspectorFieldHeightStore.swift create mode 100644 apple/TableProTests/Core/Storage/InspectorFieldHeightStoreTests.swift create mode 100644 apple/TableProTests/Models/UI/MultiRowEditStateExpansionTests.swift create mode 100644 apple/changelog.d/3975.changed.md diff --git a/.claude/rules/data-sync-storage.md b/.claude/rules/data-sync-storage.md index daafa2f767..042a2bf20a 100644 --- a/.claude/rules/data-sync-storage.md +++ b/.claude/rules/data-sync-storage.md @@ -35,6 +35,7 @@ This path holds user data. A silent wrong answer here looks exactly like an empt | Filter defaults and presets | UserDefaults | `FilterSettingsStorage`, `FilterPresetStorage` | | Per-table filters | JSON, one file per table | `FilterSettingsStorage` | | Generate Rows settings per table | UserDefaults, device-local | `RowGenerationSettingsStore` | +| Row inspector field heights, per column name | UserDefaults, device-local | `InspectorFieldHeightStore` | | Favorite tables (iCloud-synced) | UserDefaults | `FavoriteTablesStorage` | | Sidebar database and schema filters, recent tables | UserDefaults, device-local | `DatabaseTreeFilterStorage`, `RecentTablesStore` | | History drawer state | UserDefaults, device-local | `HistoryPanelPreferencesStorage` | diff --git a/apple/TablePro/AppDelegate.swift b/apple/TablePro/AppDelegate.swift index d87a339acd..c50ee76efa 100644 --- a/apple/TablePro/AppDelegate.swift +++ b/apple/TablePro/AppDelegate.swift @@ -21,7 +21,7 @@ class AppDelegate: NSObject, NSApplicationDelegate { func applicationWillFinishLaunching(_ notification: Notification) { LaunchTracer.shared.mark(.willFinishLaunchingBegan) AppSettingsStorage.shared.migrateStartupBehaviorToReopenLastIfNeeded() - AppSettingsStorage.shared.migrateJsonFieldHeightKeyIfNeeded() + AppSettingsStorage.shared.removeRetiredFieldHeightKeys() AIProviderRegistration.registerAll() /// Installed before any window exists, so the bar is correct from the first frame. diff --git a/apple/TablePro/Core/Storage/AppSettingsManager.swift b/apple/TablePro/Core/Storage/AppSettingsManager.swift index 253cc4f055..47b3a24e27 100644 --- a/apple/TablePro/Core/Storage/AppSettingsManager.swift +++ b/apple/TablePro/Core/Storage/AppSettingsManager.swift @@ -277,6 +277,7 @@ final class AppSettingsManager: ObservableObject { mcp = .default typography = .default storage.resetToDefaults() + InspectorFieldHeightStore.shared.reload() connectionListPreferences.resetShowsRecent() SoftwareUpdater.shared.resetUpdatePreferences() AppModuleActivation.resetToDefaults() diff --git a/apple/TablePro/Core/Storage/AppSettingsStorage.swift b/apple/TablePro/Core/Storage/AppSettingsStorage.swift index 6e789f10e3..42809b4038 100644 --- a/apple/TablePro/Core/Storage/AppSettingsStorage.swift +++ b/apple/TablePro/Core/Storage/AppSettingsStorage.swift @@ -37,8 +37,14 @@ final class AppSettingsStorage: Sendable { static let typography = "com.TablePro.settings.typography" static let hasSeenWelcomeSheet = "com.TablePro.settings.hasCompletedOnboarding" static let startupReopenMigration = "com.TablePro.settings.didMigrateStartupToReopenLast" - static let jsonFieldHeightMigration = "com.TablePro.settings.didMigrateJsonFieldHeightKey" - static let legacyJsonFieldHeight = "rightSidebar.jsonFieldHeight" + // One height per editor kind, before a height belonged to its column. + static let retiredFieldHeights = [ + "rightSidebar.jsonFieldHeight", + "com.TablePro.rightSidebar.jsonFieldHeight", + "com.TablePro.rightSidebar.textFieldHeight", + "com.TablePro.rightSidebar.geometryFieldHeight", + "com.TablePro.settings.didMigrateJsonFieldHeightKey", + ] } /// The storage environment's defaults, not `.standard`. A UI test runs against a per-sandbox @@ -71,15 +77,10 @@ final class AppSettingsStorage: Sendable { saveGeneral(general) } - func migrateJsonFieldHeightKeyIfNeeded() { - guard !defaults.bool(forKey: Keys.jsonFieldHeightMigration) else { return } - defaults.set(true, forKey: Keys.jsonFieldHeightMigration) - - let newKey = PreferenceKeys.rowInspectorJsonFieldHeight.name - guard defaults.object(forKey: Keys.legacyJsonFieldHeight) != nil, - defaults.object(forKey: newKey) == nil else { return } - defaults.set(defaults.double(forKey: Keys.legacyJsonFieldHeight), forKey: newKey) - defaults.removeObject(forKey: Keys.legacyJsonFieldHeight) + func removeRetiredFieldHeightKeys() { + for key in Keys.retiredFieldHeights where defaults.object(forKey: key) != nil { + defaults.removeObject(forKey: key) + } } // MARK: - Appearance Settings @@ -310,9 +311,7 @@ final class AppSettingsStorage: Sendable { saveMCP(.default) saveTypography(.default) defaults.removeObject(forKey: PreferenceKeys.selectedSettingsPane.name) - defaults.removeObject(forKey: PreferenceKeys.rowInspectorJsonFieldHeight.name) - defaults.removeObject(forKey: PreferenceKeys.rowInspectorTextFieldHeight.name) - defaults.removeObject(forKey: PreferenceKeys.rowInspectorGeometryFieldHeight.name) + defaults.removeObject(forKey: PreferenceKeys.rowInspectorFieldHeights.name) defaults.removeObject(forKey: PreferenceKeys.rowInspectorGeometryFieldMode.name) defaults.removeObject(forKey: SidebarPersistenceKey.defaultLayout) RoutineDebugOptionStore(defaults: { [defaults] in defaults }).removeAll() diff --git a/apple/TablePro/Core/Storage/InspectorFieldHeightStore.swift b/apple/TablePro/Core/Storage/InspectorFieldHeightStore.swift new file mode 100644 index 0000000000..aef607926e --- /dev/null +++ b/apple/TablePro/Core/Storage/InspectorFieldHeightStore.swift @@ -0,0 +1,83 @@ +// +// InspectorFieldHeightStore.swift +// TablePro +// + +import Combine +import Foundation +import os + +// Device-local. Keyed by column name alone, so a size holds on every row and in every result showing that column. +@MainActor +internal final class InspectorFieldHeightStore: ObservableObject { + static let shared = InspectorFieldHeightStore() + static let capacity = 200 + + nonisolated private static let logger = Logger(subsystem: "com.TablePro", category: "InspectorFieldHeightStore") + private static let key = PreferenceKeys.rowInspectorFieldHeights.name + + @Published private(set) var heights: [String: Double] = [:] + // Least recently set first, so the oldest is dropped past `capacity`. + private var order: [String] = [] + private let store: KeyValueStore + + init(defaults: KeyValueStore = AppStorageEnvironment.shared.defaults) { + store = defaults + reload() + } + + func height(for column: String) -> Double? { + heights[column] + } + + func setHeight(_ height: Double, for column: String) { + order.removeAll { $0 == column } + order.append(column) + var updated = heights + updated[column] = height + if order.count > Self.capacity { + for evicted in order.prefix(order.count - Self.capacity) { + updated[evicted] = nil + } + order.removeFirst(order.count - Self.capacity) + } + heights = updated + persist() + } + + func reload() { + var loaded: [String: Double] = [:] + var loadedOrder: [String] = [] + for entry in store.dataValue(forKey: Self.key).flatMap(Self.decode) ?? [] { + if loaded.updateValue(entry.height, forKey: entry.column) != nil { + loadedOrder.removeAll { $0 == entry.column } + } + loadedOrder.append(entry.column) + } + order = loadedOrder + heights = loaded + } + + private func persist() { + let entries = order.compactMap { column in heights[column].map { Entry(column: column, height: $0) } } + do { + store.setDataValue(try JSONEncoder().encode(entries), forKey: Self.key) + } catch { + Self.logger.error("Could not save field heights: \(error.publicLogShape, privacy: .public)") + } + } + + private static func decode(_ data: Data) -> [Entry]? { + do { + return try JSONDecoder().decode([Entry].self, from: data) + } catch { + logger.error("Discarding unreadable field heights: \(error.publicLogShape, privacy: .public)") + return nil + } + } + + private struct Entry: Codable { + let column: String + let height: Double + } +} diff --git a/apple/TablePro/Core/Storage/Preferences/PreferenceKeys.swift b/apple/TablePro/Core/Storage/Preferences/PreferenceKeys.swift index a6514424f8..e9cbca1d32 100644 --- a/apple/TablePro/Core/Storage/Preferences/PreferenceKeys.swift +++ b/apple/TablePro/Core/Storage/Preferences/PreferenceKeys.swift @@ -4,10 +4,8 @@ enum PreferenceKeys { static let linkedFolders = DefaultsKey<[LinkedFolder]>("com.TablePro.linkedFolders") static let linkedSQLFolders = DefaultsKey<[LinkedSQLFolder]>("com.TablePro.linkedSQLFolders") static let selectedSettingsPane = DefaultsKey("com.TablePro.settings.selectedPane") - static let rowInspectorJsonFieldHeight = DefaultsKey("com.TablePro.rightSidebar.jsonFieldHeight") - static let rowInspectorTextFieldHeight = DefaultsKey("com.TablePro.rightSidebar.textFieldHeight") - static let rowInspectorGeometryFieldHeight = DefaultsKey("com.TablePro.rightSidebar.geometryFieldHeight") - /// Text or Map, as the field's picker last left it. Device-local, like the heights beside it. + static let rowInspectorFieldHeights = DefaultsKey("com.TablePro.rightSidebar.fieldHeights") + // Text or Map, as the field's picker last left it. Device-local, like the heights beside it. static let rowInspectorGeometryFieldMode = DefaultsKey("com.TablePro.rightSidebar.geometryFieldMode") static let workspaceRailOrder = DefaultsKey<[WorkspaceID]>("com.TablePro.workspaceRail.order") static let queryPlanRawFontSize = DefaultsKey("com.TablePro.queryPlan.rawFontSize") @@ -33,9 +31,7 @@ enum PreferenceKeys { linkedFolders.name, linkedSQLFolders.name, selectedSettingsPane.name, - rowInspectorJsonFieldHeight.name, - rowInspectorTextFieldHeight.name, - rowInspectorGeometryFieldHeight.name, + rowInspectorFieldHeights.name, rowInspectorGeometryFieldMode.name, workspaceRailOrder.name, queryPlanRawFontSize.name, diff --git a/apple/TablePro/Models/UI/MultiRowEditState.swift b/apple/TablePro/Models/UI/MultiRowEditState.swift index b4c2c95517..26c98ce2f2 100644 --- a/apple/TablePro/Models/UI/MultiRowEditState.swift +++ b/apple/TablePro/Models/UI/MultiRowEditState.swift @@ -127,6 +127,8 @@ final class MultiRowEditState: ObservableObject { @Published private(set) var expressionCells: [Set] = [] @Published private(set) var columns: [String] = [] @Published private(set) var columnTypes: [ColumnType] = [] + // By column, not field id: every selection change reissues the ids. Cleared when the columns change. + @Published private(set) var expandedColumn: Int? private var nonNullableColumns: Set = [] // Nil and .none offer no Set Value command and promote nothing. @@ -180,6 +182,7 @@ final class MultiRowEditState: ObservableObject { self.columnTypes = columnTypes self.nonNullableColumns = nonNullableColumns self.databaseType = databaseType + if columnsChanged { expandedColumn = nil } self.cellWriter = cellWriter rowsAreInserted = !rowIDs.isEmpty && rowIDs.allSatisfy { $0.isInserted } @@ -576,6 +579,10 @@ final class MultiRowEditState: ObservableObject { } } + func toggleExpansion(ofColumn index: Int) { + expandedColumn = expandedColumn == index ? nil : index + } + func clearEdits() { for index in fields.indices { resetPending(at: index) @@ -598,6 +605,7 @@ final class MultiRowEditState: ObservableObject { columns = [] columnTypes = [] nonNullableColumns = [] + expandedColumn = nil databaseType = nil cellWriter = .none rowsAreInserted = false diff --git a/apple/TablePro/Views/Components/ResizeCursorSplitViewController.swift b/apple/TablePro/Views/Components/ResizeCursorSplitViewController.swift index 1916057b08..89529747ff 100644 --- a/apple/TablePro/Views/Components/ResizeCursorSplitViewController.swift +++ b/apple/TablePro/Views/Components/ResizeCursorSplitViewController.swift @@ -109,11 +109,16 @@ internal class ResizeCursorSplitViewController: NSSplitViewController { } private var resizeCursor: NSCursor { + .dividerResize(isVertical: splitView.isVertical) + } +} + +internal extension NSCursor { + // `isVertical` as NSSplitView means it: a vertical divider resizes left and right. + static func dividerResize(isVertical: Bool) -> NSCursor { if #available(macOS 15.0, *) { - return splitView.isVertical - ? .columnResize(directions: [.left, .right]) - : .rowResize(directions: [.up, .down]) + return isVertical ? .columnResize(directions: [.left, .right]) : .rowResize(directions: [.up, .down]) } - return splitView.isVertical ? .resizeLeftRight : .resizeUpDown + return isVertical ? .resizeLeftRight : .resizeUpDown } } diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/FieldEditorContent.swift b/apple/TablePro/Views/RowInspector/FieldEditors/FieldEditorContent.swift index 6a10a3afe3..b8094dbc0c 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/FieldEditorContent.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/FieldEditorContent.swift @@ -10,9 +10,6 @@ import SwiftUI internal struct FieldEditorContent: View { internal let context: FieldEditorContext internal let kind: FieldEditorKind - /// The height this field was dragged to, nil until it is. Required so no caller drops it: a - /// constant here takes the drag and throws it away on release. - internal let resizedHeight: Binding internal var onSetNull: (() -> Void)? internal var onSetDefault: (() -> Void)? internal var onPopOut: ((String) -> Void)? @@ -67,7 +64,7 @@ internal struct FieldEditorContent: View { private var editor: some View { switch kind { case .json: - JsonEditorView(context: context, onPopOut: onPopOut, isExpanded: isExpanded, resizedHeight: resizedHeight) + JsonEditorView(context: context, onPopOut: onPopOut, isExpanded: isExpanded) case .phpSerialized: PhpSerializedFieldView(context: context, onPopOut: onPopOut, isExpanded: isExpanded) case .binaryValue(let format): @@ -80,7 +77,6 @@ internal struct FieldEditorContent: View { descriptor: descriptor, onPopOut: onPopOut, isExpanded: isExpanded, - resizedHeight: resizedHeight, onTabStopChange: onTabStopChange ) case .blobHex: @@ -102,12 +98,7 @@ internal struct FieldEditorContent: View { case .vector: VectorFieldEditorView(context: context, onSetNull: onSetNull, onSetDefault: onSetDefault) case .multiLine: - MultiLineEditorView( - context: context, - onPopOut: onPopOut, - isExpanded: isExpanded, - resizedHeight: resizedHeight - ) + MultiLineEditorView(context: context, onPopOut: onPopOut, isExpanded: isExpanded) case .singleLine: SingleLineEditorView(context: context) } diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/FieldResizeHandle.swift b/apple/TablePro/Views/RowInspector/FieldEditors/FieldResizeHandle.swift index 8259ede410..b23f87e060 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/FieldResizeHandle.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/FieldResizeHandle.swift @@ -6,97 +6,133 @@ import AppKit import SwiftUI -/// The grip under a resizable field, reporting how far it has been dragged downward. -/// -/// AppKit on purpose. The inspector is a SwiftUI `List`, an `NSTableView`, and the table took the -/// mouse from a `DragGesture` in its row a few points into the drag: the gesture was cancelled -/// without ending, so the field snapped back to its stored height. A view that takes `mouseDown` -/// itself receives every drag and the release, as a split view's divider does. +// AppKit on purpose: in the inspector's List, an NSTableView, a SwiftUI DragGesture lost the mouse to the table a few points into the drag. internal struct FieldResizeHandle: NSViewRepresentable { - let onHover: (Bool) -> Void let onDrag: (Double) -> Void let onEnd: (Double) -> Void func makeNSView(context: Context) -> FieldResizeHandleView { - let view = FieldResizeHandleView() - view.setAccessibilityElement(false) - return view + FieldResizeHandleView() } func updateNSView(_ view: FieldResizeHandleView, context: Context) { - view.onHover = onHover view.onDrag = onDrag view.onEnd = onEnd + view.isEnabled = context.environment.isEnabled } } internal final class FieldResizeHandleView: NSView { - var onHover: ((Bool) -> Void)? + static let gripSize = NSSize(width: 26, height: 4) + var onDrag: ((Double) -> Void)? var onEnd: ((Double) -> Void)? - /// Window coordinates, which stay put while the field grows and moves this view down. - private var dragStartY: CGFloat? - private var hoverArea: NSTrackingArea? - private var isHovered = false + // The inspector keeps its field list mounted, disabled, behind the JSON rendering. + var isEnabled = true { + didSet { + guard isEnabled != oldValue, !isEnabled else { return } + drag = nil + isHovered = false + } + } + + private(set) var isHovered = false { + didSet { if isHovered != oldValue { needsDisplay = true } } + } + + // Window coordinates, which stay put while the field grows and moves this view down. + private var drag: (startY: CGFloat, hasMoved: Bool)? { + didSet { if (drag == nil) != (oldValue == nil) { needsDisplay = true } } + } + + var isHighlighted: Bool { isHovered || drag != nil } override var mouseDownCanMoveWindow: Bool { false } override func acceptsFirstMouse(for event: NSEvent?) -> Bool { true } - override func resetCursorRects() { - addCursorRect(bounds, cursor: .resizeUpDown) + override func hitTest(_ point: NSPoint) -> NSView? { + isEnabled ? super.hitTest(point) : nil } override func updateTrackingAreas() { super.updateTrackingAreas() - if let hoverArea { removeTrackingArea(hoverArea) } - let area = NSTrackingArea( + for area in trackingAreas where area.owner === self { + removeTrackingArea(area) + } + addTrackingArea(NSTrackingArea( rect: .zero, - options: [.mouseEnteredAndExited, .activeInActiveApp, .inVisibleRect], + options: [.mouseEnteredAndExited, .cursorUpdate, .activeInKeyWindow, .inVisibleRect], owner: self - ) - addTrackingArea(area) - hoverArea = area + )) + } + + override func cursorUpdate(with event: NSEvent) { + guard isEnabled else { + super.cursorUpdate(with: event) + return + } + NSCursor.dividerResize(isVertical: false).set() } override func mouseEntered(with event: NSEvent) { - isHovered = true - onHover?(true) + isHovered = isEnabled } override func mouseExited(with event: NSEvent) { isHovered = false - onHover?(false) } - /// Expanding from the keyboard removes the handle under a resting pointer, and no exit follows, - /// so the grip stayed lit after collapsing. + // A handle removed mid-drag, as Expand does, gets no mouseUp it could act on. override func viewWillMove(toWindow newWindow: NSWindow?) { super.viewWillMove(toWindow: newWindow) - guard newWindow == nil, isHovered else { return } + guard newWindow == nil else { return } + drag = nil isHovered = false - onHover?(false) } override func mouseDown(with event: NSEvent) { - dragStartY = event.locationInWindow.y + guard isEnabled else { return } + drag = (event.locationInWindow.y, false) } override func mouseDragged(with event: NSEvent) { guard let delta = downwardDelta(to: event) else { return } - NSCursor.resizeUpDown.set() + drag?.hasMoved = true + NSCursor.dividerResize(isVertical: false).set() onDrag?(delta) } override func mouseUp(with event: NSEvent) { - guard let delta = downwardDelta(to: event) else { return } - dragStartY = nil - onEnd?(delta) + guard let delta = downwardDelta(to: event), let drag else { return } + self.drag = nil + if drag.hasMoved { onEnd?(delta) } + } + + override func draw(_ dirtyRect: NSRect) { + guard let context = NSGraphicsContext.current?.cgContext else { return } + let size = Self.gripSize + let grip = NSRect( + x: bounds.midX - size.width / 2, + y: bounds.midY - size.height / 2, + width: size.width, + height: size.height + ) + context.saveGState() + context.setAlpha(isHighlighted ? 1 : 0.5) + NSColor.tertiaryLabelColor.setFill() + NSBezierPath(roundedRect: grip, xRadius: size.height / 2, yRadius: size.height / 2).fill() + context.restoreGState() + } + + override func viewDidChangeEffectiveAppearance() { + super.viewDidChangeEffectiveAppearance() + needsDisplay = true } private func downwardDelta(to event: NSEvent) -> Double? { - guard let dragStartY else { return nil } - return Double(dragStartY - event.locationInWindow.y) + guard let drag else { return nil } + return Double(drag.startY - event.locationInWindow.y) } } diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/GeometryFieldView.swift b/apple/TablePro/Views/RowInspector/FieldEditors/GeometryFieldView.swift index 5991aab102..0efecf9236 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/GeometryFieldView.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/GeometryFieldView.swift @@ -7,23 +7,19 @@ import Combine import SwiftUI import TableProTheme -/// A field whose value is a geometry: a map of that value, with the editor the field would -/// otherwise get one segment away. +// A geometry value drawn as a map, with the field's own editor one segment away. internal struct GeometryFieldView: View { @ObservedObject private var themeEngine = ThemeEngine.shared let context: FieldEditorContext let descriptor: GeometryFieldDescriptor var onPopOut: ((String) -> Void)? var isExpanded = false - let resizedHeight: Binding - /// Told whether Tab can stop here without Full Keyboard Access. A map takes no focus. + // Told whether Tab can stop here without Full Keyboard Access; a map takes no focus. var onTabStopChange: ((Bool) -> Void)? @Environment(\.isEnabled) private var isEnabled @AppStorage(PreferenceKeys.rowInspectorGeometryFieldMode.name, store: AppStorageEnvironment.shared.defaults) private var storedMode = Mode.map.rawValue - @AppStorage(PreferenceKeys.rowInspectorGeometryFieldHeight.name, store: AppStorageEnvironment.shared.defaults) - private var startingMapHeight = ResizableFieldMetrics.defaultGeometryHeight @StateObject private var previews = GeometryFieldPreviewStore() @State private var latch = ModeLatch() @@ -45,15 +41,12 @@ internal struct GeometryFieldView: View { mapPane(resolution.preview) } } - /// The row puts the field's text on its editor as an accessibility value. On a bare - /// container that lands on the picker and the caption, in place of the selected segment. + // On a bare container the row's accessibility value lands on the picker and the caption. .accessibilityElement(children: .contain) .onAppear { onTabStopChange?(mode == .text) } - /// No reset on disappear: a lazy list unmounts a row scrolled out of view while the field is - /// still in the list, and Tab would then stop on a map with nothing to focus. + // No reset on disappear: the lazy list unmounts off-screen rows, and Tab would then stop on a map. .onChange(of: mode) { onTabStopChange?($0 == .text) } - /// Runs again when a read starts or ends, so the mode is latched by whichever preview - /// comes first: the one read during this pass, or the large one read here. + // Reruns when a read starts or ends, so whichever preview comes first latches the mode. .task(id: resolution.pending) { guard let pending = resolution.pending else { latch.offer(preferred: preferredMode, preview: resolution.preview) @@ -64,7 +57,7 @@ internal struct GeometryFieldView: View { } } - /// Text mode reads nothing: the value is only parsed while a map is, or may be, on screen. + // Text mode reads nothing: the value is parsed only while a map is, or may be, on screen. private func resolvePreview() -> GeometryFieldPreviewStore.Resolution { guard latch.needsPreview(preferred: preferredMode) else { return GeometryFieldPreviewStore.Resolution(preview: nil, pending: nil) @@ -114,8 +107,7 @@ internal struct GeometryFieldView: View { } } - /// Only the picker latches a mode and only the picker writes the preference, so a value that - /// becomes drawable while it is typed never moves the field off Text. + // Only the picker latches and stores the mode, so a value turning drawable as it is typed stays on Text. private func modeSelection(shown mode: Mode) -> Binding { Binding( get: { mode }, @@ -126,7 +118,7 @@ internal struct GeometryFieldView: View { ) } - /// No window edits bytes: the text window would write the bytes back as text. + // No window edits bytes: the text window would write them back as text. private func offersWindow(in mode: Mode) -> Bool { switch mode { case .map: return true @@ -153,8 +145,7 @@ internal struct GeometryFieldView: View { return false } - /// The map window is handed the text alone, so it would call a NULL field or a selection of - /// several values unreadable. It opens for a value that draws and for nothing else. + // The map window gets the text alone and would call NULL or mixed values unreadable, so only a drawable value opens it. static func canOpenWindow(in mode: Mode, preview: GeometryFieldPreview?) -> Bool { switch mode { case .map: return isDrawable(preview) @@ -164,20 +155,14 @@ internal struct GeometryFieldView: View { // MARK: - Panes - /// The embedded editors carry no pop-out button of their own: the toolbar's one button serves - /// both segments. This kind opts out of the row's value font, so the pane names it. + // The toolbar's pop-out serves both segments; this kind opts out of the row's value font, so the pane sets it. private var textPane: some View { Group { switch descriptor.textEditor { case .multiLine: - MultiLineEditorView( - context: context, - onPopOut: nil, - isExpanded: isExpanded, - resizedHeight: resizedHeight - ) + MultiLineEditorView(context: context, onPopOut: nil, isExpanded: isExpanded) case .json: - JsonEditorView(context: context, onPopOut: nil, isExpanded: isExpanded, resizedHeight: resizedHeight) + JsonEditorView(context: context, onPopOut: nil, isExpanded: isExpanded) case .hex: BlobHexEditorView(context: context) } @@ -188,7 +173,8 @@ internal struct GeometryFieldView: View { private func mapPane(_ preview: GeometryFieldPreview?) -> some View { VStack(alignment: .leading, spacing: 2) { ResizableEditorContainer( - height: ResizableFieldMetrics.height(resized: resizedHeight, starting: startingMapHeight), + column: context.columnName, + defaultHeight: ResizableFieldMetrics.defaultGeometryHeight, range: ResizableFieldMetrics.geometryHeightRange, expandedHeight: isExpanded ? ResizableFieldMetrics.expandedHeight : nil ) { @@ -205,8 +191,7 @@ internal struct GeometryFieldView: View { } } - /// The canvas is dropped while the list is disabled, which is how the inspector hides it behind - /// its JSON rendering: a mounted map view holds about 45 MB whether or not it is on screen. + // Dropped while disabled, which is how the inspector hides it behind the JSON rendering: a mounted map holds about 45 MB. @ViewBuilder private func mapContent(_ preview: GeometryFieldPreview?) -> some View { switch preview { @@ -241,7 +226,7 @@ internal extension GeometryFieldView { case text case map - /// Nil while the first read of a large value is still running: nothing has decided yet. + // Nil while the first read of a large value is still running. static func opening(preferred: Mode, preview: GeometryFieldPreview?) -> Mode? { guard preferred == .map else { return .text } switch preview { @@ -252,8 +237,7 @@ internal extension GeometryFieldView { } } - /// The mode a field opens on is decided once per view. Following the text instead would swap - /// the editor out from under the caret the moment a typed value became drawable. + // Decided once per view: following the text would swap the editor from under the caret once a typed value draws. struct ModeLatch: Equatable { private(set) var mode: Mode? @@ -278,8 +262,7 @@ internal extension GeometryFieldView { // MARK: - Preview store -/// Reads a value once per text rather than once per body pass: the list redraws every field on -/// each keystroke in any of them. Not published, so filling it during a pass redraws nothing. +// Once per text, not per body pass, since every keystroke redraws every field. Unpublished, so filling it redraws nothing. @MainActor internal final class GeometryFieldPreviewStore: ObservableObject { internal struct Request: Equatable, Sendable { @@ -288,8 +271,7 @@ internal final class GeometryFieldPreviewStore: ObservableObject { } internal struct Resolution { - /// The newest preview there is. While `pending` is set it is the previous value's, so the - /// map stays up during a re-read. + // While `pending` is set this is the previous value's preview, so the map stays up during a re-read. let preview: GeometryFieldPreview? let pending: Request? } @@ -297,12 +279,11 @@ internal final class GeometryFieldPreviewStore: ObservableObject { internal typealias Reader = @Sendable (Request, GeometryFieldSource) -> GeometryFieldPreview internal private(set) var preview: GeometryFieldPreview? - /// Moves with the shapes, so the canvas can tell new ones from the ones it already drew. + // Moves with the shapes, so the canvas can tell new ones from the ones it already drew. internal private(set) var token = 0 private var loaded: Request? private var pending: Request? - /// A parse runs to the end once started, so a pane re-added mid-read joins it instead of - /// starting a second copy of the same work. + // A parse runs to the end, so a pane re-added mid-read joins it instead of starting a second one. private var inFlight: (request: Request, task: Task)? private let reader: Reader @@ -323,8 +304,7 @@ internal final class GeometryFieldPreviewStore: ObservableObject { return Resolution(preview: preview, pending: nil) } - /// Nil when the value moved on while this one was being read. A finished read is committed even - /// when its caller was cancelled, so a pane that comes back finds it done. + // Nil when the value moved on during the read. A finished read commits even when its caller was cancelled. internal func load(_ request: Request, source: GeometryFieldSource) async -> GeometryFieldPreview? { if request == loaded { return preview } let task: Task @@ -353,7 +333,7 @@ internal final class GeometryFieldPreviewStore: ObservableObject { } internal extension View { - /// The map pane's well, which shows while the map builds or for a value with nothing to draw. + // Shows while the map builds, or for a value with nothing to draw. func geometryMapWell() -> some View { themeBackground(.editorBackground, native: Color(nsColor: .textBackgroundColor)) .clipShape(RoundedRectangle(cornerRadius: 5)) diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/JsonEditorView.swift b/apple/TablePro/Views/RowInspector/FieldEditors/JsonEditorView.swift index 0fee9317c2..231f0c5d11 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/JsonEditorView.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/JsonEditorView.swift @@ -9,28 +9,17 @@ internal struct JsonEditorView: View { let context: FieldEditorContext var onPopOut: ((String) -> Void)? var isExpanded = false - let resizedHeight: Binding @State private var model: JsonFieldEditingModel - @AppStorage(PreferenceKeys.rowInspectorJsonFieldHeight.name, store: AppStorageEnvironment.shared.defaults) private var startingHeight = ResizableFieldMetrics - .defaultJsonHeight - init( - context: FieldEditorContext, - onPopOut: ((String) -> Void)? = nil, - isExpanded: Bool = false, - resizedHeight: Binding - ) { + init(context: FieldEditorContext, onPopOut: ((String) -> Void)? = nil, isExpanded: Bool = false) { self.context = context self.onPopOut = onPopOut self.isExpanded = isExpanded - self.resizedHeight = resizedHeight self._model = State(wrappedValue: JsonFieldEditingModel(storedValue: context.value.wrappedValue)) } - /// The editor writes through the model, which decides whether the text is worth publishing. - /// Binding `$model.displayText` straight to the editor would publish the model's own - /// corrections back into the store. + // Through the model: binding `$model.displayText` directly would publish the model's own corrections. private var editorText: Binding { Binding( get: { model.displayText }, @@ -43,7 +32,8 @@ internal struct JsonEditorView: View { var body: some View { ResizableEditorContainer( - height: ResizableFieldMetrics.height(resized: resizedHeight, starting: startingHeight), + column: context.columnName, + defaultHeight: ResizableFieldMetrics.defaultJsonHeight, range: ResizableFieldMetrics.jsonHeightRange, expandedHeight: isExpanded ? ResizableFieldMetrics.expandedHeight : nil ) { @@ -55,11 +45,7 @@ internal struct JsonEditorView: View { .valueWell(clipsContent: true) .overlay(alignment: .bottomTrailing) { actionButtons } } - /// `newValue`, never a re-read of `context.value.wrappedValue`. An `onChange` action - /// closure belongs to the render that registered it, so its captured context is a render - /// behind and answers with the value this edit has just replaced. Re-reading it made the - /// editor undo every keystroke and push the undone text back, at 75 rounds a second and - /// without ever settling (#3051). + // `newValue`, never `context.value`: the action's context is a render behind and undid every keystroke (#3051). .onChange(of: context.value.wrappedValue) { newValue in _ = model.received(newValue) } diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/MultiLineEditorView.swift b/apple/TablePro/Views/RowInspector/FieldEditors/MultiLineEditorView.swift index 0fa20ad931..b324f11faf 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/MultiLineEditorView.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/MultiLineEditorView.swift @@ -11,14 +11,11 @@ internal struct MultiLineEditorView: View { let context: FieldEditorContext var onPopOut: ((String) -> Void)? var isExpanded = false - let resizedHeight: Binding - - @AppStorage(PreferenceKeys.rowInspectorTextFieldHeight.name, store: AppStorageEnvironment.shared.defaults) - private var startingHeight = ResizableFieldMetrics.defaultTextHeight var body: some View { ResizableEditorContainer( - height: ResizableFieldMetrics.height(resized: resizedHeight, starting: startingHeight), + column: context.columnName, + defaultHeight: ResizableFieldMetrics.defaultTextHeight, range: ResizableFieldMetrics.textHeightRange, expandedHeight: isExpanded ? ResizableFieldMetrics.expandedHeight : nil ) { diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/ResizableEditorContainer.swift b/apple/TablePro/Views/RowInspector/FieldEditors/ResizableEditorContainer.swift index 22abe7b48b..df98ea8847 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/ResizableEditorContainer.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/ResizableEditorContainer.swift @@ -3,26 +3,43 @@ // TablePro // -import AppKit import SwiftUI internal struct ResizableEditorContainer: View { - @Binding var height: Double + let column: String + let defaultHeight: Double let range: ClosedRange - - /// Set while the field is expanded in place. It wins over the dragged height for as long as it - /// lasts and never overwrites it, so collapsing returns the editor to the size the user chose. + // Wins over the dragged height without overwriting it, so collapsing returns to the user's size. var expandedHeight: Double? - + @ObservedObject var heights: InspectorFieldHeightStore @ViewBuilder let content: () -> Content + @Environment(\.isEnabled) private var isEnabled @State private var liveDelta: Double = 0 - @State private var isHandleHovered = false + + init( + column: String, + defaultHeight: Double, + range: ClosedRange, + expandedHeight: Double? = nil, + heights: InspectorFieldHeightStore = .shared, + @ViewBuilder content: @escaping () -> Content + ) { + self.column = column + self.defaultHeight = defaultHeight + self.range = range + self.expandedHeight = expandedHeight + self.heights = heights + self.content = content + } + + private var baseHeight: Double { + heights.height(for: column) ?? defaultHeight + } private var resolvedHeight: Double { - let dragged = ResizableFieldMetrics.resolve(base: height, delta: liveDelta, range: range) - /// Never below what the user already dragged. The allowed ranges reach 600, so a field - /// enlarged past the expanded height would otherwise shrink when asked to expand. + let dragged = ResizableFieldMetrics.resolve(base: baseHeight, delta: liveDelta, range: range) + // The ranges reach 600, so a field dragged past the expanded height must not shrink on Expand. if let expandedHeight { return max(expandedHeight, dragged) } return dragged } @@ -35,24 +52,24 @@ internal struct ResizableEditorContainer: View { resizeHandle } } + .onChange(of: isEnabled) { enabled in + if !enabled { liveDelta = 0 } + } } private var resizeHandle: some View { - Capsule() - .fill(Color(nsColor: .tertiaryLabelColor)) - .frame(width: 26, height: 4) - .opacity(isHandleHovered ? 1 : 0.5) - .frame(maxWidth: .infinity, minHeight: 11) - .overlay { - FieldResizeHandle( - onHover: { isHandleHovered = $0 }, - onDrag: { liveDelta = $0 }, - onEnd: { delta in - height = ResizableFieldMetrics.resolve(base: height, delta: delta, range: range) - liveDelta = 0 - } - ) + FieldResizeHandle( + onDrag: { liveDelta = $0 }, + onEnd: { delta in + if let height = ResizableFieldMetrics.committedHeight(base: baseHeight, delta: delta, range: range) { + heights.setHeight(height, for: column) + } + liveDelta = 0 } - .accessibilityHidden(true) + ) + .frame(maxWidth: .infinity) + .frame(height: 11) + .onDisappear { liveDelta = 0 } + .accessibilityHidden(true) } } diff --git a/apple/TablePro/Views/RowInspector/FieldEditors/ResizableFieldMetrics.swift b/apple/TablePro/Views/RowInspector/FieldEditors/ResizableFieldMetrics.swift index c3d7be33aa..00be373360 100644 --- a/apple/TablePro/Views/RowInspector/FieldEditors/ResizableFieldMetrics.swift +++ b/apple/TablePro/Views/RowInspector/FieldEditors/ResizableFieldMetrics.swift @@ -4,7 +4,6 @@ // import Foundation -import SwiftUI internal enum ResizableFieldMetrics { static let jsonHeightRange: ClosedRange = 80...600 @@ -13,33 +12,24 @@ internal enum ResizableFieldMetrics { static let textHeightRange: ClosedRange = 60...600 static let defaultTextHeight: Double = 110 - /// MapKit hides its logo and legal link on a map under 100pt tall, and the HIG asks for both. + // MapKit hides its logo and legal link on a map under 100pt tall, and the HIG asks for both. static let geometryHeightRange: ClosedRange = 100...600 static let defaultGeometryHeight: Double = 160 - /// What a field grows to when it is expanded in place. Tall enough to be worth the gesture and - /// short enough that the fields around it are still on screen, which is the whole difference - /// between this and the full-pane takeover it replaces. + // Short enough that the fields around an expanded one stay on screen, unlike a full-pane takeover. static let expandedHeight: Double = 320 - /// The base is clamped before the drag is added. A field's height is shared by editors with - /// different floors (a geometry map and its text pane, or a column whose values switch between - /// JSON and text), and an unclamped base below the floor left the handle dead until the drag - /// had covered the difference. + // The base is clamped first: a geometry map and its text pane share one height under different floors. static func resolve(base: Double, delta: Double, range: ClosedRange) -> Double { clamp(clamp(base, to: range) + delta, to: range) } - private static func clamp(_ value: Double, to range: ClosedRange) -> Double { - min(max(value, range.lowerBound), range.upperBound) + // A drag that ends where it began stores nothing, so the field keeps following its default. + static func committedHeight(base: Double, delta: Double, range: ClosedRange) -> Double? { + delta == 0 ? nil : resolve(base: base, delta: delta, range: range) } - /// A drag sizes its own field only. Writing the per-kind default instead resized every field of - /// that kind at once; the default is where a field starts until it is dragged. - static func height(resized: Binding, starting: Double) -> Binding { - Binding( - get: { resized.wrappedValue ?? starting }, - set: { resized.wrappedValue = $0 } - ) + private static func clamp(_ value: Double, to range: ClosedRange) -> Double { + min(max(value, range.lowerBound), range.upperBound) } } diff --git a/apple/TablePro/Views/RowInspector/InspectorFieldListView.swift b/apple/TablePro/Views/RowInspector/InspectorFieldListView.swift index 8edfb869ba..c3ec4ce0c4 100644 --- a/apple/TablePro/Views/RowInspector/InspectorFieldListView.swift +++ b/apple/TablePro/Views/RowInspector/InspectorFieldListView.swift @@ -7,12 +7,7 @@ import AppKit import Combine import SwiftUI -/// The inspected row's fields. -/// -/// A `List` and not a `Form`. Measured at the pane's width: `List` is lazy and stays lazy with rows -/// of differing heights, holding 336 subviews and about 135ms to first display whether the row has -/// 100 columns or 2000. `Form(.formStyle(.grouped))` builds every row eagerly, four subviews each, -/// and costs 525ms at 2000. A wide table would have paid that on every selection change. +// A List, not a Form: measured lazy at 336 subviews and ~135ms for 100 or 2000 columns; a grouped Form costs 525ms at 2000. internal struct InspectorFieldListView: View { @ObservedObject internal var editState: MultiRowEditState internal let isEditable: Bool @@ -26,16 +21,11 @@ internal struct InspectorFieldListView: View { @State private var searchText = "" @State private var showsModifiedOnly = false - @State private var expandedColumnIndex: Int? - /// By column for the same reason as the expansion: a dragged size holds while rows change. - @State private var resizedHeights: [Int: Double] = [:] @StateObject private var tabStops = InspectorTabStopRegistry() @FocusState private var focusedField: UUID? var body: some View { - /// Resolved once per pass. `editState.fields` is observed, so the body runs on every - /// keystroke in every field, and filtering inside `List` as well as inside the keyboard - /// handlers walked the whole column list several times over for each one. + // Once per pass: the body runs on every keystroke, and filtering per use walked every column several times. let fields = visibleFields return VStack(spacing: 0) { filterBar @@ -47,17 +37,13 @@ internal struct InspectorFieldListView: View { } } .onChange(of: editState.fields.map(\.columnName)) { _ in - expandedColumnIndex = nil - resizedHeights = [:] focusedField = nil } } // MARK: - Filtering - /// Always visible rather than bolted above the list on some modes and not others. A row of two - /// hundred columns is unreadable without it, and a control that is the only way to reach a - /// field cannot be conditional. + // Always visible: on a row of two hundred columns it is the only way to reach a field. private var filterBar: some View { HStack(spacing: 6) { NativeSearchField( @@ -79,12 +65,7 @@ internal struct InspectorFieldListView: View { .padding(.vertical, 5) } - /// Whether this field accepts a change at all. - /// - /// One answer, read by the editor, by the value menu and by the keyboard shortcuts alike. A - /// PHP-serialized value cannot be round-tripped without PHP, so the field is presented but never - /// mutated: every Set Value command would overwrite the serialized payload with something the app - /// cannot rebuild. Bytes decoded under a binary format have no writer back either. + // One answer for the editor, the value menu and the shortcuts. PHP-serialized and decoded binary values have no writer back. internal static func isFieldEditable( _ field: FieldEditState, kind: FieldEditorKind, @@ -138,9 +119,7 @@ internal struct InspectorFieldListView: View { )) } } - /// `.plain`, not `.inset`: the inset style spends a hard 16pt per side before any - /// `listRowInsets` is consulted, so the pane's content sat at 24pt while its own header sat - /// at 10. `.plain` lands at half of `intercellSpacing` and can be corrected onto the edge. + // `.plain`, not `.inset`: the inset style spends 16pt per side before `listRowInsets`, which no inset can undo. .listStyle(.plain) .splitItemGlassBackground() .modifier(InspectorFieldKeyShortcuts( @@ -160,18 +139,13 @@ internal struct InspectorFieldListView: View { isModified: field.hasEdit || field.hasCommittedEdit, isPrimaryKey: field.isPrimaryKey, isForeignKey: field.isForeignKey, - isExpanded: Self.isExpanded(field, expandedColumnIndex: expandedColumnIndex), - resizedHeight: Self.resizedHeight(of: field, in: $resizedHeights), + isExpanded: editState.expandedColumn == field.columnIndex, commands: commands, isPendingExpression: field.isPendingExpression, onSetValue: { editState.setValue($0, at: field.columnIndex) }, onRemoveField: offersFieldRemoval ? { editState.removeField(at: field.columnIndex) } : nil, onToggleExpand: FieldEditorContent.canExpand(kind: kind, state: FieldValueState.resolve(field)) - ? { - expandedColumnIndex = Self.isExpanded(field, expandedColumnIndex: expandedColumnIndex) - ? nil - : field.columnIndex - } + ? { editState.toggleExpansion(ofColumn: field.columnIndex) } : nil, onPopOut: { onPopOut?(field, $0, kind) }, onTabStopChange: { [tabStops] in tabStops.record($0, for: field.id) }, @@ -180,20 +154,6 @@ internal struct InspectorFieldListView: View { ) } - /// Keyed by column, not by field id: ids are reissued on every selection change, so a field - /// expanded on one row collapsed the moment the next row was selected. - internal static func isExpanded(_ field: FieldEditState, expandedColumnIndex: Int?) -> Bool { - field.columnIndex == expandedColumnIndex - } - - internal static func resizedHeight(of field: FieldEditState, in heights: Binding<[Int: Double]>) -> Binding { - let column = field.columnIndex - return Binding( - get: { heights.wrappedValue[column] }, - set: { heights.wrappedValue[column] = $0 } - ) - } - private func context( for field: FieldEditState, kind: FieldEditorKind, @@ -205,11 +165,7 @@ internal struct InspectorFieldListView: View { columnName: field.columnName, columnType: field.columnTypeEnum, isLongText: field.isLongText, - /// The getter reads the store, not the `state` resolved above. That value is a copy - /// taken when this context was built, so a getter closing over it answers whatever the - /// field held during that render, and an `onChange` action, which belongs to the - /// render that registered it, is a render behind again. An editor comparing its text - /// against that answer sees the value it had just replaced and puts it back (#3051). + // Reads the store, not `state`: a copy from this render made editors put back the value just replaced (#3051). value: isEditable ? Binding( get: { editState.currentText(at: columnIndex) }, @@ -229,21 +185,19 @@ internal struct InspectorFieldListView: View { // MARK: - Keyboard - /// Tab is intercepted because the key view loop inside a SwiftUI `List` has one stop, measured. - /// Ignored at either end so the key falls through and focus can leave the list, not wrap. + // A List's key view loop has one stop, measured. Ignored at either end so focus leaves the list instead of wrapping. private func moveFocus(within fields: [FieldEditState], forward: Bool) -> KeyPressResultCompat { guard !fields.isEmpty else { return .ignored } let order = fields.map(\.id) tabStops.forget(allBut: order) - /// With Full Keyboard Access on, the controls of a field showing a map are key views. + // With Full Keyboard Access on, the controls of a field showing a map are key views. let skipped = NSApplication.shared.isFullKeyboardAccessEnabled ? [] : tabStops.fieldsWithoutTabStop let destination = Self.focusDestination(from: focusedField, in: order, forward: forward, skipping: skipped) focusedField = destination return destination == nil ? .ignored : .handled } - /// The field Tab lands on, or nil to let the key leave the list. A field with nothing to focus - /// is passed over, so focus is never sent where no view can take it. + // Nil lets the key leave the list. A field with nothing to focus is passed over. internal static func focusDestination( from current: UUID?, in order: [UUID], @@ -273,8 +227,7 @@ internal struct InspectorFieldListView: View { return .handled } - /// Control-Option-N and Control-Option-D do only what the field's menu offers. Unhandled, the key goes on - /// to the next responder rather than staging a NULL the column refuses. + // Only what the field's menu offers; otherwise the key goes on rather than staging a NULL the column refuses. internal static func stateShortcutCommand(for key: Character, offered: [SetValueCommand]) -> SetValueCommand? { let command: SetValueCommand switch key { @@ -286,8 +239,7 @@ internal struct InspectorFieldListView: View { } } -/// Fields whose editor holds nothing Tab can land on. A reference that publishes nothing, so a -/// field reporting in does not redraw the list. +// Fields with nothing Tab can land on. Publishes nothing, so a field reporting in does not redraw the list. @MainActor internal final class InspectorTabStopRegistry: ObservableObject { internal private(set) var fieldsWithoutTabStop: Set = [] @@ -300,15 +252,13 @@ internal final class InspectorTabStopRegistry: ObservableObject { } } - /// Field ids are reissued on every selection change, so the old ones are dropped. + // Field ids are reissued on every selection change. internal func forget(allBut fieldIDs: [UUID]) { fieldsWithoutTabStop.formIntersection(fieldIDs) } } -/// `onKeyPress` is macOS 14, and `KeyPress` cannot appear outside the check, so the handlers -/// are gated as a pair. Tab still moves focus on macOS 13 through the responder chain; what is -/// lost there is the Control-Option state shortcut, which the field's own menu also offers. +// `onKeyPress` is macOS 14. On 13 Tab still moves focus through the responder chain; the field's menu covers the shortcuts. private struct InspectorFieldKeyShortcuts: ViewModifier { let moveFocus: (Bool) -> KeyPressResultCompat let applyStateShortcut: (Character) -> KeyPressResultCompat @@ -332,7 +282,7 @@ private struct InspectorFieldKeyShortcuts: ViewModifier { } } -/// Mirrors `KeyPress.Result`, which is macOS 14, so the handlers can be declared outside the check. +// Mirrors macOS 14's `KeyPress.Result`, so the handlers can be declared outside the check. internal enum KeyPressResultCompat { case handled case ignored diff --git a/apple/TablePro/Views/RowInspector/InspectorFieldRow.swift b/apple/TablePro/Views/RowInspector/InspectorFieldRow.swift index f8fbb09648..20ca42288f 100644 --- a/apple/TablePro/Views/RowInspector/InspectorFieldRow.swift +++ b/apple/TablePro/Views/RowInspector/InspectorFieldRow.swift @@ -15,7 +15,6 @@ internal struct InspectorFieldRow: View { internal let isPrimaryKey: Bool internal let isForeignKey: Bool internal let isExpanded: Bool - internal let resizedHeight: Binding internal let commands: [SetValueCommand] internal var isPendingExpression = false internal let onSetValue: (SetValueCommand) -> Void @@ -170,7 +169,6 @@ internal struct InspectorFieldRow: View { FieldEditorContent( context: context, kind: kind, - resizedHeight: resizedHeight, onSetNull: context.canMutate && commands.contains(.null) ? { onSetValue(.null) } : nil, onSetDefault: context.canMutate && commands.contains(.defaultValue) ? { onSetValue(.defaultValue) } : nil, onPopOut: onPopOut, diff --git a/apple/TableProTests/Core/Storage/AppSettingsStorageMigrationTests.swift b/apple/TableProTests/Core/Storage/AppSettingsStorageMigrationTests.swift index f35a5bd28d..2b3771f3ac 100644 --- a/apple/TableProTests/Core/Storage/AppSettingsStorageMigrationTests.swift +++ b/apple/TableProTests/Core/Storage/AppSettingsStorageMigrationTests.swift @@ -2,7 +2,7 @@ // AppSettingsStorageMigrationTests.swift // TableProTests // -// Tests the one-time startup-behavior migration to "Reopen Last Session". +// Tests the startup-behavior migration and the retired inspector field heights. // import Foundation @@ -67,39 +67,38 @@ struct AppSettingsStorageMigrationTests { #expect(storage.loadGeneral().startupBehavior == .showWelcome) } - private let legacyJsonHeightKey = "rightSidebar.jsonFieldHeight" + private let retiredFieldHeightKeys = [ + "rightSidebar.jsonFieldHeight", + "com.TablePro.rightSidebar.jsonFieldHeight", + "com.TablePro.rightSidebar.textFieldHeight", + "com.TablePro.rightSidebar.geometryFieldHeight", + "com.TablePro.settings.didMigrateJsonFieldHeightKey", + ] - @Test("A saved legacy JSON field height moves to the namespaced key") - func migratesLegacyJsonFieldHeight() { + @Test("The per-kind field heights an older build saved are removed") + func removesRetiredFieldHeights() { let (storage, defaults, suite) = makeStorage() defer { defaults.removePersistentDomain(forName: suite) } - defaults.set(320.0, forKey: legacyJsonHeightKey) - storage.migrateJsonFieldHeightKeyIfNeeded() + for key in retiredFieldHeightKeys { + defaults.set(320.0, forKey: key) + } + storage.removeRetiredFieldHeightKeys() - #expect(defaults.double(forKey: PreferenceKeys.rowInspectorJsonFieldHeight.name) == 320.0) - #expect(defaults.object(forKey: legacyJsonHeightKey) == nil) + for key in retiredFieldHeightKeys { + #expect(defaults.object(forKey: key) == nil, "\(key) survived") + } } - @Test("A fresh install writes no JSON field height key") - func skipsJsonFieldHeightOnFreshInstall() { + @Test("Removing the per-kind heights leaves the per-column ones") + func keepsPerColumnFieldHeights() { let (storage, defaults, suite) = makeStorage() defer { defaults.removePersistentDomain(forName: suite) } - storage.migrateJsonFieldHeightKeyIfNeeded() + let saved = Data("[]".utf8) + defaults.set(saved, forKey: PreferenceKeys.rowInspectorFieldHeights.name) + storage.removeRetiredFieldHeightKeys() - #expect(defaults.object(forKey: PreferenceKeys.rowInspectorJsonFieldHeight.name) == nil) - } - - @Test("An existing namespaced JSON field height is not overwritten") - func leavesNamespacedJsonFieldHeight() { - let (storage, defaults, suite) = makeStorage() - defer { defaults.removePersistentDomain(forName: suite) } - - defaults.set(200.0, forKey: PreferenceKeys.rowInspectorJsonFieldHeight.name) - defaults.set(320.0, forKey: legacyJsonHeightKey) - storage.migrateJsonFieldHeightKeyIfNeeded() - - #expect(defaults.double(forKey: PreferenceKeys.rowInspectorJsonFieldHeight.name) == 200.0) + #expect(defaults.data(forKey: PreferenceKeys.rowInspectorFieldHeights.name) == saved) } } diff --git a/apple/TableProTests/Core/Storage/AppSettingsStorageResetTests.swift b/apple/TableProTests/Core/Storage/AppSettingsStorageResetTests.swift index 433da457a9..3a986cf84a 100644 --- a/apple/TableProTests/Core/Storage/AppSettingsStorageResetTests.swift +++ b/apple/TableProTests/Core/Storage/AppSettingsStorageResetTests.swift @@ -16,29 +16,23 @@ struct AppSettingsStorageResetTests { let storage = AppSettingsStorage(userDefaults: defaults) defaults.set("account", forKey: PreferenceKeys.selectedSettingsPane.name) defaults.set("tree", forKey: SidebarPersistenceKey.defaultLayout) - defaults.set(320.0, forKey: PreferenceKeys.rowInspectorJsonFieldHeight.name) storage.resetToDefaults() #expect(defaults.string(forKey: PreferenceKeys.selectedSettingsPane.name) == nil) #expect(defaults.string(forKey: SidebarPersistenceKey.defaultLayout) == nil) - #expect(defaults.object(forKey: PreferenceKeys.rowInspectorJsonFieldHeight.name) == nil) } @Test("Reset clears every remembered inspector field height and the geometry field's mode") func resetClearsInspectorFieldPreferences() throws { let defaults = try pool.make().defaults let storage = AppSettingsStorage(userDefaults: defaults) - defaults.set(320.0, forKey: PreferenceKeys.rowInspectorJsonFieldHeight.name) - defaults.set(240.0, forKey: PreferenceKeys.rowInspectorTextFieldHeight.name) - defaults.set(400.0, forKey: PreferenceKeys.rowInspectorGeometryFieldHeight.name) + defaults.set(Data("[]".utf8), forKey: PreferenceKeys.rowInspectorFieldHeights.name) defaults.set("text", forKey: PreferenceKeys.rowInspectorGeometryFieldMode.name) storage.resetToDefaults() - #expect(defaults.object(forKey: PreferenceKeys.rowInspectorJsonFieldHeight.name) == nil) - #expect(defaults.object(forKey: PreferenceKeys.rowInspectorTextFieldHeight.name) == nil) - #expect(defaults.object(forKey: PreferenceKeys.rowInspectorGeometryFieldHeight.name) == nil) + #expect(defaults.object(forKey: PreferenceKeys.rowInspectorFieldHeights.name) == nil) #expect(defaults.object(forKey: PreferenceKeys.rowInspectorGeometryFieldMode.name) == nil) } } diff --git a/apple/TableProTests/Core/Storage/InspectorFieldHeightStoreTests.swift b/apple/TableProTests/Core/Storage/InspectorFieldHeightStoreTests.swift new file mode 100644 index 0000000000..0bd2edb500 --- /dev/null +++ b/apple/TableProTests/Core/Storage/InspectorFieldHeightStoreTests.swift @@ -0,0 +1,80 @@ +// +// InspectorFieldHeightStoreTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +@MainActor +struct InspectorFieldHeightStoreTests { + private let scratch: EphemeralDefaults + + init() throws { + scratch = try EphemeralDefaults() + } + + @Test("A height belongs to its column, and other columns keep their default") + func heightIsPerColumn() { + let store = InspectorFieldHeightStore(defaults: scratch.defaults) + + store.setHeight(300, for: "payload") + + #expect(store.height(for: "payload") == 300) + #expect(store.height(for: "metadata") == nil) + } + + @Test("A height survives a relaunch") + func heightSurvivesRelaunch() { + InspectorFieldHeightStore(defaults: scratch.defaults).setHeight(300, for: "payload") + + #expect(InspectorFieldHeightStore(defaults: scratch.defaults).height(for: "payload") == 300) + } + + @Test("Setting a column again replaces its height") + func heightIsReplaced() { + let store = InspectorFieldHeightStore(defaults: scratch.defaults) + + store.setHeight(300, for: "payload") + store.setHeight(180, for: "payload") + + #expect(store.height(for: "payload") == 180) + #expect(InspectorFieldHeightStore(defaults: scratch.defaults).heights == ["payload": 180]) + } + + @Test("Past capacity the least recently set column is dropped") + func evictsLeastRecentlySet() { + let store = InspectorFieldHeightStore(defaults: scratch.defaults) + for index in 0.. FieldEditState { - FieldEditState( - columnIndex: column, - columnName: "payload", - columnTypeEnum: .text(rawType: "JSON"), - isLongText: false, - isJson: true, - hasMultipleValues: false, - pendingValue: nil, - isPendingNull: false, - isPendingDefault: false - ) - } - - /// Selecting another row builds its fields afresh, each with a new id. - @Test("An expanded field stays expanded on the next row") - func expansionSurvivesSelectionChange() { - let first = field(column: 2) - let next = field(column: 2) - #expect(first.id != next.id) - #expect(InspectorFieldListView.isExpanded(first, expandedColumnIndex: 2)) - #expect(InspectorFieldListView.isExpanded(next, expandedColumnIndex: 2)) - } - - @Test("Only the expanded column is expanded") - func otherColumnsStayCollapsed() { - #expect(InspectorFieldListView.isExpanded(field(column: 3), expandedColumnIndex: 2) == false) - #expect(InspectorFieldListView.isExpanded(field(column: 2), expandedColumnIndex: nil) == false) - } - - /// Dragging one field once resized every field of its kind, and the size it was given went - /// away with the row. - @Test("A dragged height belongs to its column and outlives the row") - func resizedHeightIsPerColumn() { - var heights: [Int: Double] = [:] - let store = Binding(get: { heights }, set: { heights = $0 }) - - InspectorFieldListView.resizedHeight(of: field(column: 2), in: store).wrappedValue = 300 - - #expect(InspectorFieldListView.resizedHeight(of: field(column: 2), in: store).wrappedValue == 300) - #expect(InspectorFieldListView.resizedHeight(of: field(column: 3), in: store).wrappedValue == nil) - } - // MARK: - Geometry private func geometry(_ editor: GeometryTextEditor, _ source: GeometryFieldSource) -> FieldEditorKind { diff --git a/apple/TableProTests/Views/RowInspector/FieldResizeHandleTests.swift b/apple/TableProTests/Views/RowInspector/FieldResizeHandleTests.swift index ffd077c034..736d9ecb9f 100644 --- a/apple/TableProTests/Views/RowInspector/FieldResizeHandleTests.swift +++ b/apple/TableProTests/Views/RowInspector/FieldResizeHandleTests.swift @@ -2,23 +2,25 @@ // FieldResizeHandleTests.swift // TableProTests // -// The handle owns the mouse from press to release, so the drag is measured from the press and -// the release always reports where it ended. -// import AppKit import SwiftUI @testable import TablePro +import TableProTestSupport import Testing @MainActor struct FieldResizeHandleTests { - private let window = NSWindow( - contentRect: NSRect(x: 0, y: 0, width: 200, height: 400), - styleMask: [.titled], - backing: .buffered, - defer: true - ) + private let window: NSWindow = { + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 300, height: 400), + styleMask: [.titled], + backing: .buffered, + defer: true + ) + window.isReleasedWhenClosed = false + return window + }() private func event(_ type: NSEvent.EventType, y: CGFloat) -> NSEvent { NSEvent.mouseEvent( @@ -34,85 +36,176 @@ struct FieldResizeHandleTests { )! } + private func recordingHandle() -> (FieldResizeHandleView, Recorder) { + let view = FieldResizeHandleView() + let recorder = Recorder() + view.onDrag = { recorder.drags.append($0) } + view.onEnd = { recorder.ends.append($0) } + return (view, recorder) + } + @Test("Dragging down reports a growing delta, and the release reports the final one") func reportsDownwardDrag() { - let view = FieldResizeHandleView() - var drags: [Double] = [] - var ended: Double? - view.onDrag = { drags.append($0) } - view.onEnd = { ended = $0 } + let (view, recorder) = recordingHandle() view.mouseDown(with: event(.leftMouseDown, y: 300)) view.mouseDragged(with: event(.leftMouseDragged, y: 290)) view.mouseDragged(with: event(.leftMouseDragged, y: 260)) view.mouseUp(with: event(.leftMouseUp, y: 250)) - #expect(drags == [10, 40]) - #expect(ended == 50) + #expect(recorder.drags == [10, 40]) + #expect(recorder.ends == [50]) } @Test("Dragging up reports a negative delta") func reportsUpwardDrag() { - let view = FieldResizeHandleView() - var ended: Double? - view.onEnd = { ended = $0 } + let (view, recorder) = recordingHandle() view.mouseDown(with: event(.leftMouseDown, y: 300)) + view.mouseDragged(with: event(.leftMouseDragged, y: 320)) view.mouseUp(with: event(.leftMouseUp, y: 330)) - #expect(ended == -30) + #expect(recorder.ends == [-30]) } - /// The press has to land on the AppKit view, not on SwiftUI's hosting view: a gesture there lost - /// the mouse to the inspector's table a few points into the drag. - @Test("A press on the grip lands on the handle view") - func pressLandsOnHandle() throws { - let container = ResizableEditorContainer(height: .constant(120), range: 80...600) { - Color.clear - } - let hosting = NSHostingView(rootView: container) - hosting.frame = NSRect(x: 0, y: 0, width: 200, height: 200) - window.contentView = hosting - hosting.layoutSubtreeIfNeeded() + @Test("A click on the grip resizes nothing") + func clickReportsNothing() { + let (view, recorder) = recordingHandle() - let handle = try #require(Self.firstSubview(of: FieldResizeHandleView.self, in: hosting)) - let center = handle.convert(NSPoint(x: handle.bounds.midX, y: handle.bounds.midY), to: nil) - let root = try #require(hosting.superview) + view.mouseDown(with: event(.leftMouseDown, y: 300)) + view.mouseUp(with: event(.leftMouseUp, y: 300)) - #expect(root.hitTest(root.convert(center, from: nil)) === handle) + #expect(recorder.drags.isEmpty) + #expect(recorder.ends.isEmpty) } - private static func firstSubview(of type: View.Type, in view: NSView) -> View? { - for subview in view.subviews { - if let match = subview as? View ?? firstSubview(of: type, in: subview) { return match } - } - return nil + @Test("A drag that did not start on the handle reports nothing") + func ignoresStrayEvents() { + let (view, recorder) = recordingHandle() + + view.mouseDragged(with: event(.leftMouseDragged, y: 200)) + view.mouseUp(with: event(.leftMouseUp, y: 200)) + + #expect(recorder.drags.isEmpty) + #expect(recorder.ends.isEmpty) } - @Test("A handle removed under the pointer stops reporting a hover") - func removalEndsHover() { - let view = FieldResizeHandleView() - var hovers: [Bool] = [] - view.onHover = { hovers.append($0) } + @Test("A disabled handle takes no press, no hover and no hit") + func disabledHandleIsInert() { + let (view, recorder) = recordingHandle() + view.frame = NSRect(x: 0, y: 0, width: 100, height: 11) + view.isEnabled = false + + view.mouseEntered(with: event(.mouseMoved, y: 5)) + view.mouseDown(with: event(.leftMouseDown, y: 300)) + view.mouseDragged(with: event(.leftMouseDragged, y: 250)) + view.mouseUp(with: event(.leftMouseUp, y: 250)) + + #expect(view.isHighlighted == false) + #expect(view.hitTest(NSPoint(x: 50, y: 5)) == nil) + #expect(recorder.ends.isEmpty) + } + + @Test("Disabling the handle mid-drag drops the drag") + func disablingCancelsDrag() { + let (view, recorder) = recordingHandle() + + view.mouseDown(with: event(.leftMouseDown, y: 300)) + view.mouseDragged(with: event(.leftMouseDragged, y: 280)) + view.isEnabled = false + view.mouseUp(with: event(.leftMouseUp, y: 250)) + + #expect(recorder.ends.isEmpty) + #expect(view.isHighlighted == false) + } + + @Test("A handle removed mid-drag ends neither the drag nor the hover later") + func removalDropsDragAndHover() { + let (view, recorder) = recordingHandle() window.contentView?.addSubview(view) view.mouseEntered(with: event(.mouseMoved, y: 10)) + view.mouseDown(with: event(.leftMouseDown, y: 300)) + view.mouseDragged(with: event(.leftMouseDragged, y: 280)) view.removeFromSuperview() + view.mouseUp(with: event(.leftMouseUp, y: 250)) - #expect(hovers == [true, false]) + #expect(recorder.ends.isEmpty) + #expect(view.isHighlighted == false) } - /// A release with no press on the handle belongs to some other drag. - @Test("A drag that did not start on the handle reports nothing") - func ignoresStrayEvents() { - let view = FieldResizeHandleView() - var reported = false - view.onDrag = { _ in reported = true } - view.onEnd = { _ in reported = true } + private func mountInList(_ heights: InspectorFieldHeightStore) async throws -> FieldResizeHandleView { + let list = List { + ResizableEditorContainer(column: "payload", defaultHeight: 120, range: 80...600, heights: heights) { + Color.clear + } + } + .listStyle(.plain) + let hosting = NSHostingView(rootView: list) + hosting.frame = NSRect(x: 0, y: 0, width: 300, height: 400) + window.contentView = hosting + try await waitUntil { + hosting.layoutSubtreeIfNeeded() + hosting.displayIfNeeded() + return Self.firstSubview(of: FieldResizeHandleView.self, in: hosting) != nil + } + return try #require(Self.firstSubview(of: FieldResizeHandleView.self, in: hosting)) + } - view.mouseDragged(with: event(.leftMouseDragged, y: 200)) - view.mouseUp(with: event(.leftMouseUp, y: 200)) + private func windowEvent(_ type: NSEvent.EventType, at point: NSPoint) -> NSEvent { + NSEvent.mouseEvent( + with: type, + location: point, + modifierFlags: [], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + eventNumber: 0, + clickCount: 1, + pressure: 1 + )! + } - #expect(reported == false) + // The table took the mouse from a SwiftUI gesture in its row; the press must land on the AppKit view instead. + @Test("Inside a List, a press on the grip lands on the handle view") + func pressInListLandsOnHandle() async throws { + let scratch = try EphemeralDefaults() + let handle = try await mountInList(InspectorFieldHeightStore(defaults: scratch.defaults)) + defer { window.contentView = nil } + + let center = handle.convert(NSPoint(x: handle.bounds.midX, y: handle.bounds.midY), to: nil) + let root = try #require(handle.window?.contentView?.superview) + #expect(root.hitTest(root.convert(center, from: nil)) === handle) + } + + // Driven on the view: an offscreen test window is never key, so `sendEvent` delivers no mouse event. + @Test("Inside a List, a drag released off the handle stores the column's height") + func dragInListStoresHeight() async throws { + let scratch = try EphemeralDefaults() + let heights = InspectorFieldHeightStore(defaults: scratch.defaults) + let handle = try await mountInList(heights) + defer { window.contentView = nil } + + let start = handle.convert(NSPoint(x: handle.bounds.midX, y: handle.bounds.midY), to: nil) + let end = NSPoint(x: start.x + 120, y: start.y - 40) + handle.mouseDown(with: windowEvent(.leftMouseDown, at: start)) + handle.mouseDragged(with: windowEvent(.leftMouseDragged, at: NSPoint(x: start.x, y: start.y - 20))) + handle.mouseDragged(with: windowEvent(.leftMouseDragged, at: end)) + handle.mouseUp(with: windowEvent(.leftMouseUp, at: end)) + + #expect(heights.height(for: "payload") == 160) + #expect(InspectorFieldHeightStore(defaults: scratch.defaults).height(for: "payload") == 160) } + + private static func firstSubview(of type: View.Type, in view: NSView) -> View? { + for subview in view.subviews { + if let match = subview as? View ?? firstSubview(of: type, in: subview) { return match } + } + return nil + } +} + +private final class Recorder { + var drags: [Double] = [] + var ends: [Double] = [] } diff --git a/apple/TableProTests/Views/RowInspector/InspectorValueThemeTests.swift b/apple/TableProTests/Views/RowInspector/InspectorValueThemeTests.swift index 3305a04f4c..dae89e5df5 100644 --- a/apple/TableProTests/Views/RowInspector/InspectorValueThemeTests.swift +++ b/apple/TableProTests/Views/RowInspector/InspectorValueThemeTests.swift @@ -33,7 +33,7 @@ struct InspectorValueThemeTests { } private func field(_ value: String, kind: FieldEditorKind, state: FieldValueState? = nil) -> FieldEditorContent { - FieldEditorContent(context: context(value, state: state), kind: kind, resizedHeight: .constant(nil)) + FieldEditorContent(context: context(value, state: state), kind: kind) } private func pixel(of view: some View, size: CGSize, x: Int, fromBottom: Int? = nil) throws -> NSColor { @@ -85,7 +85,7 @@ struct InspectorValueThemeTests { valueState: .value(value.wrappedValue), isReadOnly: false ) - return FieldEditorContent(context: context, kind: kind, resizedHeight: .constant(nil)) + return FieldEditorContent(context: context, kind: kind) } private func editableField(_ value: String, kind: FieldEditorKind = .singleLine) -> FieldEditorContent { diff --git a/apple/TableProTests/Views/RowInspector/ResizableFieldMetricsTests.swift b/apple/TableProTests/Views/RowInspector/ResizableFieldMetricsTests.swift index 6d98af9f3d..eddb12388a 100644 --- a/apple/TableProTests/Views/RowInspector/ResizableFieldMetricsTests.swift +++ b/apple/TableProTests/Views/RowInspector/ResizableFieldMetricsTests.swift @@ -1,4 +1,3 @@ -import SwiftUI import Testing @testable import TablePro @@ -46,27 +45,17 @@ struct ResizableFieldMetricsTests { #expect(ResizableFieldMetrics.resolve(base: 160, delta: 5_000, range: mapRange) == 600) } - /// A text pane dragged to 60 and then shown as a 100pt map: the drag moves it from 100 at once. - @Test func testABaseBelowTheFloorMovesWithTheFirstPointOfDrag() { + // A text pane dragged to 60 and then shown as a 100pt map: the drag moves it from 100 at once. + @Test func testABaseOutsideTheRangeMovesWithTheFirstPointOfDrag() { let mapRange = ResizableFieldMetrics.geometryHeightRange #expect(ResizableFieldMetrics.resolve(base: 60, delta: 30, range: mapRange) == 130) #expect(ResizableFieldMetrics.resolve(base: 700, delta: -50, range: mapRange) == 550) } - /// Every field of a kind once shared one stored height, so dragging one resized them all. - @Test func testDraggingOneFieldLeavesTheOthersAtTheirStartingHeight() { - var resized: [Int: Double] = [:] - func height(for column: Int) -> Binding { - ResizableFieldMetrics.height( - resized: Binding(get: { resized[column] }, set: { resized[column] = $0 }), - starting: 120 - ) - } - - height(for: 1).wrappedValue = 300 - - #expect(height(for: 1).wrappedValue == 300) - #expect(height(for: 2).wrappedValue == 120) + @Test func testADragThatEndsWhereItBeganStoresNothing() { + #expect(ResizableFieldMetrics.committedHeight(base: 120, delta: 0, range: range) == nil) + #expect(ResizableFieldMetrics.committedHeight(base: 120, delta: 40, range: range) == 160) + #expect(ResizableFieldMetrics.committedHeight(base: 600, delta: 40, range: range) == 600) } @Test func testTextHeightResolvesWithinItsOwnRange() { diff --git a/apple/changelog.d/3975.changed.md b/apple/changelog.d/3975.changed.md new file mode 100644 index 0000000000..8637f18f2a --- /dev/null +++ b/apple/changelog.d/3975.changed.md @@ -0,0 +1 @@ +A dragged field in the row inspector keeps its height per column, not for every field of its kind. diff --git a/apple/changelog.d/3975.fixed.md b/apple/changelog.d/3975.fixed.md index 228e200024..f0c149de81 100644 --- a/apple/changelog.d/3975.fixed.md +++ b/apple/changelog.d/3975.fixed.md @@ -1 +1 @@ -Dragging a field's resize handle in the row inspector snapped back, or resized every field of that kind. +Dragging a field's resize handle in the row inspector snapped back a few points into the drag.