fix (desktop): fix rename right click on terminal tabs#471
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughMove rename focus/selection out of a startRename timeout into a dedicated useEffect, render the rename input outside the Radix context-menu wrapper, and prevent the context-menu's rename selection from returning focus to the trigger. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TabButton as TabItem (button)
participant ContextMenu as TabContextMenu (Radix)
participant RenameInput as Rename Input (isolated)
Note over TabButton,ContextMenu: Changed rename control flow
User->>TabButton: Right-click (contextmenu)
TabButton->>TabButton: onContextMenu -> select tab
TabButton->>ContextMenu: open context menu
User->>ContextMenu: Click "Rename Tab"
ContextMenu->>ContextMenu: handleRenameSelect(event)\n(event.preventDefault())
ContextMenu->>TabButton: onRename()
TabButton->>TabButton: set isRenaming=true\nmark justStartedRenamingRef
TabButton->>RenameInput: render rename input outside Radix wrapper
RenameInput->>RenameInput: useEffect -> focus & select (short delay)
User->>RenameInput: type / Enter or blur
RenameInput->>TabButton: submitRename() or handleBlur (ignores immediate blur)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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: 3
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx (1)
106-106: Extract magic timing values to named constants.The timing values (50ms, 200ms) are repeated and lack clear semantic meaning. If you retain the timing-based approach (though I recommend against it), extract these as named constants with explanatory comments.
🔎 Example with named constants
+// Time window to ignore blur events after starting rename (to prevent context menu focus restoration) +const RENAME_BLUR_IGNORE_WINDOW_MS = 200; +// Delay before focusing input to allow context menu to close +const FOCUS_DELAY_MS = 50; + useEffect(() => { if (isRenaming && inputRef.current) { - const timeoutId = setTimeout(() => { + const timeoutId = setTimeout(() => { inputRef.current?.focus(); inputRef.current?.select(); - }, 50); + }, FOCUS_DELAY_MS); return () => clearTimeout(timeoutId); } }, [isRenaming]); const handleBlur = () => { - if (Date.now() - renameStartTimeRef.current < 200) { + if (Date.now() - renameStartTimeRef.current < RENAME_BLUR_IGNORE_WINDOW_MS) { setTimeout(() => { inputRef.current?.focus(); inputRef.current?.select(); - }, 50); + }, FOCUS_DELAY_MS); return; } submitRename(); };Also applies to: 114-114, 119-119
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
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/Sidebar/TabsView/TabItem/index.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to src/components/{ui,ai-elements,react-flow}/**/*.tsx : Use kebab-case single files for shadcn/ui components in src/components/ui/, src/components/ai-elements, and src/components/react-flow/ directories
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx (2)
packages/ui/src/components/ui/input.tsx (1)
Input(29-29)packages/ui/src/components/ui/tooltip.tsx (3)
Tooltip(76-76)TooltipTrigger(76-76)TooltipContent(76-76)
⏰ 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 (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx (1)
87-91: Good UX improvement: activate tab on right-click.Setting the active tab when right-clicking provides better context for the context menu actions (like rename). This is consistent with common tab interface patterns.
| // When renaming, render input outside the context menu to avoid Radix focus management interference | ||
| if (isRenaming) { | ||
| return ( | ||
| <div className="w-full"> | ||
| <div className="relative group"> | ||
| <div | ||
| ref={(el) => { | ||
| drag(el); | ||
| drop(el); | ||
| }} | ||
| className={` | ||
| w-full text-start px-3 py-2 rounded-md flex items-center gap-2 justify-between pr-8 | ||
| ${isActive ? "bg-tertiary-active" : ""} | ||
| ${isDragging ? "opacity-50" : ""} | ||
| ${isDragOver ? "bg-tertiary-active/50" : ""} | ||
| `} | ||
| > | ||
| <HiMiniCommandLine className="size-4" /> | ||
| <div className="flex items-center gap-1 flex-1 min-w-0"> | ||
| <Input | ||
| ref={inputRef} | ||
| variant="ghost" | ||
| value={renameValue} | ||
| onChange={(e) => setRenameValue(e.target.value)} | ||
| onBlur={handleBlur} | ||
| onKeyDown={handleKeyDown} | ||
| onClick={(e) => e.stopPropagation()} | ||
| className="flex-1" | ||
| /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Address code duplication from dual rendering paths.
The dedicated rename block duplicates layout, styling, and drag-drop logic from the main rendering path (lines 184-241). This creates maintenance burden and risks divergence.
The duplication exists to "avoid Radix focus management interference," but this is treating the symptom rather than the cause. If you resolve the focus management issue (see previous comments), you could render the rename input inline within the TabContextMenu wrapper, eliminating this duplication.
🔎 Potential refactor to eliminate duplication
Extract common layout into a reusable component:
interface TabLayoutProps {
isActive: boolean;
isDragging: boolean;
isDragOver: boolean;
attachRef: (el: HTMLElement | null) => void;
children: React.ReactNode;
className?: string;
}
function TabLayout({ isActive, isDragging, isDragOver, attachRef, children, className }: TabLayoutProps) {
return (
<div
ref={attachRef}
className={`
w-full text-start px-3 py-2 rounded-md flex items-center gap-2 justify-between pr-8
${isActive ? "bg-tertiary-active" : ""}
${isDragging ? "opacity-50" : ""}
${isDragOver ? "bg-tertiary-active/50" : ""}
${className ?? ""}
`}
>
<HiMiniCommandLine className="size-4" />
<div className="flex items-center gap-1 flex-1 min-w-0">
{children}
</div>
</div>
);
}Then use it in both paths, or better yet, render inline once focus is properly managed.
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
around lines 148-182, the rename branch duplicates the same layout, styling and
drag/drop ref logic found later (lines ~184-241); extract the shared container
into a single reusable TabLayout component that accepts
isActive/isDragging/isDragOver, an attachRef callback (so you can call drag(el);
drop(el);) and children, then replace both render paths to use that TabLayout
(render the rename <Input> as children when isRenaming) so the layout, classes
and ref logic are only declared once and the rename input is rendered inline
under the same wrapper (or, if preferred, fix the Radix focus handling first and
render the input inside the existing TabContextMenu wrapper).
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The rename fix is directionally good, but there are a couple of maintainability/UX risks: an uncleared setTimeout in handleBlur can cause post-cancel focus jumps, and the early-return rename UI changes semantics/keyboard focus compared to the normal Button path. Additionally, onContextMenu may need preventDefault() to avoid native context menu interference depending on how TabContextMenu is wired.
Additional notes (2)
- Compatibility |
...ns/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx:87-92
handleContextMenuactivates the tab on right-click, but it doesn’t prevent default. Depending on howTabContextMenuis implemented (Radix context menu vs native), letting the native context menu open in some edge cases can interfere with the custom menu and/or focus behavior.
If TabContextMenu relies on the browser contextmenu event, you likely want to ensure consistent behavior by explicitly calling e.preventDefault() (and possibly e.stopPropagation()) here.
- Maintainability |
...ns/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx:99-121
The rename focus/blur workaround is implemented with multiple hard-coded timing constants (50msand200ms). These “magic numbers” are brittle across platforms and can regress easily if Radix behavior changes.
At minimum, these should be named constants (and ideally centralized) so future tweaks are safer and self-explanatory.
Summary of changes
What changed
- Added
useEffectto manage focusing/selecting the rename<Input />after entering rename mode, instead of doing it inline viasetTimeout(..., 0). - Introduced a
renameStartTimeRef+handleBlurguard to avoid immediate blur cancelling rename (likely caused by context menu focus restoration). - Ensured right-clicking a tab activates it via new
onContextMenu={handleContextMenu}handler. - Refactored rendering: when
isRenaming, the component now returns early and renders the rename input outside ofTabContextMenuto avoid Radix focus management interference. - Simplified the non-renaming path by removing the inline
{isRenaming ? ... : ...}branch inside the tab button.
| const handleBlur = () => { | ||
| // Ignore blur events within 200ms of starting rename | ||
| // This prevents the context menu's focus restoration from canceling rename | ||
| if (Date.now() - renameStartTimeRef.current < 200) { | ||
| // Re-focus after the context menu finishes its focus restoration | ||
| setTimeout(() => { | ||
| inputRef.current?.focus(); | ||
| inputRef.current?.select(); | ||
| }, 50); | ||
| return; | ||
| } | ||
| submitRename(); |
There was a problem hiding this comment.
handleBlur schedules a setTimeout to refocus the input, but that timeout is not cleaned up if the component unmounts or isRenaming flips to false quickly (e.g., user presses Escape while the timeout is pending). That can lead to attempting to focus a stale ref, and it can also cause confusing focus jumps after rename was cancelled.
Because you already use an effect cleanup pattern for the initial focus, it’s worth applying the same discipline here (store the timeout id in a ref and clear it when leaving rename mode / unmounting).
Suggestion
Consider tracking the refocus timeout in a ref and clearing it when rename ends/unmounts.
Example:
const refocusTimeoutRef = useRef<number | null>(null);
useEffect(() => {
return () => {
if (refocusTimeoutRef.current != null) {
window.clearTimeout(refocusTimeoutRef.current);
refocusTimeoutRef.current = null;
}
};
}, []);
const handleBlur = () => {
if (Date.now() - renameStartTimeRef.current < 200) {
if (refocusTimeoutRef.current != null) {
window.clearTimeout(refocusTimeoutRef.current);
}
refocusTimeoutRef.current = window.setTimeout(() => {
inputRef.current?.focus();
inputRef.current?.select();
}, 50);
return;
}
submitRename();
};Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this change.
| // When renaming, render input outside the context menu to avoid Radix focus management interference | ||
| if (isRenaming) { | ||
| return ( | ||
| <div className="w-full"> | ||
| <div className="relative group"> | ||
| <div | ||
| ref={(el) => { | ||
| drag(el); | ||
| drop(el); | ||
| }} | ||
| className={` | ||
| w-full text-start px-3 py-2 rounded-md flex items-center gap-2 justify-between pr-8 | ||
| ${isActive ? "bg-tertiary-active" : ""} | ||
| ${isDragging ? "opacity-50" : ""} | ||
| ${isDragOver ? "bg-tertiary-active/50" : ""} | ||
| `} | ||
| > | ||
| <HiMiniCommandLine className="size-4" /> | ||
| <div className="flex items-center gap-1 flex-1 min-w-0"> | ||
| <Input | ||
| ref={inputRef} | ||
| variant="ghost" | ||
| value={renameValue} | ||
| onChange={(e) => setRenameValue(e.target.value)} | ||
| onBlur={handleBlur} | ||
| onKeyDown={handleKeyDown} | ||
| onClick={(e) => e.stopPropagation()} | ||
| className="flex-1" | ||
| /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
The new “rename mode” early-return renders a <div> styled like a tab, but it is no longer a focusable/button element and it no longer wires up interactions that existed on the Button (e.g. onClick, onContextMenu, keyboard handling for tab activation). This is probably acceptable during renaming, but the regression risk is that:
- the tab might stop being keyboard-focusable while renaming (accessibility/UX),
- right-click behaviors may differ while renaming,
- drag/drop ref attachment is duplicated inline, increasing drift between the two render paths.
At minimum, consider reusing attachRef and preserving semantics by rendering a Button wrapper even in rename mode (or adding appropriate role="button"/tabIndex + keyboard handling).
Suggestion
To avoid behavior drift, reuse attachRef and keep consistent semantics.
Option A (preferred): keep using <Button variant="ghost"> as the wrapper in rename mode too, but render the <Input /> inside it (outside TabContextMenu, as you’re doing).
Option B: if you must keep a <div>, add role="button", tabIndex={0}, and key handling consistent with the normal path.
Also, you can use the existing helper:
ref={attachRef}instead of duplicating drag/drop wiring.
Reply with "@CharlieHelps yes please" if you’d like me to add a commit implementing option A plus ref reuse.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx (3)
150-189: Code duplication and accessibility regression remain unaddressed.This dedicated rename block duplicates layout, styling, and drag-drop ref attachment from the main render path (lines 198-234), creating maintenance burden and drift risk. A past review comment specifically flagged this duplication.
Additionally, the
<div>at lines 155-163 lacks proper button semantics (norole="button",tabIndex, or keyboard handling), regressing accessibility compared to the<Button>used in the normal path. Another past comment flagged this regression.If you resolve the focus management issue (via Radix's
onCloseAutoFocus), you can render the rename input inline within the singleTabContextMenuwrapper, eliminating both duplication and accessibility concerns.🔎 Path forward
- Use
onCloseAutoFocusinTabContextMenuto prevent focus restoration- Render rename input inline within the single
<Button>wrapper:<Button ref={attachRef} /* ... */> <HiMiniCommandLine className="size-4" /> <div className="flex items-center gap-1 flex-1 min-w-0"> {isRenaming ? ( <Input ref={inputRef} value={renameValue} onChange={(e) => setRenameValue(e.target.value)} onBlur={handleBlur} onKeyDown={handleKeyDown} className="flex-1" /> ) : ( <> <span className="truncate flex-1">{displayName}</span> {needsAttention && /* ... */} </> )} </div> </Button>This maintains semantics, eliminates duplication, and coordinates with Radix properly.
114-125: Focus re-entrancy pattern is still fragile.While
queueMicrotaskis better thansetTimeout, this pattern still creates a focus battle: blur fires → check flag → re-focus via microtask. This fights against Radix's focus restoration rather than coordinating with it.If you prevent Radix from restoring focus (via
onCloseAutoFocusas suggested above), this re-focusing logic becomes unnecessary—you can simply callsubmitRename()unconditionally inhandleBlur.🔎 Simplified blur handling
Once
onCloseAutoFocusprevents Radix focus restoration:const handleBlur = () => { - // Ignore blur if we just started renaming (focus may be temporarily stolen) - if (justStartedRenamingRef.current) { - // Re-focus the input after current event processing completes - queueMicrotask(() => { - inputRef.current?.focus(); - inputRef.current?.select(); - }); - return; - } submitRename(); };The
justStartedRenamingRefcan also be removed.
101-112: Replace timing-based useEffect with Radix focus callbacks.This
useEffectwith a 100ms timeout violates the coding guideline: "Do not use effect unless absolutely necessary." The arbitrary delay is still a race-condition workaround fighting Radix's focus management.Based on coding guidelines, avoid
useEffectunless absolutely necessary. The focus management should be coordinated with Radix's lifecycle callbacks instead.🔎 Recommended approach using Radix callbacks
In
TabContextMenu.tsx, expose the menu's open state:export function TabContextMenu({ paneCount, onClose, onRename, children, }: TabContextMenuProps) { const [isOpen, setIsOpen] = useState(false); // ... existing code ... return ( <ContextMenu onOpenChange={setIsOpen}> <ContextMenuContent className="w-48" onCloseAutoFocus={(e) => { e.preventDefault(); // Focus will be managed by TabItem after menu closes }} > {/* menu items */} </ContextMenuContent> </ContextMenu> ); }Then in
TabItem, focus synchronously instartRename:const startRename = () => { setRenameValue(tab.userTitle ?? tab.name ?? displayName); justStartedRenamingRef.current = true; setIsRenaming(true); // Focus synchronously after state update completes queueMicrotask(() => { inputRef.current?.focus(); inputRef.current?.select(); justStartedRenamingRef.current = false; }); };Remove the
useEffectentirely.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsx (1)
26-30: Consider using Radix's built-in focus management callback.While
event.preventDefault()does prevent focus restoration, Radix Context Menu providesonCloseAutoFocusonContextMenuContentspecifically for controlling focus when the menu closes. This is more idiomatic and explicit about intent.🔎 Refactor to use Radix's focus callback
- const handleRenameSelect = (event: Event) => { - // Prevent default to stop Radix from restoring focus to the trigger - event.preventDefault(); - onRename(); - }; const contextMenuContent = ( <ContextMenu> <ContextMenuTrigger asChild>{children}</ContextMenuTrigger> - <ContextMenuContent className="w-48"> - <ContextMenuItem onSelect={handleRenameSelect}> + <ContextMenuContent + className="w-48" + onCloseAutoFocus={(e) => { + // Prevent focus restoration when entering rename mode + e.preventDefault(); + }} + > + <ContextMenuItem onSelect={onRename}> Rename Tab </ContextMenuItem>Apply the same pattern to the tooltip-wrapped menu at lines 57-65.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
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/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to src/components/{ui,ai-elements,react-flow}/**/*.tsx : Use kebab-case single files for shadcn/ui components in src/components/ui/, src/components/ai-elements, and src/components/react-flow/ directories
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsx (1)
packages/ui/src/components/ui/context-menu.tsx (4)
ContextMenu(237-237)ContextMenuTrigger(238-238)ContextMenuContent(239-239)ContextMenuItem(240-240)
⏰ 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 (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx (4)
46-47: Improvement over the previous timing approach.The
justStartedRenamingRefflag is a clearer pattern than the previousDate.now()timestamp comparison. This makes the rename flow's intent more explicit.
88-93: LGTM! Correct context menu behavior.Selecting the tab on right-click ensures that context menu actions (like "Rename Tab") apply to the correct tab. This is the expected UX pattern.
145-148: LGTM! Clean ref attachment pattern.The
attachRefhelper correctly combines drag and drop refs, eliminating duplication at the call sites.
198-234: Main rendering path looks good.The button properly maintains semantics with keyboard handling, drag-drop support, and the attention indicator with tooltip. The conditional rendering of
needsAttentionis correctly implemented.
🧹 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.