Repository navigation
feat: draggable terminal groups with insertion-line drop indicator - #44
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughSessionGroupListView improves group renaming UX with two coordinated fixes: lifecycle-driven NSEvent monitor management that installs on rename start and removes on state change or view disappearance, plus enhanced GroupNameTextField activation logic that retries with state checks (first responder, key window, app active) rather than a single fixed delay. ChangesGroup Renaming Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
A SwiftUI .onDrag in this accessory app returns frontmost-app focus to the previously active process. The DrawerPanel still claims key status internally, but key events route to whichever app is frontmost — so the rename field appeared focused yet received no input. Reactivate the app, clear any stuck editing session, and extend the retry window to survive slower drag teardown. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
f1dbe57 to
cee62c5
Compare
![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)
macos/Sources/Ocak/Views/SessionListView.swift (1)
1148-1176:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCancel or gate these focus retries when rename ends.
The delayed retries (0.15s, 0.35s, 0.6s) are scheduled during
makeNSView()and cannot be cancelled—they will fire regardless of whether the user later commits or cancels the rename. If the user presses Escape to cancel, then immediately switches apps, the queued retry can still executeNSApp.activate(ignoringOtherApps: true)and pull Ocak back to the front, unexpectedly interrupting their work.Add a check in the retry closure to bail out if the rename session is no longer active (e.g.,
guard isRenamingGroup else { return }), or store and invalidate the retry tokens if using a cancellation mechanism. Test by: start rename → press Escape → immediately switch to another app → Ocak should not regain focus during the retry window.🤖 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 `@macos/Sources/Ocak/Views/SessionListView.swift` around lines 1148 - 1176, The delayed focus-retry closures scheduled in makeNSView (the DispatchQueue.main.asyncAfter loop calling activate(tf)) must be gated or cancellable so they don't re-activate the app after a rename has ended; update the retry closure to early-return if the rename session is no longer active (e.g., guard isRenamingGroup else { return }) or instead schedule the retries as DispatchWorkItem instances stored on the view/session and cancel them from the rename-completion/cancel path (the method that ends renaming). Locate the activate(_:) closure and the loop that iterates delays [0.15,0.35,0.6] and either add the guard check referencing isRenamingGroup (or equivalent state/property) or switch to DispatchWorkItem + cancel logic and ensure cancellation is invoked when the rename finishes or is aborted.
🤖 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 `@macos/Sources/Ocak/Views/SessionListView.swift`:
- Around line 1148-1176: The delayed focus-retry closures scheduled in
makeNSView (the DispatchQueue.main.asyncAfter loop calling activate(tf)) must be
gated or cancellable so they don't re-activate the app after a rename has ended;
update the retry closure to early-return if the rename session is no longer
active (e.g., guard isRenamingGroup else { return }) or instead schedule the
retries as DispatchWorkItem instances stored on the view/session and cancel them
from the rename-completion/cancel path (the method that ends renaming). Locate
the activate(_:) closure and the loop that iterates delays [0.15,0.35,0.6] and
either add the guard check referencing isRenamingGroup (or equivalent
state/property) or switch to DispatchWorkItem + cancel logic and ensure
cancellation is invoked when the rename finishes or is aborted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9df76815-903f-4516-a666-4d47b0ee969d
📒 Files selected for processing (1)
macos/Sources/Ocak/Views/SessionListView.swift
434fdb5 to
cee62c5
Compare
…ery paths Reserve the heavy NSApp/window reactivation for the post-drag retry path so the common no-drag rename only does a plain makeFirstResponder. Document why endEditing is recovery-only and why the retry delays were chosen. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
GroupDropTargetDelegateon each group card, which now handles both session drops and group reordering in a single delegateasyncAfter 120ms) to survive AppKit drag session teardown reclaiming first responder.onDragis absent from the header when renaming is active, preventing gesture competition with the NSTextFieldTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit