diff --git a/Docs/editor-investigation.md b/Docs/editor-investigation.md index 0b0c968..9355f9e 100644 --- a/Docs/editor-investigation.md +++ b/Docs/editor-investigation.md @@ -1372,6 +1372,41 @@ Current conclusion: The live editor path now has regression coverage for the bugs that escaped Milestone 3.4. Future presentation claims should be based on live harness tests or manual app verification, not only semantic render snapshots or direct styler tests. +## Finding #21 — Initial Checklist Overlay Geometry + +The remaining live presentation bug was not in Markdown parsing or rendered task construction. The source document produced the correct rendered task elements, the checkbox controls existed, and the task label text had the correct attributes. The divergence was in overlay materialization. + +Root cause: + +```mermaid +sequenceDiagram + participant SwiftUI + participant Scroll as ComfortableEditorScrollView + participant TextKit + participant Coord as Coordinator + participant Widgets as Checklist overlays + + SwiftUI->>Coord: Initial full render + Coord->>TextKit: Apply rendered checklist attributes + Coord->>Widgets: Position checkbox overlays + SwiftUI->>Scroll: First real layout width/insets + Scroll->>TextKit: Text container origin changes + TextKit->>TextKit: Label glyph geometry updates + Widgets-->>Widgets: Overlay frames stay stale +``` + +Checklist labels are ordinary TextKit glyphs, so they automatically follow the final scroll-view width and readable-column inset. Checklist checkboxes are overlay `NSButton` controls, so their frames must be re-materialized whenever the text container origin or scroll-view layout size changes. Before this fix, the initial render positioned buttons using the pre-layout geometry. The first focus/unfocus sequence repaired the view because it re-entered `applyHybridAttributes`, and that path rebuilt checklist overlay frames after the scroll view already had final layout geometry. + +Corrected invalidation path: + +- `ComfortableEditorScrollView.layout()` now detects content-size or inset changes. +- Layout changes call `syncChecklistControlFrames(in:)`. +- Frame sync reuses existing `RenderedTaskElement` values and only updates overlay frames. +- Markdown styling, render-model construction, and source state are not rerun. +- Checkbox frame measurement explicitly ensures TextKit layout for the measured checkbox range before reading glyph bounds. + +The live regression `testInitialChecklistOverlayTracksFirstLiveLayoutPass` reproduces the escaped bug by creating the real editor harness, recording checkbox-to-label spacing, applying the first realistic scroll-view layout change, and verifying that every checklist overlay still tracks its label. Before the fix, the label gap changed from the initial rendered value to a different post-layout value because the label moved with the text container while the button did not. + ## AttributedString and NSAttributedString Swift `AttributedString` is useful for renderer-facing APIs and SwiftUI previews. diff --git a/Sources/SaplingEditor/HybridMarkdownEditor.swift b/Sources/SaplingEditor/HybridMarkdownEditor.swift index 807a45c..108b9dd 100644 --- a/Sources/SaplingEditor/HybridMarkdownEditor.swift +++ b/Sources/SaplingEditor/HybridMarkdownEditor.swift @@ -231,6 +231,9 @@ private struct NativeMarkdownTextView: NSViewRepresentable { scrollView.documentView = textView scrollView.editorTextView = textView + scrollView.onEditorLayoutInvalidated = { [weak coordinator = context.coordinator] textView in + coordinator?.syncChecklistControlFrames(in: textView) + } scrollView.updateEditorInsets() context.coordinator.applyHybridAttributes(to: textView) return scrollView @@ -452,6 +455,12 @@ private struct NativeMarkdownTextView: NSViewRepresentable { checklistButtons[task.lineIndex] = button } + syncChecklistControlFrames(in: textView) + } + + func syncChecklistControlFrames(in textView: NSTextView) { + guard !checklistButtons.isEmpty else { return } + for (lineIndex, button) in Array(checklistButtons) { guard let task = button.task, let frame = checklistFrame(for: task, in: textView) @@ -502,6 +511,7 @@ private struct NativeMarkdownTextView: NSViewRepresentable { else { return nil } let characterRange = NSRange(location: task.checkboxRange.location, length: 1) + layoutManager.ensureLayout(forCharacterRange: characterRange) let glyphRange = layoutManager.glyphRange(forCharacterRange: characterRange, actualCharacterRange: nil) guard glyphRange.length > 0 else { return nil } @@ -614,14 +624,24 @@ private final class ChecklistOverlayButton: NSButton { private final class ComfortableEditorScrollView: NSScrollView { weak var editorTextView: NSTextView? + var onEditorLayoutInvalidated: ((NSTextView) -> Void)? + private var lastLayoutSize: NSSize = .zero override func layout() { super.layout() - updateEditorInsets() + let didChangeInsets = updateEditorInsets() + + guard let editorTextView else { return } + let layoutSize = contentView.bounds.size + if didChangeInsets || layoutSize != lastLayoutSize { + lastLayoutSize = layoutSize + onEditorLayoutInvalidated?(editorTextView) + } } - func updateEditorInsets() { - guard let editorTextView else { return } + @discardableResult + func updateEditorInsets() -> Bool { + guard let editorTextView else { return false } let readableWidth: CGFloat = 760 let horizontalInset = max(36, floor((contentView.bounds.width - readableWidth) / 2)) @@ -629,7 +649,9 @@ private final class ComfortableEditorScrollView: NSScrollView { if editorTextView.textContainerInset != targetInset { editorTextView.textContainerInset = targetInset + return true } + return false } } @@ -646,7 +668,11 @@ public final class HybridMarkdownLiveEditorHarness { private let scrollView: ComfortableEditorScrollView private let textView: EditorTextView - public init(source: String, selectedRange: NSRange = NSRange(location: 0, length: 0)) { + public init( + source: String, + selectedRange: NSRange = NSRange(location: 0, length: 0), + initialWidth: CGFloat = 640 + ) { let stateBox = StateBox(text: source, selection: EditorSelection(range: selectedRange)) self.text = source self.selection = EditorSelection(range: selectedRange) @@ -698,7 +724,10 @@ public final class HybridMarkdownLiveEditorHarness { layoutManager.addTextContainer(textContainer) textStorage.addLayoutManager(layoutManager) - self.textView = EditorTextView(frame: NSRect(x: 0, y: 0, width: 640, height: 480), textContainer: textContainer) + self.textView = EditorTextView( + frame: NSRect(x: 0, y: 0, width: initialWidth, height: 480), + textContainer: textContainer + ) self.textView.autoresizingMask = [.width] self.textView.minSize = NSSize(width: 0, height: 480) self.textView.maxSize = NSSize(width: CGFloat.greatestFiniteMagnitude, height: CGFloat.greatestFiniteMagnitude) @@ -726,15 +755,18 @@ public final class HybridMarkdownLiveEditorHarness { self.textView.insertionPointColor = .controlAccentColor self.textView.font = .systemFont(ofSize: 16, weight: .regular) self.textView.textContainer?.widthTracksTextView = true - self.textView.textContainer?.containerSize = NSSize(width: 640, height: CGFloat.greatestFiniteMagnitude) + self.textView.textContainer?.containerSize = NSSize(width: initialWidth, height: CGFloat.greatestFiniteMagnitude) - self.scrollView.frame = NSRect(x: 0, y: 0, width: 640, height: 480) + self.scrollView.frame = NSRect(x: 0, y: 0, width: initialWidth, height: 480) self.scrollView.documentView = textView self.scrollView.editorTextView = textView + self.scrollView.onEditorLayoutInvalidated = { [weak coordinator] textView in + coordinator?.syncChecklistControlFrames(in: textView) + } self.scrollView.updateEditorInsets() self.window = NSWindow( - contentRect: NSRect(x: 0, y: 0, width: 640, height: 480), + contentRect: NSRect(x: 0, y: 0, width: initialWidth, height: 480), styleMask: [.titled], backing: .buffered, defer: false @@ -767,6 +799,16 @@ public final class HybridMarkdownLiveEditorHarness { syncState() } + public func simulateLayout(width: CGFloat) { + window.setContentSize(NSSize(width: width, height: 480)) + scrollView.frame = NSRect(x: 0, y: 0, width: width, height: 480) + textView.frame = NSRect(x: 0, y: 0, width: width, height: textView.frame.height) + textView.textContainer?.containerSize = NSSize(width: width, height: CGFloat.greatestFiniteMagnitude) + scrollView.layoutSubtreeIfNeeded() + scrollView.layout() + syncState() + } + public func headingMarkerIsHidden() -> Bool { isHidden(at: 0) } @@ -804,6 +846,22 @@ public final class HybridMarkdownLiveEditorHarness { checklistButton(lineIndex: lineIndex)?.frame } + public func checklistAlignmentDelta(lineIndex: Int) -> CGFloat? { + guard let buttonFrame = checklistButtonFrame(lineIndex: lineIndex), + let labelFrame = checklistLabelFrame(lineIndex: lineIndex) + else { return nil } + + return abs(buttonFrame.midY - labelFrame.midY) + } + + public func checklistLabelGap(lineIndex: Int) -> CGFloat? { + guard let buttonFrame = checklistButtonFrame(lineIndex: lineIndex), + let labelFrame = checklistLabelFrame(lineIndex: lineIndex) + else { return nil } + + return labelFrame.minX - buttonFrame.maxX + } + public func selectedRange() -> NSRange { textView.selectedRange() } @@ -820,6 +878,30 @@ public final class HybridMarkdownLiveEditorHarness { textView.subviews.compactMap { $0 as? ChecklistOverlayButton }.first { $0.task?.lineIndex == lineIndex } } + private func checklistLabelFrame(lineIndex: Int) -> CGRect? { + guard let task = checklistButton(lineIndex: lineIndex)?.task, + task.contentRange.location < textView.string.utf16.count, + let layoutManager = textView.layoutManager, + let textContainer = textView.textContainer + else { return nil } + + layoutManager.ensureLayout(for: textContainer) + let glyphRange = layoutManager.glyphRange( + forCharacterRange: NSRange(location: task.contentRange.location, length: 1), + actualCharacterRange: nil + ) + guard glyphRange.length > 0 else { return nil } + + let fragment = layoutManager.lineFragmentRect(forGlyphAt: glyphRange.location, effectiveRange: nil) + let origin = textView.textContainerOrigin + return CGRect( + x: origin.x + fragment.minX, + y: origin.y + fragment.minY, + width: fragment.width, + height: fragment.height + ) + } + private func isHidden(at location: Int) -> Bool { guard let textStorage = textView.textStorage else { return false } let color = textStorage.attribute(.foregroundColor, at: location, effectiveRange: nil) as? NSColor diff --git a/Tests/SaplingEditorTests/HybridMarkdownLiveEditorHarnessTests.swift b/Tests/SaplingEditorTests/HybridMarkdownLiveEditorHarnessTests.swift index ddbf719..71b5fcd 100644 --- a/Tests/SaplingEditorTests/HybridMarkdownLiveEditorHarnessTests.swift +++ b/Tests/SaplingEditorTests/HybridMarkdownLiveEditorHarnessTests.swift @@ -62,5 +62,38 @@ final class HybridMarkdownLiveEditorHarnessTests: XCTestCase { XCTAssertEqual(checkboxFrameBefore.size.width, checkboxFrameAfter.size.width, accuracy: 0.001) XCTAssertEqual(checkboxFrameBefore.size.height, checkboxFrameAfter.size.height, accuracy: 0.001) } + + func testInitialChecklistOverlayTracksFirstLiveLayoutPass() throws { + let source = """ + ## Navigation Checklist + + Use this section for quick keyboard testing. + + * [ ] Move with arrow keys. + * [ ] Jump by word with Option + Arrow. + * [ ] Extend selection with Shift + Arrow. + * [ ] Select across multiple lines. + * [ ] Use Home, End, Page Up, and Page Down. + * [ ] Type into the active line after moving quickly. + """ + let harness = HybridMarkdownLiveEditorHarness(source: source, initialWidth: 640) + let initialGaps = try Dictionary(uniqueKeysWithValues: (4...9).map { lineIndex in + (lineIndex, try XCTUnwrap(harness.checklistLabelGap(lineIndex: lineIndex))) + }) + + harness.simulateLayout(width: 900) + + for lineIndex in 4...9 { + let alignmentDelta = try XCTUnwrap(harness.checklistAlignmentDelta(lineIndex: lineIndex)) + let labelGap = try XCTUnwrap(harness.checklistLabelGap(lineIndex: lineIndex)) + XCTAssertLessThan(alignmentDelta, 8, "Checklist overlay for line \(lineIndex) is not aligned with its label") + XCTAssertEqual( + labelGap, + initialGaps[lineIndex] ?? labelGap, + accuracy: 0.001, + "Checklist overlay for line \(lineIndex) did not track the label through the first layout pass" + ) + } + } } #endif