fix(editor): preserve viewport during presentation transitions
This commit is contained in:
parent
d12a0a58ed
commit
95be4348cd
3 changed files with 253 additions and 7 deletions
|
|
@ -1547,6 +1547,62 @@ Future container affordances:
|
|||
- Line numbers are feasible as a body gutter drawn by `EditorTextView`, but should be deferred until wrapping and selection behavior are validated.
|
||||
- These affordances do not require changing the Markdown source model; they are presentation-layer additions.
|
||||
|
||||
## Finding #25 — Presentation Stability
|
||||
|
||||
Milestone 3.8 addresses viewport movement during rendered/source transitions. The remaining visible jump was not caused by the render model; it came from preserving the scroll view's numeric origin while TextKit changed the geometry of the line being edited.
|
||||
|
||||
Root cause:
|
||||
|
||||
- Rendered headings use larger fonts and heading paragraph spacing.
|
||||
- Source headings use monospaced source styling and active-line background.
|
||||
- Rendered code blocks use hidden fence lines, container spacing, and monospaced body text.
|
||||
- Source code blocks reveal the whole fenced block.
|
||||
- `applyHybridAttributes` restored the previous `NSClipView.bounds.origin`, but the edited glyph could move relative to that origin after attributes changed.
|
||||
- Programmatic selection also called `scrollRangeToVisible` even when the selection was already visible, which could pre-scroll before the presentation transition ran.
|
||||
|
||||
Corrected stabilization model:
|
||||
|
||||
```mermaid
|
||||
flowchart LR
|
||||
Before["Before styling"] --> Capture["Capture source location + viewport-relative glyph Y"]
|
||||
Capture --> Style["Apply rendered/source attributes"]
|
||||
Style --> Relayout["TextKit lays out changed character range"]
|
||||
Relayout --> Restore["Scroll so same source location has same viewport-relative Y"]
|
||||
```
|
||||
|
||||
The anchor is source-based, not element-specific. `ViewportPresentationAnchor` captures a character location and its viewport-relative Y position before a presentation transition. After styling, the coordinator asks TextKit for the same source location's new glyph position and scrolls the clip view by the delta. This applies equally to headings, task lists, code blocks, tables, and future rendered widgets because the mechanism does not inspect the rendered element type during restoration.
|
||||
|
||||
Anchor selection:
|
||||
|
||||
- The selected source location is the default anchor.
|
||||
- If selection is on hidden or structural syntax for headings, lists, blockquotes, or tasks, the anchor moves to the visible content range for that line.
|
||||
- Programmatic selection changes only scroll when the target point is outside the visible viewport; visible targets preserve the existing origin.
|
||||
|
||||
Geometry audit:
|
||||
|
||||
| Element | Rendered geometry | Source geometry | Stability handling |
|
||||
| --- | --- | --- | --- |
|
||||
| Heading | Larger font and heading spacing | Monospaced active line | Anchor selected heading text |
|
||||
| Task list | Hidden list marker, overlay checkbox | Raw marker and checkbox syntax | Anchor selected task content when syntax is selected |
|
||||
| Code block | Hidden fences, rendered container, code body | Full fenced source block | Anchor selected code/fence source location |
|
||||
| Paragraph inline markup | Hidden delimiters, styled content | Raw Markdown delimiters | Anchor selected source location |
|
||||
|
||||
Validation:
|
||||
|
||||
- `testHeadingTransitionKeepsEditedContentVisuallyAnchored` reproduced the heading jump and now verifies the heading text keeps the same viewport-relative Y position when entering and leaving source mode.
|
||||
- `testCodeBlockTransitionKeepsEditedContentVisuallyAnchored` verifies the same contract for entering and leaving a rendered code block.
|
||||
- The tests use `HybridMarkdownLiveEditorHarness`, so they exercise the same `NSTextView`, coordinator, TextKit layout, and scroll-view path as the app.
|
||||
|
||||
Performance impact:
|
||||
|
||||
- Anchor measurement uses `NSLayoutManager.ensureLayout(forCharacterRange:)` on the anchor character rather than full-document layout.
|
||||
- Dirty-line styling remains unchanged.
|
||||
- The anchoring work only runs when a presentation pass is already required.
|
||||
|
||||
Known boundary:
|
||||
|
||||
At the extreme end of a document, the scroll origin may still clamp if the document height changes and there is no remaining scrollable space below the viewport. That is a natural scroll-view constraint rather than interaction-history nondeterminism.
|
||||
|
||||
## AttributedString and NSAttributedString
|
||||
|
||||
Swift `AttributedString` is useful for renderer-facing APIs and SwiftUI previews.
|
||||
|
|
|
|||
|
|
@ -324,7 +324,11 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
|
|||
guard invalidationPlan.requiresStyling else { return }
|
||||
|
||||
let selectedRange = textView.selectedRange()
|
||||
let visibleOrigin = textView.enclosingScrollView?.contentView.bounds.origin
|
||||
let viewportAnchor = ViewportPresentationAnchor.capture(
|
||||
in: textView,
|
||||
selectedRange: selectedRange,
|
||||
lineIndex: currentLineIndex
|
||||
)
|
||||
let start = Date()
|
||||
var stylingResult = MarkdownTextStylingResult.empty
|
||||
var didRestoreVisibleOrigin = false
|
||||
|
|
@ -346,8 +350,8 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
|
|||
selectedRange.location <= textView.string.utf16.count {
|
||||
textView.setSelectedRange(selectedRange)
|
||||
}
|
||||
didRestoreVisibleOrigin = restoreVisibleOrigin(visibleOrigin, in: textView)
|
||||
}
|
||||
didRestoreVisibleOrigin = restoreViewportAnchor(viewportAnchor, in: textView)
|
||||
|
||||
lastStyledText = textView.string
|
||||
lastStyledActiveLineIndex = activeLineIndex
|
||||
|
|
@ -387,11 +391,26 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
|
|||
func setSelection(_ range: NSRange, in textView: NSTextView) {
|
||||
guard textView.selectedRange() != range else { return }
|
||||
performProgrammaticUpdate {
|
||||
let visibleOrigin = textView.enclosingScrollView?.contentView.bounds.origin
|
||||
let shouldScroll = selectionNeedsScroll(range, in: textView)
|
||||
textView.setSelectedRange(range)
|
||||
textView.scrollRangeToVisible(range)
|
||||
if shouldScroll {
|
||||
textView.scrollRangeToVisible(range)
|
||||
} else if let visibleOrigin {
|
||||
_ = scrollVisibleOrigin(visibleOrigin, in: textView)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private func selectionNeedsScroll(_ range: NSRange, in textView: NSTextView) -> Bool {
|
||||
guard let point = textView.presentationAnchorPoint(at: range.location),
|
||||
let scrollView = textView.enclosingScrollView
|
||||
else { return true }
|
||||
|
||||
let visibleRect = scrollView.contentView.bounds.insetBy(dx: 0, dy: 12)
|
||||
return !visibleRect.contains(point)
|
||||
}
|
||||
|
||||
func performProgrammaticUpdate(_ updates: () -> Void) {
|
||||
programmaticUpdateDepth += 1
|
||||
defer {
|
||||
|
|
@ -577,11 +596,27 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
|
|||
checklistButtons.removeAll()
|
||||
}
|
||||
|
||||
private func restoreVisibleOrigin(_ origin: NSPoint?, in textView: NSTextView) -> Bool {
|
||||
guard let origin,
|
||||
let scrollView = textView.enclosingScrollView
|
||||
else { return false }
|
||||
private func restoreViewportAnchor(
|
||||
_ anchor: ViewportPresentationAnchor?,
|
||||
in textView: NSTextView
|
||||
) -> Bool {
|
||||
guard let anchor,
|
||||
let point = textView.presentationAnchorPoint(at: anchor.characterLocation)
|
||||
else {
|
||||
return false
|
||||
}
|
||||
|
||||
return scrollVisibleOrigin(
|
||||
NSPoint(
|
||||
x: anchor.visibleOrigin.x,
|
||||
y: point.y - anchor.viewportOffsetY
|
||||
),
|
||||
in: textView
|
||||
)
|
||||
}
|
||||
|
||||
private func scrollVisibleOrigin(_ origin: NSPoint, in textView: NSTextView) -> Bool {
|
||||
guard let scrollView = textView.enclosingScrollView else { return false }
|
||||
let maxY = max(0, textView.bounds.height - scrollView.contentView.bounds.height)
|
||||
let maxX = max(0, textView.bounds.width - scrollView.contentView.bounds.width)
|
||||
let clampedOrigin = NSPoint(
|
||||
|
|
@ -600,6 +635,62 @@ private struct CodeBlockContainerPresentation: Equatable {
|
|||
var languageLabel: String
|
||||
}
|
||||
|
||||
private struct ViewportPresentationAnchor {
|
||||
var characterLocation: Int
|
||||
var visibleOrigin: NSPoint
|
||||
var viewportOffsetY: CGFloat
|
||||
|
||||
static func capture(
|
||||
in textView: NSTextView,
|
||||
selectedRange: NSRange,
|
||||
lineIndex: DocumentLineIndex
|
||||
) -> ViewportPresentationAnchor? {
|
||||
guard let scrollView = textView.enclosingScrollView,
|
||||
textView.string.utf16.count > 0
|
||||
else { return nil }
|
||||
|
||||
let location = presentationAnchorLocation(
|
||||
for: selectedRange,
|
||||
lineIndex: lineIndex,
|
||||
textLength: textView.string.utf16.count
|
||||
)
|
||||
guard let point = textView.presentationAnchorPoint(at: location) else { return nil }
|
||||
|
||||
let visibleOrigin = scrollView.contentView.bounds.origin
|
||||
return ViewportPresentationAnchor(
|
||||
characterLocation: location,
|
||||
visibleOrigin: visibleOrigin,
|
||||
viewportOffsetY: point.y - visibleOrigin.y
|
||||
)
|
||||
}
|
||||
|
||||
private static func presentationAnchorLocation(
|
||||
for selectedRange: NSRange,
|
||||
lineIndex: DocumentLineIndex,
|
||||
textLength: Int
|
||||
) -> Int {
|
||||
let selectedLocation = min(max(0, selectedRange.location), max(0, textLength - 1))
|
||||
let lineNumber = lineIndex.lineIndex(containing: selectedLocation)
|
||||
guard let line = lineIndex.editorLine(at: lineNumber, activeLineIndex: lineNumber) else {
|
||||
return selectedLocation
|
||||
}
|
||||
|
||||
let renderPlan = HybridMarkdownLineRenderer().renderPlan(for: line)
|
||||
switch renderPlan.kind {
|
||||
case .heading(_, _, let textRange):
|
||||
return selectedLocation < textRange.location ? textRange.location : selectedLocation
|
||||
case .taskList(_, _, let contentRange, _, _):
|
||||
return selectedLocation < contentRange.location ? contentRange.location : selectedLocation
|
||||
case .unorderedList(_, let contentRange, _),
|
||||
.orderedList(_, let contentRange, _),
|
||||
.blockquote(_, let contentRange):
|
||||
return selectedLocation < contentRange.location ? contentRange.location : selectedLocation
|
||||
default:
|
||||
return selectedLocation
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private final class EditorTextView: NSTextView {
|
||||
var onFocusStateChange: ((NSTextView) -> Void)?
|
||||
var onUserEditingInteraction: ((NSTextView) -> Void)?
|
||||
|
|
@ -972,6 +1063,18 @@ public final class HybridMarkdownLiveEditorHarness {
|
|||
syncState()
|
||||
}
|
||||
|
||||
public func scrollViewport(toY y: CGFloat) {
|
||||
let maxY = max(0, textView.bounds.height - scrollView.contentView.bounds.height)
|
||||
let target = NSPoint(x: 0, y: max(0, min(y, maxY)))
|
||||
scrollView.contentView.scroll(to: target)
|
||||
scrollView.reflectScrolledClipView(scrollView.contentView)
|
||||
syncState()
|
||||
}
|
||||
|
||||
public func viewportOrigin() -> CGPoint {
|
||||
scrollView.contentView.bounds.origin
|
||||
}
|
||||
|
||||
public func headingMarkerIsHidden() -> Bool {
|
||||
isHidden(at: 0)
|
||||
}
|
||||
|
|
@ -1000,6 +1103,12 @@ public final class HybridMarkdownLiveEditorHarness {
|
|||
return CGPoint(x: origin.x + fragment.minX + glyphLocation.x, y: origin.y + fragment.minY + glyphLocation.y)
|
||||
}
|
||||
|
||||
public func viewportPoint(for text: String) -> CGPoint? {
|
||||
guard let point = point(for: text) else { return nil }
|
||||
let origin = viewportOrigin()
|
||||
return CGPoint(x: point.x - origin.x, y: point.y - origin.y)
|
||||
}
|
||||
|
||||
public func presentationSignature() -> String {
|
||||
guard let storage = textView.textStorage else { return "" }
|
||||
return MarkdownPresentationSnapshot.make(
|
||||
|
|
@ -1930,6 +2039,30 @@ private extension NSRange {
|
|||
}
|
||||
}
|
||||
|
||||
#if os(macOS)
|
||||
private extension NSTextView {
|
||||
func presentationAnchorPoint(at location: Int) -> NSPoint? {
|
||||
guard let layoutManager,
|
||||
string.utf16.count > 0
|
||||
else { return nil }
|
||||
|
||||
let clampedLocation = min(max(0, location), string.utf16.count - 1)
|
||||
let characterRange = NSRange(location: clampedLocation, length: 1)
|
||||
layoutManager.ensureLayout(forCharacterRange: characterRange)
|
||||
let glyphRange = layoutManager.glyphRange(
|
||||
forCharacterRange: characterRange,
|
||||
actualCharacterRange: nil
|
||||
)
|
||||
guard glyphRange.length > 0 else { return nil }
|
||||
|
||||
let fragment = layoutManager.lineFragmentRect(forGlyphAt: glyphRange.location, effectiveRange: nil)
|
||||
let glyphLocation = layoutManager.location(forGlyphAt: glyphRange.location)
|
||||
let origin = textContainerOrigin
|
||||
return NSPoint(x: origin.x + fragment.minX + glyphLocation.x, y: origin.y + fragment.minY + glyphLocation.y)
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
||||
private var platformTextBackground: Color {
|
||||
#if os(macOS)
|
||||
Color(nsColor: .textBackgroundColor)
|
||||
|
|
|
|||
|
|
@ -169,6 +169,63 @@ final class HybridMarkdownLiveEditorHarnessTests: XCTestCase {
|
|||
XCTAssertEqual(initialFrame.origin.x, restoredFrame.origin.x, accuracy: 0.001)
|
||||
XCTAssertEqual(initialFrame.size.width, restoredFrame.size.width, accuracy: 0.001)
|
||||
}
|
||||
|
||||
func testHeadingTransitionKeepsEditedContentVisuallyAnchored() throws {
|
||||
let intro = (0..<24).map { "Intro paragraph \($0)" }.joined(separator: "\n")
|
||||
let outro = (0..<24).map { "Outro paragraph \($0)" }.joined(separator: "\n")
|
||||
let source = """
|
||||
\(intro)
|
||||
# Navigation Checklist
|
||||
Body text below the heading.
|
||||
\(outro)
|
||||
"""
|
||||
let nsSource = source as NSString
|
||||
let harness = HybridMarkdownLiveEditorHarness(source: source, initialWidth: 700)
|
||||
harness.simulateLaunchFirstResponder()
|
||||
let headingPoint = try XCTUnwrap(harness.point(for: "Navigation"))
|
||||
harness.scrollViewport(toY: headingPoint.y - 160)
|
||||
let renderedViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "Navigation"))
|
||||
|
||||
harness.setSelection(NSRange(location: nsSource.range(of: "Navigation").location, length: 0))
|
||||
|
||||
let sourceViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "Navigation"))
|
||||
XCTAssertEqual(sourceViewportPoint.y, renderedViewportPoint.y, accuracy: 0.5)
|
||||
|
||||
harness.simulateFocusAway()
|
||||
|
||||
let restoredViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "Navigation"))
|
||||
XCTAssertEqual(restoredViewportPoint.y, renderedViewportPoint.y, accuracy: 0.5)
|
||||
}
|
||||
|
||||
func testCodeBlockTransitionKeepsEditedContentVisuallyAnchored() throws {
|
||||
let intro = (0..<24).map { "Intro paragraph \($0)" }.joined(separator: "\n")
|
||||
let outro = (0..<24).map { "Outro paragraph \($0)" }.joined(separator: "\n")
|
||||
let source = """
|
||||
\(intro)
|
||||
```swift
|
||||
struct Example {
|
||||
let value = 42
|
||||
}
|
||||
```
|
||||
\(outro)
|
||||
"""
|
||||
let nsSource = source as NSString
|
||||
let harness = HybridMarkdownLiveEditorHarness(source: source, initialWidth: 700)
|
||||
harness.simulateLaunchFirstResponder()
|
||||
let codePoint = try XCTUnwrap(harness.point(for: "value"))
|
||||
harness.scrollViewport(toY: codePoint.y - 180)
|
||||
let renderedViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "value"))
|
||||
|
||||
harness.setSelection(NSRange(location: nsSource.range(of: "value").location, length: 0))
|
||||
|
||||
let sourceViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "value"))
|
||||
XCTAssertEqual(sourceViewportPoint.y, renderedViewportPoint.y, accuracy: 0.5)
|
||||
|
||||
harness.simulateFocusAway()
|
||||
|
||||
let restoredViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "value"))
|
||||
XCTAssertEqual(restoredViewportPoint.y, renderedViewportPoint.y, accuracy: 0.5)
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue