Selection menu: system actions and an app hook - #88
PSchmiedmayer wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The macOS selection action menu dispatch can misbehave due to id collisions/array lookup and currently surfaces actions even without a non-collapsed selection.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR expands Textual’s text selection feature to (1) restore the platform-provided selection menu items on iOS, (2) let apps append their own selection actions (receiving the selected plain text), and (3) improve selection robustness in dynamic/lazy layouts while enabling selection coordination across multiple text views.
Changes:
- Add
textSelectionActions(_:)andTextSelectionActionto allow app-defined menu actions for selected text (iOS + macOS). - Fix iOS menu suppression by deferring unknown selectors to
super.canPerformAction. - Improve selection reliability in streaming/lazy layouts (size-driven layout collection invalidation), fix empty-paragraph crashes in positioning, and add subtree-wide selection coordination.
File summaries
| File | Description |
|---|---|
| Sources/Textual/View+Textual.swift | Adds public view modifiers for selection actions and selection coordination. |
| Sources/Textual/TextSelectionAction.swift | Introduces the TextSelectionAction public type and environment entry. |
| Sources/Textual/Internal/TextInteraction/UIKit/UITextInteractionView.swift | Restores iOS system selection items and inserts app actions into the edit menu. |
| Sources/Textual/Internal/TextInteraction/UIKit/UIKitTextInteractionOverlay.swift | Plumbs textSelectionActions environment into the UIKit interaction view. |
| Sources/Textual/Internal/TextInteraction/Shared/TextSelectionInteraction.swift | Makes layout overlay re-evaluate when content size changes to support lazy/streaming text. |
| Sources/Textual/Internal/TextInteraction/Shared/TextSelectionCoordinator.swift | Enables reusing an inherited coordinator so a subtree shares one selection. |
| Sources/Textual/Internal/TextInteraction/Shared/TextLayout/View+TextLayoutCollection.swift | Adds size-based invalidation (.id(size)) to force layout overlay refresh. |
| Sources/Textual/Internal/TextInteraction/Shared/TextLayout/TextLayoutCollection+Positioning.swift | Hardens positioning/range logic for empty paragraphs and adds safe indexing helpers. |
| Sources/Textual/Internal/TextInteraction/Shared/TextLayout/LiveTextLayoutCollection.swift | Updates equality to include geometry size to distinguish measurement passes in lazy layouts. |
| Sources/Textual/Internal/TextInteraction/AppKit/NSTextInteractionView.swift | Appends app selection actions to the macOS context menu and dispatches handlers. |
| Sources/Textual/Internal/TextInteraction/AppKit/AppKitTextInteractionOverlay.swift | Plumbs textSelectionActions environment into the AppKit interaction view. |
Review details
Suppressed comments (1)
Sources/Textual/Internal/TextInteraction/AppKit/NSTextInteractionView.swift:307
performSelectionAction(_:)looks up the action by id inselectionActions, which can mis-dispatch when ids collide (the defaultidistitle) and can also fail if the environment updates between menu creation and click. If the menu item stores theTextSelectionActionitself inrepresentedObject, the handler can be invoked reliably without requiring unique ids or array lookups. This is also a good place to require a non-collapsed selection to match iOS behavior.
@objc private func performSelectionAction(_ sender: NSMenuItem) {
guard let selectedRange = model.selectedRange,
let action = selectionActions.first(where: { $0.id == sender.representedObject as? String })
else {
return
}
action.handler(Formatter(model.attributedText(in: selectedRange)).plainText())
}
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
b5413f4 to
a1758b5
Compare
a1758b5 to
c08495f
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new public Identifiable default for TextSelectionAction risks id collisions, and the iOS menu-building path does avoidable UI-thread work by eagerly formatting selected text.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
Sources/Textual/TextSelectionAction.swift:24
TextSelectionActionconforms toIdentifiable, but defaultingidtotitlecan easily create duplicate identities (e.g. two actions with the same title), which breaks theIdentifiablecontract and can cause SwiftUI diffing issues in user code.
Consider generating a unique default id when none is provided (and still allow callers to pass a stable id when needed).
Sources/Textual/Internal/TextInteraction/Shared/TextLayout/View+TextLayoutCollection.swift:18
- The last sentence of this doc comment is unclear/grammatically incorrect ("Reading the size here makes them"), which makes the rationale harder to follow.
Suggest rewording to explicitly state that size is used to force the overlay to re-evaluate.
- Files reviewed: 11/12 changed files
- Comments generated: 1
- Review effort level: Lite
Selecting text in a
StructuredTexton iOS gives you a menu with Copy and nothing else.canPerformActionreturnsfalsefor every selector it does not know, which also hides Look Up, Translate, Search Web and Share. And there is no way for an app to put its own item on that menu.This PR adds
textual.textSelectionActions(_:). ATextSelectionActionis a title, an optional SF Symbol and a closure that gets the selected plain text. On iOS the items are inserted inbuildMenu(with:), so UIKit still owns the menu; on macOS they are appended to theNSMenuthe view already builds. Unknown selectors now go tosuper, which brings the system items back. Once Copy or one of the app's actions has run, the selection goes away on iOS, like it does in Messages. On macOS an action clears it and Copy keeps it, as in any text view.On iOS 27 the system files its writing assistant entry on the first page of the menu, and that page is short enough that an app's own item can land behind the overflow chevron.
textSelectionActions(_:prominence:)takes aTextSelectionActionProminence:.standardleaves the menu as the system builds it,.prominentputs the app's actions right after Copy and moves the assistant entry to the end, at every level of the menu. The entry is recognised by its selector,showWritingTools(_:), not by a private identifier. On macOS the two cases read the same.I ran into three bugs while using selection in a chat that streams messages into a
LazyVStack, fixed in separate commits:Text.LayoutKeysits in aGeometryReader, and SwiftUI does not re-run it when only the size changes. Text that grows after its first pass keeps the layouts of that pass. In a lazy container that pass has no width and the layouts have no lines, so hit testing never finds anything and a long press selects nothing. The overlay now takes the content size as a dependency andLiveTextLayoutCollectioncompares it as well.beginningOfDocument,endOfDocumentandlocalCharacterRange(at:)crash on an empty paragraph. They now use the first and last slices that exist.StructuredTexthad its ownTextSelectionCoordinator, so selecting in one view left the selection in another in place.textual.textSelectionCoordination()puts one coordinator on a subtree and aStructuredTextbelow it joins that one.A selection now lives the way it does in a text view: Copy, Look Up and the app's own items leave it standing, and it ends when the view resigns first responder (a text field taking focus, a sheet, navigating away) or when a tap lands anywhere else. On iOS the view keeps a tap recognizer on its window only while it has a selection; it cancels nothing and clears only for taps outside the view's bounds, so tapping the selected text to bring the menu back still works. On macOS a local mouse-down monitor does the same.
The iOS test target had stopped compiling: the snapshot helper called
overlayTextLayoutCollectionwithout thesizeargument this PR adds, and on Xcode 27 the chained.jsonresolved to thejson(_:)overload. Both are fixed in the helper.Tests: 140 pass on macOS and 164 on an iOS 26.5 simulator, including four for the UIKit view and two for the AppKit view. On the iOS 27.0 runtime the
twoParagraphsBidiStructuredTextLayoutsnapshot differs by one run boundary around the isolate terminator; it does so onmainas well and is left alone. The menu and the streaming fix were checked on an iOS 26 simulator with a UI test in the app this came from (long press, menu shows Copy / the app item / Look Up / Translate / Search Web / Share, the app item receives the text). No snapshot test for the menu itself since it is UIKit's.