Fix: find/jump scroll works without the reading column#91
Open
ChristineTham wants to merge 1 commit into
Open
Conversation
handleFindQuery's default (non-readingWidth) path fell back to NSText.scrollRangeToVisible, which is unreliable for off-screen content in a TextKit 2 text view (it routes through the absent TextKit 1 layout manager), so find/jump could not scroll to matches below the fold. Use the TextKit 2 fragment-enumeration scroll (already used for the reading column) universally. When the text view IS the document view, tv.frame.origin.y is 0, so the same math serves both layouts; scrollRangeToVisible remains only as a last-resort fallback when the layout manager/scroll view are unavailable. Enables host-side "jump to heading" (outline) and [[Note#heading]] navigation to scroll to the target, not just open the note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes find/jump scrolling in the default (non–reading-column) layout by ensuring the TextKit 2 fragment-based scroll-into-view path is used universally, avoiding NSTextView.scrollRangeToVisible behavior that can be unreliable for off-screen matches under TextKit 2.
Changes:
- Removes the
configuration.readingWidth != nilguard so the TextKit 2 fragment enumeration scroll path runs in both layouts. - Updates inline comments to document why the TextKit 2 path is preferred and why the same offset math works when the text view is the document view.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
handleFindQuery's scroll-into-view only worked when a reading column was configured. In the default (non-reading-column) layout it fell back toNSTextView.scrollRangeToVisible, which is unreliable for off-screen content in a TextKit 2 text view (it routes through the absent TextKit 1 layout manager), so find/jump did not reveal off-screen matches.Fix
Drop the
readingWidth != nilguard so the TextKit 2 fragment-enumeration scroll path (with.ensuresLayout) runs universally. When the text view is the document view,tv.frame.origin.yis 0 and the offset is a no-op, so the same math serves both layouts.~7-line change in
NativeTextViewCoordinator+Find.swift. Enables reliable scroll-to-match for in-document find and outline/heading jumps.Context: one of a few focused patches extracted from a downstream fork (ChristineTham/swift-markdown-engine) used by the HelloNotes app.