Skip to content

Text Highlight: Pin #341

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.

Sign up for GitHub

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

Open
lk340 wants to merge 5 commits into
base: feature/text-highlight/updated-input-view-behavior
Choose a base branch
from
Open

Text Highlight: Pin #341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
49d910b
Add inputs history, chat messages builder, and OnitPanelState updates
lk340 Jul 17, 2025
2087d26
Update AccessibliityNotificationsManager and KeyboardShortcutsManager…
lk340 Jul 17, 2025
3a0dcdd
Add UI Updates
lk340 Jul 17, 2025
7e8507f
Update FileRow with pinned highlighted text updates.
lk340 Jul 17, 2025
916f84b
Add borders to unpinned and pinned context tags
lk340 Jul 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions macos/Onit/Accessibility/Notifications/AccessibilityNotificationsManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,15 +595,32 @@ class AccessibilityNotificationsManager: ObservableObject {

showDebug()
}

private func clearPendingInputsOnInvalidSelectedText() {
let currentHighlightedTextIsInView = PanelStateCoordinator.shared.state.selectedPendingInput == PanelStateCoordinator.shared.state.unpinnedPendingInput


/// Clearing `InputView` when it's showing the current highlighted text that's about to be cleared.
if currentHighlightedTextIsInView {
PanelStateCoordinator.shared.state.selectedPendingInput = nil
}

PanelStateCoordinator.shared.state.unpinnedPendingInput = nil
PanelStateCoordinator.shared.state.trackedPendingInput = nil
}

private func addSelectedTextToContextAndShowInView(_ input: Input) {
PanelStateCoordinator.shared.state.selectedPendingInput = input
PanelStateCoordinator.shared.state.unpinnedPendingInput = input
}

