feror/Sapling
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(ui): sync checklist overlays after layout

Browse source
This commit is contained in:
Feror 2026-06-01 09:56:36 +02:00
parent 78a44184f6
commit 3e7fe6ef03
3 changed files with 158 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
Docs/editor-investigation.md
View file

@ -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.

98
Sources/SaplingEditor/HybridMarkdownEditor.swift
View file

@ -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

33
Tests/SaplingEditorTests/HybridMarkdownLiveEditorHarnessTests.swift
View file

@ -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