Add FindablePanel protocol and route Cmd+F find across all panel types#6054
Add FindablePanel protocol and route Cmd+F find across all panel types#6054wowlegend wants to merge 4 commits into
Conversation
|
@wowlegend is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a FindablePanel protocol and implements find UI routing for FilePreviewPanel, MarkdownPanel, and AgentSessionPanel (text and preview modes), exposes renderer webViews, wires ProjectPanel search-focus, and updates TabManager to route Cmd+F and related actions to the focused findable panel. ChangesFind support across panels
Sequence Diagram(s)sequenceDiagram
participant User
participant TabManager
participant FocusedPanel as FindablePanel
participant NSTextView
participant WKWebView
User->>TabManager: Cmd+F (startSearch)
TabManager->>FocusedPanel: startFind()
alt panel in .text mode
FocusedPanel->>NSTextView: performTextFinderAction(show/next/prev/setSearchString)
else panel in .preview mode
FocusedPanel->>WKWebView: performFindPanelAction: (via NSApp.sendAction)
end
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (18 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryIntroduces a
Confidence Score: 5/5Safe to merge; the new find routing is purely additive and no existing find paths for terminals or browser panels are altered. The change introduces a new protocol, adds conformances to existing panels, and extends TabManager dispatch without touching any existing terminal or browser find path. The only inconsistency (sequential vs if-else in the routing helpers) is safe under the current type hierarchy because focusedBrowserPanel and focusedFindablePanel are mutually exclusive at runtime today. Sources/TabManager.swift — the find-routing helpers for findNext, findPrevious, and hideFind use a different dispatch pattern than startFind; worth aligning before BrowserPanel gains FindablePanel conformance. Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User (Cmd+F)
participant TM as TabManager
participant TP as TerminalPanel
participant BP as BrowserPanel
participant FP as FindablePanel
User->>TM: startFind()
alt TerminalPanel focused
TM->>TP: ghosttySearchFocus + search_selection
else BrowserPanel focused
TM->>BP: startFind()
BP-->>TM: "searchState != nil"
else FindablePanel focused
TM->>FP: startFind()
FP-->>TM: true/false
end
User->>TM: findNext()
alt TerminalPanel focused
TM->>TP: performBindingAction(search:next)
else BrowserPanel focused
TM->>BP: findNext()
Note right of TM: focusedFindablePanel also called (nil today)
else FindablePanel focused
TM->>FP: findNext()
end
Reviews (7): Last reviewed commit: "Address CodeRabbit architecture and docs..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @discardableResult | ||
| private func sendFindPanelAction(_ action: NSTextFinder.Action) -> Bool { | ||
| guard let webView = rendererSession.webView else { return false } | ||
| let item = NSMenuItem() | ||
| item.tag = action.rawValue | ||
| return NSApp.sendAction( | ||
| NSSelectorFromString("performFindPanelAction:"), | ||
| to: webView, | ||
| from: item | ||
| ) | ||
| } |
There was a problem hiding this comment.
sendFindPanelAction duplicates the NSMenuItem construction that textFinderSender already encapsulates. The two helpers are otherwise identical in purpose, so sendFindPanelAction should delegate to the one it already has.
| @discardableResult | |
| private func sendFindPanelAction(_ action: NSTextFinder.Action) -> Bool { | |
| guard let webView = rendererSession.webView else { return false } | |
| let item = NSMenuItem() | |
| item.tag = action.rawValue | |
| return NSApp.sendAction( | |
| NSSelectorFromString("performFindPanelAction:"), | |
| to: webView, | |
| from: item | |
| ) | |
| } | |
| @discardableResult | |
| private func sendFindPanelAction(_ action: NSTextFinder.Action) -> Bool { | |
| guard let webView = rendererSession.webView else { return false } | |
| return NSApp.sendAction( | |
| NSSelectorFromString("performFindPanelAction:"), | |
| to: webView, | |
| from: textFinderSender(action) | |
| ) | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| private func sendFindPanelAction(_ action: NSTextFinder.Action) -> Bool { | ||
| guard let webView = rendererSession.webView else { return false } | ||
| let item = NSMenuItem() | ||
| item.tag = action.rawValue | ||
| return NSApp.sendAction( | ||
| NSSelectorFromString("performFindPanelAction:"), | ||
| to: webView, | ||
| from: item | ||
| ) | ||
| } |
There was a problem hiding this comment.
hideFind for WebKit preview mode may silently fail
sendFindPanelAction(.hideFindInterface) sets item.tag = NSTextFinder.Action.hideFindInterface.rawValue (11), but WKWebView's performFindPanelAction: maps only the NSFindPanelAction values (1–10). Action code 11 has no corresponding entry in that enum, so WebKit is likely to ignore it without returning false. The practical effect is that programmatic hideFind() in markdown preview mode doesn't close the find bar, leaving it open even after the user triggers dismiss (e.g., via Escape key routing through TabManager.hideFind).
| private func textFinderSender(_ action: NSTextFinder.Action) -> NSMenuItem { | ||
| let item = NSMenuItem() | ||
| item.tag = action.rawValue | ||
| return item | ||
| } |
There was a problem hiding this comment.
textFinderSender is duplicated across panels
FilePreviewPanel defines an identical private func textFinderSender(_ action: NSTextFinder.Action) -> NSMenuItem at line 1075. This helper is now copy-pasted in two production classes. A shared extension on NSMenuItem or a free function in a FindSupport file would eliminate the drift risk if the NSMenuItem setup ever needs to change (e.g., adding keyEquivalent or a title for accessibility).
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/TabManager.swift (1)
2380-2382:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
isFindVisibleis now stale for newly supported panel types.Line 2380 still reports visibility only for terminal/browser find state. After adding File Preview and Markdown find routing, this can misreport active find UI state and break find-related UI toggles/commands for those panels. Extend this computed property to account for the new panel-specific find visibility signals.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/TabManager.swift` around lines 2380 - 2382, The isFindVisible computed property currently only checks selectedTerminalPanel?.searchState and focusedBrowserPanel?.searchState; update it to also include the new panel types so it reflects File Preview and Markdown find UI state (for example, include checks like selectedFilePreviewPanel?.searchState != nil and focusedMarkdownPanel?.searchState != nil or whatever the actual properties are on those panel classes); modify the isFindVisible getter to OR together the existing checks and the new panel-specific find visibility signals so find-related toggles/commands work correctly across all supported panels (ensure you reference the exact property names on the File Preview and Markdown panel types when implementing).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/TabManager.swift`:
- Around line 2380-2382: The isFindVisible computed property currently only
checks selectedTerminalPanel?.searchState and focusedBrowserPanel?.searchState;
update it to also include the new panel types so it reflects File Preview and
Markdown find UI state (for example, include checks like
selectedFilePreviewPanel?.searchState != nil and
focusedMarkdownPanel?.searchState != nil or whatever the actual properties are
on those panel classes); modify the isFindVisible getter to OR together the
existing checks and the new panel-specific find visibility signals so
find-related toggles/commands work correctly across all supported panels (ensure
you reference the exact property names on the File Preview and Markdown panel
types when implementing).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 537aa07d-394e-4646-9884-c3dd33f6b925
📒 Files selected for processing (4)
Sources/Panels/FilePreviewPanel.swiftSources/Panels/MarkdownPanel.swiftSources/Panels/MarkdownWebSupport.swiftSources/TabManager.swift
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| // MARK: - Find in text mode | ||
|
|
||
| extension FilePreviewPanel { | ||
| var isFindVisible: Bool { false } |
There was a problem hiding this comment.
isFindVisible stub disables the "Hide Find Bar" menu item
isFindVisible is hardcoded to false, so TabManager.isFindVisible (and isNonTerminalFindVisible) remain false even after the NSTextFinder bar opens. In cmuxApp.swift:827 the "Hide Find Bar" command is gated on activeTabManager.isFindVisible:
.disabled(!(activeTabManager.isFindVisible))
This means that after the user presses Cmd+F in a file preview text-mode panel and the system find bar appears, the Hide Find Bar menu item stays grayed out. The same stub is present in MarkdownPanel.isFindVisible at line 458. Both panels need a real tracking mechanism (e.g. a stored flag set on startFind/hideFind) so the menu item is correctly enabled whenever the bar is actually showing.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Panels/FilePreviewPanel.swift`:
- Around line 4466-4468: The isFindVisible getter currently always returns
false; add a private Bool property (e.g., private var findBarVisible = false) to
track local find-bar state, update startFind() to set findBarVisible = true and
hideFind() to set findBarVisible = false, and make var isFindVisible return that
flag; additionally, clear findBarVisible (set to false) when leaving .text mode
and when the text view is detached (where the preview switches modes or the text
view is replaced) so TabManager.isNonTerminalFindVisible sees the real
visibility state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b6bfc8a-cca8-41c4-9cd1-2a4d83292713
📒 Files selected for processing (9)
Sources/Panels/AgentSessionPanel.swiftSources/Panels/AgentSessionWebRendererSession.swiftSources/Panels/FilePreviewPanel.swiftSources/Panels/MarkdownPanel.swiftSources/Panels/Panel.swiftSources/Panels/ProjectBuildSettingsTabView.swiftSources/Panels/ProjectFilesTabView.swiftSources/Panels/ProjectPanel.swiftSources/TabManager.swift
| /// The AppKit find bar visibility is not publicly exposed on `NSTextView`, | ||
| /// so this is conservative and reports `false` until a public signal exists. | ||
| var isFindVisible: Bool { false } |
There was a problem hiding this comment.
isFindVisible is permanently false, breaking global find visibility state.
Line 4468 always returns false, but TabManager.isNonTerminalFindVisible relies on this signal to detect active find UI. When the file-preview find bar is open, global find-state checks still report “not visible.”
Track a panel-local visibility flag (set in startFind()/hideFind(), and reset when leaving .text mode or detaching the text view) so this property reflects real state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Panels/FilePreviewPanel.swift` around lines 4466 - 4468, The
isFindVisible getter currently always returns false; add a private Bool property
(e.g., private var findBarVisible = false) to track local find-bar state, update
startFind() to set findBarVisible = true and hideFind() to set findBarVisible =
false, and make var isFindVisible return that flag; additionally, clear
findBarVisible (set to false) when leaving .text mode and when the text view is
detached (where the preview switches modes or the text view is replaced) so
TabManager.isNonTerminalFindVisible sees the real visibility state.
wowlegend
changed the title
…odes FilePreviewPanel now routes find actions to its NSTextView in text mode. MarkdownPanel routes find actions to its text view or WKWebView depending on display mode. TabManager now forwards find commands to file preview and markdown panels after terminal/browser. Closes manaflow-ai#6050, related to manaflow-ai#158 and manaflow-ai#6049.
- Move panel find helpers into extensions within the same files to keep routing centralized. - Update isFindVisible to include file preview and markdown panels (placeholder visibility until NSTextView exposes find-bar state). - Refresh swift-file-length-budget.tsv for TabManager, FilePreviewPanel, and MarkdownPanel. - Build now succeeds and the file-length guard passes.
- Introduce FindablePanel protocol with shared NSTextFinder.Action helper and default selection-find no-ops. - FilePreviewPanel & MarkdownPanel: conform to FindablePanel, support text selection for find. - MarkdownPanel preview & AgentSessionPanel: route find actions to WKWebView; hideFind is a documented no-op because NSFindPanelAction lacks a hide value. - ProjectPanel: Cmd+F focuses the search field in Files / Build Settings tabs via SwiftUI FocusState. - TabManager: route all find commands, selection-for-find, and visibility checks through FindablePanel. - Refresh swift-file-length-budget.tsv for TabManager, FilePreviewPanel, and MarkdownPanel. - Build succeeds and file-length budget passes.
wowlegend
force-pushed
the
fix-filepreview-markdown-find
branch
from
e8c551c to
0a24455
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/TabManager.swift`:
- Around line 3788-3789: When clearing workspace probe tracking in TabManager
(the places that call sidebarGitMetadataService.clearWorkspaceGitProbes(...) and
sidebarSelectedWorkspaceIds.remove(...)) also stop/reset the pull request
polling state by clearing the entry in pullRequestProbing for that workspace
(the same workspace/tabId handled by detachWorkspace and
restoreSessionSnapshot); in practice add logic to stop any running PR poller and
remove or nil out pullRequestProbing[tabId] at those same code paths so PR
polling cannot continue for workspaces no longer owned by this TabManager.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cdab1e57-a803-4c36-b3e1-054685fcb64f
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (10)
Sources/Panels/AgentSessionPanel.swiftSources/Panels/AgentSessionWebRendererSession.swiftSources/Panels/FilePreviewPanel.swiftSources/Panels/MarkdownPanel.swiftSources/Panels/MarkdownWebSupport.swiftSources/Panels/Panel.swiftSources/Panels/ProjectBuildSettingsTabView.swiftSources/Panels/ProjectFilesTabView.swiftSources/Panels/ProjectPanel.swiftSources/TabManager.swift
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/TabManager.swift`:
- Around line 3788-3789: When clearing workspace probe tracking in TabManager
(the places that call sidebarGitMetadataService.clearWorkspaceGitProbes(...) and
sidebarSelectedWorkspaceIds.remove(...)) also stop/reset the pull request
polling state by clearing the entry in pullRequestProbing for that workspace
(the same workspace/tabId handled by detachWorkspace and
restoreSessionSnapshot); in practice add logic to stop any running PR poller and
remove or nil out pullRequestProbing[tabId] at those same code paths so PR
polling cannot continue for workspaces no longer owned by this TabManager.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cdab1e57-a803-4c36-b3e1-054685fcb64f
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (10)
Sources/Panels/AgentSessionPanel.swiftSources/Panels/AgentSessionWebRendererSession.swiftSources/Panels/FilePreviewPanel.swiftSources/Panels/MarkdownPanel.swiftSources/Panels/MarkdownWebSupport.swiftSources/Panels/Panel.swiftSources/Panels/ProjectBuildSettingsTabView.swiftSources/Panels/ProjectFilesTabView.swiftSources/Panels/ProjectPanel.swiftSources/TabManager.swift
🛑 Comments failed to post (1)
Sources/TabManager.swift (1)
3788-3789:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing pull-request tracking cleanup in non-close lifecycle paths.
Line 3788 and Line 8143 clear git probe tracking, but neither path clears/reset
pullRequestProbing. AfterdetachWorkspaceandrestoreSessionSnapshot, PR polling can keep running for workspaces no longer owned by thisTabManager.Proposed fix
@@ func detachWorkspace(tabId: UUID) -> Workspace? { guard let index = tabs.firstIndex(where: { $0.id == tabId }) else { return nil } sidebarGitMetadataService.clearWorkspaceGitProbes(workspaceId: tabId) + pullRequestProbing.clearWorkspacePullRequestTracking(workspaceId: tabId) sidebarSelectedWorkspaceIds.remove(tabId) invalidateFocusHistoryTarget(workspaceId: tabId, panelId: nil) @@ ClosedItemHistoryStore.shared.removePanelRecords( forWorkspaceIds: Set(previousTabs.map(\.id)) ) sidebarGitMetadataService.resetAllWorkspaceGitProbeTracking() + for tab in previousTabs { + pullRequestProbing.clearWorkspacePullRequestTracking(workspaceId: tab.id) + }Also applies to: 8143-8143
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/TabManager.swift` around lines 3788 - 3789, When clearing workspace probe tracking in TabManager (the places that call sidebarGitMetadataService.clearWorkspaceGitProbes(...) and sidebarSelectedWorkspaceIds.remove(...)) also stop/reset the pull request polling state by clearing the entry in pullRequestProbing for that workspace (the same workspace/tabId handled by detachWorkspace and restoreSessionSnapshot); in practice add logic to stop any running PR poller and remove or nil out pullRequestProbing[tabId] at those same code paths so PR polling cannot continue for workspaces no longer owned by this TabManager.
wowlegend
force-pushed
the
fix-filepreview-markdown-find
branch
from
0a24455 to
a8f253c
Compare
- Remove isFindVisible from FindablePanel protocol and panel implementations to avoid a placeholder UI-state source of truth. - TabManager.isFindVisible now only reflects terminal and browser find state, which are the only panels with a public visibility signal. - Add docstrings to all new FindablePanel methods and TabManager routing helpers. - Refresh FilePreviewPanel file-length budget.
wowlegend
force-pushed
the
fix-filepreview-markdown-find
branch
from
a8f253c to
fdf7ff5
Compare
|
Hi — I just pushed a fix for the Swift warning-budget failure introduced by the |
1 participant
Closes #6050
Problem
Cmd+Fdid not open the find UI for several panel types. The existingTabManagerfind routing only handledTerminalPanelandBrowserPanel, silently ignoring other focused panels.Solution
Introduce a
FindablePanelcapability protocol and route global find commands (startFind,findNext,findPrevious,hideFind, plus selection-for-find) through it. Menu visibility (isFindVisible) is kept accurate by only reflecting terminal and browser state, which are the only panels that expose a public find-bar visibility signal.Panels now supported
FilePreviewPaneltext mode — AppKitNSTextViewfind bar.MarkdownPaneltext and preview modes —NSTextViewfind bar orWKWebViewfind panel.AgentSessionPanel—WKWebViewfind panel.ProjectPanelFiles / Build Settings tabs — focuses the tab-specific filter search field via SwiftUIFocusState.Implementation notes
NSTextFinder.Action.menuItemSenderhelper removes duplicatedNSMenuItemconstruction.MarkdownPanelpreview andAgentSessionPanelhide-find are documented no-ops becauseWKWebView.performFindPanelAction:usesNSFindPanelAction, which only defines values 1–10 and has no hide value.FilePreviewPanelandMarkdownPaneltext mode support "Use Selection for Find".canUseSelectionForFindnow includesFindablePanelpanels that report a text selection.Verification
scripts/swift_file_length_budget.pypasses after refreshing.github/swift-file-length-budget.tsv.Related issues
Summary by CodeRabbit