private func processSelectedText(_ text: String?) {
guard Defaults[.autoContextFromHighlights],
let selectedText = text,
HighlightedTextValidator.isValid(text: selectedText) else {

screenResult.userInteraction.selectedText = nil
PanelStateCoordinator.shared.state.pendingInput = nil
PanelStateCoordinator.shared.state.trackedPendingInput = nil
clearPendingInputsOnInvalidSelectedText()
return
}

Expand All @@ -612,7 +629,7 @@ class AccessibilityNotificationsManager: ObservableObject {
let input = Input(selectedText: selectedText, application: currentSource ?? "")

if Defaults[.autoAddHighlightedTextToContext] {
PanelStateCoordinator.shared.state.pendingInput = input
addSelectedTextToContextAndShowInView(input)
} else {
PanelStateCoordinator.shared.state.trackedPendingInput = input
}
Expand Down
15 changes: 15 additions & 0 deletions macos/Onit/Assets.xcassets/Icons/pin.imageset/Contents.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"images" : [
{
"filename" : "pin.svg",
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
},
"properties" : {
"preserves-vector-representation" : true
}
}
3 changes: 3 additions & 0 deletions macos/Onit/Assets.xcassets/Icons/pin.imageset/pin.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
17 changes: 11 additions & 6 deletions macos/Onit/Data/Fetching/ChatEndpointMessagesBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import Foundation
*/
struct ChatEndpointMessagesBuilder {

static func user(instructions: [String], inputs: [Input?], files: [[URL]], autoContexts: [[String: String]], webSearchContexts: [[(title: String, content: String, source: String, url: URL?)]]) -> [String] {
static func user(instructions: [String], inputs: [[Input]], files: [[URL]], autoContexts: [[String: String]], webSearchContexts: [[(title: String, content: String, source: String, url: URL?)]]) -> [String] {
var userMessages: [String] = []
for (index, instruction) in instructions.enumerated() {
var message = ""
Expand Down Expand Up @@ -46,13 +46,18 @@ struct ChatEndpointMessagesBuilder {
message += "\n\(webSearchContext.content)"
}
}

if !inputs[index].isEmpty {
let pendingInputs = inputs[index]

if let input = inputs[index], !input.selectedText.isEmpty {
message += "\n\nUse the following selected text as context. When present, selected text should take priority over other context."
Copy link
Contributor

@timlenardo timlenardo Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also update this to respect the ordering of the selected text. For example, if there's 4 highlighted text sections pinned and they send a prompt like "rephrase this", I think they're probably referring to the most recently added highlight.

So this prompt should be modified to say like "the selected text should be given priority in the order they appear", and then we need to make sure that the most recent highlighted text is added first (which probably isn't the default, since we most likely use 'append').

if let application = input.application {
message += "\n\nSelected Text from \(application): \(input.selectedText)"
} else {
message += "\n\nSelected Text: \(input.selectedText)"

for input in pendingInputs {
if let application = input.application {
message += "\n\nSelected Text from \(application): \(input.selectedText)"
} else {
message += "\n\nSelected Text: \(input.selectedText)"
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions macos/Onit/Data/Fetching/FetchingClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ actor FetchingClient {
func chat(
systemMessage: String,
instructions: [String],
inputs: [Input?],
inputs: [[Input]],
files: [[URL]],
images: [[URL]],
autoContexts: [[String: String]],
Expand Down Expand Up @@ -75,7 +75,7 @@ actor FetchingClient {
func localChat(
systemMessage: String,
instructions: [String],
inputs: [Input?],
inputs: [[Input]],
files: [[URL]],
images: [[URL]],
autoContexts: [[String: String]],
Expand Down
4 changes: 2 additions & 2 deletions macos/Onit/Data/Fetching/Streaming/StreamingClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ actor StreamingClient {

func chat(systemMessage: String,
instructions: [String],
inputs: [Input?],
inputs: [[Input]],
files: [[URL]],
images: [[URL]],
autoContexts: [[String: String]],
Expand Down Expand Up @@ -56,7 +56,7 @@ actor StreamingClient {

func localChat(systemMessage: String,
instructions: [String],
inputs: [Input?],
inputs: [[Input]],
files: [[URL]],
images: [[URL]],
autoContexts: [[String: String]],
Expand Down
6 changes: 3 additions & 3 deletions macos/Onit/Data/Persistence/Prompt.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import SwiftData

var instruction: String
var timestamp: Date
var input: Input?
var inputs: [Input]
Copy link
Contributor

@timlenardo timlenardo Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you able to get this code to run on your computer? It's insta-crashing for me. We can't change anything in any of the SwiftData model objects (Prompt, Chat, SystemPrompt, etc - search for the @model decorator) without writing a migration. In this case, transitioning from an optional Input to a non-optional input array results in a fatal error. Many of my old chats have a value of 'nil' for the 'input', which can't be converted into a non-optional array.

To reproduce the issue, checkout an older version of the app and send a prompt with no Input. Then, check out this branch and run it. You'll get a fatal error.

You can see some examples of migrations I've written in the past ('maybeUpdatePromptPriorInstructions' and 'maybeCleanupHangingPromptReferences') in SwiftDataContainer. It's a pain in the ass, but you have to do it if you want to change anything in Swift Data.

var contextList: [Context] = []

// @Relationship(deleteRule: .cascade, inverse: \Response.prompt)
Expand All @@ -30,12 +30,12 @@ import SwiftData
var generationIndex = -1

init(
instruction: String, timestamp: Date, input: Input? = nil,
instruction: String, timestamp: Date, inputs: [Input] = [],
contextList: [Context] = [], responses: [Response] = []
) {
self.instruction = instruction
self.timestamp = timestamp
self.input = input
self.inputs = inputs
self.contextList = contextList
self.responses = responses
self.generationState = GenerationState.done
Expand Down
2 changes: 1 addition & 1 deletion macos/Onit/Data/Structures/Input.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import Foundation

struct Input: Codable, Equatable {
struct Input: Codable, Equatable, Hashable {
var selectedText: String
var application: String?
}
Expand Down
4 changes: 2 additions & 2 deletions macos/Onit/KeyboardShortcuts/KeyboardShortcutsManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ struct KeyboardShortcutsManager {
if state.panel != nil {
if state.showContextMenuBrowserTabs {
state.showContextMenuBrowserTabs = false
} else if state.pendingInput != nil {
state.pendingInput = nil
} else if state.hasHighlightedText {
state.clearHighlightedTextStates()
Copy link
Contributor

@timlenardo timlenardo Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I wonder if this should only clear out the most recent, instead of all of them. It seems like it might be irritating if you hit Escape by accident and it clears your entire context window.

Without really playing with it, my intuition is that we should:

  1. Only do this if the user is in the mode where highlights get auto-added.
  2. Only do it for the automatically added highlight, not any that they've pinned.

} else {
PanelStateCoordinator.shared.closePanel()
}
Expand Down
61 changes: 49 additions & 12 deletions macos/Onit/UI/Components/ContextTag.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ struct ContextTag: View {
private let borderColor: Color?
private let iconBundleURL: URL?
private let iconView: (any View)?
private let iconViewCornerIcon: ImageResource?
Copy link
Contributor

@timlenardo timlenardo Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd probably call this the 'leftIcon' and 'rightIcon' instead of 'iconView' and 'iconViewCornerIcon'. That's what this new field is, right? It's a new right-aligned icon?

private let caption: String?
private let tooltip: String?
private let errorDotColor: Color?
private let action: (() -> Void)?
private let pinAction: (() -> Void)?
private let removeAction: (() -> Void)?

init(
Expand All @@ -41,10 +43,12 @@ struct ContextTag: View {
borderColor: Color? = nil,
iconBundleURL: URL? = nil,
iconView: (any View)? = nil,
iconViewCornerIcon: ImageResource? = nil,
caption: String? = nil,
tooltip: String? = nil,
errorDotColor: Color? = nil,
action: (() -> Void)? = nil,
pinAction: (() -> Void)? = nil,
Copy link
Contributor

@timlenardo timlenardo Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling this 'pinAction' makes it specific to pinning, which defeats the purpose of a general name like 'iconViewCornerIcon'. If we're only supporting a pin action, there's no need to pass in the CornerIcon; it can just be hardcoded. Alternatively, if we want to make this general, we should give both new fields corresponding generic names, like "rightIcon" and "rightIconAction." Either approach works for me since we have no immediate plans for anything other than pinning, but it should be one or the other.

removeAction: (() -> Void)? = nil
) {
self.text = text
Expand All @@ -60,10 +64,12 @@ struct ContextTag: View {
self.borderColor = borderColor
self.iconBundleURL = iconBundleURL
self.iconView = iconView
self.iconViewCornerIcon = iconViewCornerIcon
self.caption = caption
self.tooltip = tooltip
self.errorDotColor = errorDotColor
self.action = action
self.pinAction = pinAction
self.removeAction = removeAction
}

Expand All @@ -78,6 +84,10 @@ struct ContextTag: View {
return NSWorkspace.shared.icon(forFile: bundleUrl.path)
}

private var hasHoverActions: Bool {
pinAction != nil || removeAction != nil
}

var body: some View {
ZStack(alignment: .leading) {
HStack(alignment: .center, spacing: 6) {
Expand All @@ -103,7 +113,21 @@ struct ContextTag: View {
}

if let iconView = iconView {
AnyView(iconView)
ZStack(alignment: .bottomTrailing) {
AnyView(iconView)

if let cornerIcon = iconViewCornerIcon {
ZStack(alignment: .center) {
Circle()
.fill(isHoveredBody ? hoverBackground : background)
.frame(width: 13, height: 13)

Image(cornerIcon)
.addIconStyles(iconSize: 7.45)
}
.offset(x: 4, y: 4)
Comment on lines +119 to +128
Copy link
Contributor

@timlenardo timlenardo Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From reading the code, I can't tell the difference between this icon and the hoverActionButton added below. Do we need both?

}
}
}

if isLoading { textView.shimmering() }
Expand All @@ -120,16 +144,29 @@ struct ContextTag: View {
}
}

HStack(spacing: 0) {
Spacer()

if let removeAction = removeAction {
if hasHoverActions {
HStack(spacing: 0) {
Spacer()
FadeHorizontal(color: hoverBackground)
removeButton(removeAction)

HStack(spacing: 6) {
if let pinAction = pinAction {
hoverActionButton(icon: .pin) {
pinAction()
}
}

if let removeAction = removeAction {
hoverActionButton(icon: .cross) {
removeAction()
}
}
}
}
.frame(height: height)
.opacity(isHoveredBody ? 1 : 0)
}
.frame(height: height)
.opacity(isHoveredBody ? 1 : 0)
}
.padding(.leading, 4)
.padding(.trailing, 6)
Expand Down Expand Up @@ -185,13 +222,13 @@ extension ContextTag {
.addAnimation(dependency: [isHoveredBody, isHoveredRemove])
}

private func removeButton(_ removeAction: @escaping () -> Void) -> some View {
private func hoverActionButton(icon: ImageResource, hoverAction: @escaping () -> Void) -> some View {
Button {
removeAction()
hoverAction()
} label: {
Image(.cross)
Image(icon)
.addIconStyles(
foregroundColor: isHoveredRemove ? .white : .gray100,
foregroundColor: isHoveredRemove ? Color.primary : .gray100,
iconSize: 9
)
.addAnimation(dependency: isHoveredRemove)
Expand Down
2 changes: 1 addition & 1 deletion macos/Onit/UI/Content/ExternalTetheredButton.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ struct ExternalTetheredButton: View {
}

private var capturedHighlightedText: Bool {
return windowState?.pendingInput != nil
return windowState?.unpinnedPendingInput != nil || windowState?.trackedPendingInput != nil
}

private var foregroundWindowIcon: NSImage? {
Expand Down
16 changes: 12 additions & 4 deletions macos/Onit/UI/Panels/State/OnitPanelState+Chat.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,18 @@ import SwiftData
extension OnitPanelState {

func createAndSavePrompt(accountId: Int?) {
var inputs = pinnedPendingInputs

if let unpinnedPendingInput = unpinnedPendingInput,
!inputs.contains(unpinnedPendingInput)
{
inputs.append(unpinnedPendingInput)
}

let prompt = Prompt(
instruction: pendingInstruction,
timestamp: .now,
input: pendingInput,
inputs: inputs,
contextList: pendingContextList
)

Expand Down Expand Up @@ -62,7 +70,7 @@ extension OnitPanelState {
currentChat?.prompts.append(prompt)
currentPrompts?.append(prompt)
pendingInstruction = ""
pendingInput = nil
clearHighlightedTextStates()

do {
try container.mainContext.save()
Expand Down Expand Up @@ -91,7 +99,7 @@ extension OnitPanelState {
let curInstruction = prompt.instruction

var filesHistory: [[URL]] = [prompt.contextList.files]
var inputsHistory: [Input?] = [prompt.input]
var inputsHistory: [[Input]] = [prompt.inputs]
var imagesHistory: [[URL]] = [prompt.contextList.images]
var instructionsHistory: [String] = [curInstruction]
var autoContextsHistory: [[String: String]] = [prompt.contextList.autoContexts]
Expand All @@ -110,7 +118,7 @@ extension OnitPanelState {

if response.type != .error {
instructionsHistory.insert(currentPrompt!.instruction, at: 0)
inputsHistory.insert(currentPrompt!.input, at: 0)
inputsHistory.insert(currentPrompt!.inputs, at: 0)
filesHistory.insert(currentPrompt!.contextList.files, at: 0)
imagesHistory.insert(currentPrompt!.contextList.images, at: 0)
autoContextsHistory.insert(currentPrompt!.contextList.autoContexts, at: 0)
Expand Down
2 changes: 1 addition & 1 deletion macos/Onit/UI/Panels/State/OnitPanelState+Input.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ extension OnitPanelState {
pendingInstruction = ""
if (clearContext) {
pendingContextList.removeAll()
pendingInput = nil
clearHighlightedTextStates()
}
focusText()

Expand Down
Loading