feature (desktop): double click to rename terminal tabs#628
Conversation
|
Warning Rate limit exceeded@AviPeltz has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 43 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughInline editing capability is added to tab group items in the UI layer. The GroupItem component introduces editing state and keyboard handling, while GroupStrip wires a rename handler to the renameTab API, enabling users to rename tab groups directly from the UI. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsx:
- Around line 27-30: The editValue state is initialized from
getTabDisplayName(tab) but never updates when the tab prop or displayName
changes; add a useEffect in GroupItem that watches displayName (and optionally
isEditing) and calls setEditValue(displayName) whenever displayName changes
while not editing, so editValue stays in sync with getTabDisplayName(tab) unless
the user is actively editing.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsx (1)
82-97: Consider UX implications of save-on-blur behavior.The
onBlurevent triggershandleSave, which auto-saves when the user clicks outside the input. This might lead to unintended renames if the user accidentally clicks elsewhere. Many inline editors cancel on blur and require explicit Enter confirmation, or at least give users a choice.Alternative: Cancel on blur, save only on Enter
<input ref={inputRef} type="text" value={editValue} onChange={(e) => setEditValue(e.target.value)} - onBlur={handleSave} + onBlur={handleCancel} onKeyDown={handleKeyDown} onClick={(e) => e.stopPropagation()} className="text-sm bg-transparent border-none outline-none flex-1 text-left min-w-0" />This makes the interaction more deliberate and prevents accidental renames.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx (1)
150-150: Prop wiring looks correct.The
onRenamecallback properly capturestab.idand forwardsnewNametohandleRenameGroup.Consider adding error handling if
renameTabcan fail (e.g., validation errors or persistence failures). While zustand actions are typically synchronous, providing user feedback for failures would improve UX.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsx (2)
apps/desktop/src/renderer/stores/tabs/utils.ts (1)
getTabDisplayName(19-31)packages/ui/src/lib/utils.ts (1)
cn(4-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsx (2)
1-17: LGTM!The interface properly extends
GroupItemPropswith theonRenamecallback, and all imports are renderer-safe.
32-65: LGTM on focus management and keyboard handling!The auto-focus effect is appropriate, and keyboard handling correctly implements Enter to save and Escape to cancel. The
handleDoubleClickrefresheseditValuefrom the currentdisplayName, which provides a clean starting point for editing.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx (1)
127-129: LGTM! Simple delegation to the store.The
handleRenameGrouphandler correctly delegates torenameTabfrom the zustand store.
| const displayName = getTabDisplayName(tab); | ||
| const [isEditing, setIsEditing] = useState(false); | ||
| const [editValue, setEditValue] = useState(displayName); | ||
| const inputRef = useRef<HTMLInputElement>(null); |
There was a problem hiding this comment.
Sync editValue with displayName changes when not editing.
The editValue state is initialized once but won't update if the underlying tab prop changes (e.g., if another part of the app renames the tab, or the tab reference changes). When the user double-clicks to edit, they'll see a stale name.
🔎 Proposed fix to synchronize editValue with displayName
const displayName = getTabDisplayName(tab);
const [isEditing, setIsEditing] = useState(false);
const [editValue, setEditValue] = useState(displayName);
const inputRef = useRef<HTMLInputElement>(null);
+
+ // Sync editValue when displayName changes and we're not editing
+ useEffect(() => {
+ if (!isEditing) {
+ setEditValue(displayName);
+ }
+ }, [displayName, isEditing]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const displayName = getTabDisplayName(tab); | |
| const [isEditing, setIsEditing] = useState(false); | |
| const [editValue, setEditValue] = useState(displayName); | |
| const inputRef = useRef<HTMLInputElement>(null); | |
| const displayName = getTabDisplayName(tab); | |
| const [isEditing, setIsEditing] = useState(false); | |
| const [editValue, setEditValue] = useState(displayName); | |
| const inputRef = useRef<HTMLInputElement>(null); | |
| // Sync editValue when displayName changes and we're not editing | |
| useEffect(() => { | |
| if (!isEditing) { | |
| setEditValue(displayName); | |
| } | |
| }, [displayName, isEditing]); |
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsx
around lines 27 - 30, The editValue state is initialized from
getTabDisplayName(tab) but never updates when the tab prop or displayName
changes; add a useEffect in GroupItem that watches displayName (and optionally
isEditing) and calls setEditValue(displayName) whenever displayName changes
while not editing, so editValue stays in sync with getTabDisplayName(tab) unless
the user is actively editing.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.