Text Highlight: Update Highlighted Text Behavior#339
Merged
Conversation
* Add `showHighlightedTextInput` Defaults property. This handles the permanent showing/hiding of the `inputView`. * `inputView` can now be permanently hidden. * Update settings "General" tab to allow for toggling of `showHighlightedTextInput` boolean, which determines the show/hide behavior of `inputView`. * FileRow now shows highlighted text as a ContextTag. Clicking on the ContextTag toggles `showHighlightedTextInput` to `true`, revealing the `inputView` if it were hidden.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Greptile Summary
This PR introduces a significant enhancement to the text highlighting feature by adding persistent visibility control for the inputView. The changes span across several files to implement two key mechanisms for controlling the input view's visibility:
- A new close button ("x") in the
inputViewtitle section - A toggle switch in the Settings > General tab
The implementation also introduces a new UI pattern where highlighted text appears as a context tag in FileRow, positioned between pending and added window context items. This improves the discoverability and interaction model for highlighted text.
The changes are well-structured across the codebase:
- Adds persistent state management via
Defaults.Keys - Implements UI controls in both the input view and settings
- Maintains consistent styling and follows existing patterns
Confidence score: 5/5
- This PR is extremely safe to merge as it follows existing patterns and primarily adds UI control features
- The changes are well-isolated, properly implemented, and don't modify any critical application logic
- Key files to review:
InputButtons.swiftandGeneralTab.swiftfor the new UI controls
4 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
Comment on lines
210
to
211
| text: pendingInput.selectedText, | ||
| iconView: Image(.text).addIconStyles(iconSize: 14) |
Contributor
There was a problem hiding this comment.
logic: Since this is dark mode only app, text should have an explicit foregroundColor or foregroundStyle set
timlenardo
requested changes
Jul 16, 2025
Contributor
There was a problem hiding this comment.
Hey @lk340 - thanks so much for splitting up the original PR. This one is MUCH easier to review.
I have two small change requests:
- Can you update the buttons in InputButtons per my comments in Slack?
- There's an edge case the occurs when the text that you've highlighted starts with a newline. The logic that you have to determine what gets displayed in the tag uses the empty characters, and you end up with an empty tag. Here's an screenshot illustrating the issue:
Can you update it to ignore empty lines and whitespace? We should call something like '.trim()' on the input string to remove leading whitespace, and also ignore new lines inside the string when creating the preview.
* Fix whitespace and new lines being included in highlighted text. * Remove copy and close buttons in InputTitle. Caret button no longer toggles height of `inputView`; it now handles removing the `inputView` from the UI (previous close button behavior). * Because the caret button no longer toggles the height of `inputView`, height toggle logic has been removed.
This was referenced Jul 17, 2025
timlenardo
requested changes
Jul 17, 2025
Contributor
timlenardo
left a comment
timlenardo
left a comment
There was a problem hiding this comment.
Two changes requested:
- We can't strip the newlines and spaces in AccessibilityNotificationManager- we want to send the unedited text to the LLM. Instead, we should do that in the views that display them.
- We need to make InputView and InputButtons work in FinalContextView, too.
Regarding your questions in Slack:
- x vs carrot - let's keep the carrot since it's more descriptive in the context of FinalContextView.
- contextTag vs. inputView - By 'preview' I meant the tag. I don't think we need to remove spaces in the InputView itself.
- replace multi-spaces with single space - Since it's getting complex, I would recommend only removing spaces for the context tag as discussed in point 2. Then you don't have to deal with this issue. In the full InputView, the spaces are helpful, particularly when it's code.
macos/Onit/Accessibility/Notifications/AccessibilityNotificationsManager.swift
Outdated
Show resolved
Hide resolved
* Add toggle feature to auto-adding of highlighted text to context. * Add new `autoAddHighlightedTextToContext` boolean property to Defaults store. Property defaults to `true`. * Add new switch button to settings "General" tab to toggle `autoAddHighlightedTextToContext` boolean. * Add new `trackedPendingInput` property to OnitPanelState to track highlighted text when `autoAddHighlightedTextToContext` is `false`. * When `autoAddHighlightedTextToContext` is `false`, highlighting text reveals the "ghost" context tag in FileRow to manually add highlighted text to context. * When `autoAddHighlightedTextToContext` is `true`, the app maintains previous highlight behavior (auto-adding to context). If some text was already highlighted, this switch will automatically add the highlighted text to context. * Update highlighted text ContextTag to have a remove action for highlighted text context. * Update to show both ghost tags in FileRow.
* Remove trimming on the `processSelectedText()` level and apply it only on the context tags level instead. * Add new StringHelpers struct for reusable removable of whitespace + new lines. * Update ordering of context tags in FileRow. * Update hiding of InputView to the PromptCore level. InputView can now be seen in the FinalContextView, regardless of show state. * Update InputView to be a collapsible element when in FinalContextView.
* Increase max height of InputView. * `highlightedTextContext` button now shows border when InputView is active. Update ContextTag to make this type of update more reusable. * Update `inputString` output in InputTitle. * Update background, borders and spacing around InputView. * InputView is now inside of PromptCore to reflect design updates. * Update spacing in PromptCore.
timlenardo
approved these changes
Jul 18, 2025
Contributor
timlenardo
left a comment
timlenardo
left a comment
There was a problem hiding this comment.
This looks great, thanks for tackling all this @lk340 ⭐️
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
2 participants
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.
This is part 1 in a series of mini-PRs that tackle various aspects of the overarching text highlight context experience.
Please review this first before reviewing all other text highlight PRs.
Part 2: #340
Summary
inputViewcan now be permanently shown or hidden.inputViewis permanently hidden when closing it through the newly-added cross ("x") button in theinputViewtitle section.FileRow.inputView.inputViewshowing/hiding can also be toggled in the settings "General" tab, under the "Highlighted Text" section.Parts (Pin 1 and Pin 2 have been closed)
YOU ARE HERE