-
Notifications
You must be signed in to change notification settings - Fork 89
fix: enable link click handling in VSelectableTextView (LUM-748) #23986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚩 Delegate link handler replicates default NSTextView behavior for non-editable text views
For a non-editable
NSTextView(isEditable = false), the default behavior already opens links viaNSWorkspace.shared.open()when no delegate intercepts the click. The newtextView(_:clickedOnLink:at:)at line 249 does exactly what the default handler would do — open the URL in the default browser. This means the explicit delegate is currently a no-op from a behavioral standpoint.This is likely intentional as a foundation for future customization (e.g., intercepting specific URL schemes, adding analytics, or routing internal links differently). However, if the only goal was to "enable" link clicking, the delegate wasn't needed — links already worked in the non-editable text view. Worth confirming the intent.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The claim that the default
NSTextViewbehavior already opens links for non-editable text views is not accurate in this configuration. The user confirmed links are visually styled but completely inert (LUM-748 screenshot shows pointer cursor and underline but no click response).Several factors can prevent the default link-opening fallback from firing in practice:
NSTextViewis created with a manually constructed TextKit 1 stack (NSTextStorage→NSLayoutManager→NSTextContainer), not the default initializer. The defaultclicked(onLink:at:)fallback behavior may not be wired identically in this path.isSelectable = true+isEditable = false— in this mode, mouse events are primarily routed through the selection machinery. Link click detection depends on the text view correctly distinguishing a click-on-link from a selection gesture, which can be unreliable without an explicit delegate.devin/1774970368-lum-635-selectable-text-view) independently reached the same conclusion and used the delegate approach.The explicit delegate is the Apple-recommended pattern for handling link clicks (
NSTextViewDelegate.textView(_:clickedOnLink:at:)). It provides reliable, deterministic behavior regardless of the TextKit stack configuration, and gives us a hook for future customization (e.g., routing internal links, analytics).