Repository navigation
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds UI for drop overlays and context menus, replaces drop hook with useTabContentDrop (attachDrop), shows DropOverlay for drop targets, refactors Group/Single tab views to use TabContentContextMenu, adds sidebar TabContextMenu and useTabRename, and updates tab store/drag logic and tests to remove empty groups and adjust active/history state. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TabsContent
participant DropHook as useTabContentDrop
participant DragLogic
participant Store
User->>TabsContent: Drag tab over content
TabsContent->>DropHook: register tabToRender via attachDrop
DropHook->>DropHook: validate (tabToRender != dragged)
DropHook-->>TabsContent: isDropZone = true
TabsContent->>TabsContent: render DropOverlay(message)
User->>TabsContent: Drop
TabsContent->>DropHook: attachDrop triggers drop handler
DropHook->>DragLogic: dragTabToTab(draggedId, targetId)
DragLogic->>Store: update tabs (parentId/layout)
DragLogic->>DragLogic: removeFromOldParent(oldParentId)
alt old parent becomes empty
DragLogic->>Store: remove empty group, update active/history
else
DragLogic->>Store: persist updated layout
end
Store-->>TabsContent: state/UI updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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
🧹 Nitpick comments (4)
apps/desktop/src/renderer/stores/tabs/drag-logic.ts (1)
77-105: Empty‑group cleanup viaremoveFromOldParentlooks correct; consider documenting the parentId assumptionThe new
removeFromOldParentbehavior plus its use in Rule 2/3/4 correctly:
- Updates the old group’s
layoutviaremoveTabFromLayout, and- Removes the old group entirely when no remaining tabs have
parentId === oldParentId(fixing the “orphan empty group” cases your new tests cover).Because
hasChildrenrelies on thetabsarray passed in (which must already have the dragged tab’sparentIdupdated), this helper is slightly coupled to its callers’ ordering. A short comment like “expectstabsto reflect updated parentIds for the dragged tab” near the helper would make that contract explicit and future‑proof.Also applies to: 283-297
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/DropOverlay.tsx (1)
1-13: DropOverlay implementation is clean; optional a11y hintThe overlay is nicely self‑contained and
pointer-events-noneavoids breaking drop behavior. If the message duplicates other visible/announced context, you might consideraria-hidden="true"on the outer<div>to ensure screen readers don’t get redundant instructions, but that’s optional.apps/desktop/src/renderer/stores/tabs/drag-logic.test.ts (1)
708-869: New “last child dragged out of group” tests align well with the updated logicThe added cases around dragging the final child:
- Onto a single tab (new group creation, old group removed), and
- Into another group (source group removed, target group layout expanded),
accurately exercise the new empty‑group cleanup path and would catch regressions in
removeFromOldParent/ Rule 4.The last test (“nested layout…simple layout”) is effectively another single‑child case very similar to the first; it’s fine to keep for extra safety, but you could consolidate those two to reduce maintenance overhead.
apps/desktop/src/renderer/stores/tabs/store.ts (1)
117-171: Centralized empty‑group cleanup viahandleEmptyGroupRemovallooks correct and makes the store behavior easier to reason aboutThe new
handleEmptyGroupRemovalhelper, plus its use in:
removeChildTabFromGroupfor the “no children left” case, andungroupTabwhen the parent ends up empty,correctly:
- Drops the specified ids (
groupIdand optionally a child),- Cleans up
tabHistoryStacksby stripping removed ids, and- Recomputes
activeTabIds[workspaceId], preferring the ungrouped tab when provided, then history, then first remaining tab, ornullwhen the workspace is empty.Using the helper in both paths removes duplicated logic and should prevent subtle divergence in active‑tab behavior between drag/ungroup/remove flows. As an optional polish, you might:
- Add a brief doc comment on
fallbackActiveTabId’s intent (e.g. “prefer this id if still present”), and- Consider whether future top‑level group removal paths should also go through this helper for consistency.
Functionally, the current implementation looks sound.
Also applies to: 377-385, 430-472
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/DropOverlay.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/index.tsx(5 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SingleTabView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx(2 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/useTabContentDrop.ts(3 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx(1 hunks)apps/desktop/src/renderer/stores/tabs/drag-logic.test.ts(1 hunks)apps/desktop/src/renderer/stores/tabs/drag-logic.ts(2 hunks)apps/desktop/src/renderer/stores/tabs/store.test.ts(1 hunks)apps/desktop/src/renderer/stores/tabs/store.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/useTabContentDrop.ts (1)
useTabContentDrop(6-36)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx (1)
EmptyTabView(1-18)apps/desktop/src/shared/types.ts (1)
TabType(23-30)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SingleTabView.tsx (1)
SingleTabView(8-16)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/DropOverlay.tsx (1)
DropOverlay(5-13)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/index.tsx (1)
GroupTabView(44-130)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/index.tsx (1)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
useActiveTabIds(522-523)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
apps/desktop/src/renderer/stores/tabs/types.ts (1)
Tab(25-25)
apps/desktop/src/renderer/stores/tabs/store.test.ts (1)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
useTabsStore(173-519)
apps/desktop/src/renderer/stores/tabs/drag-logic.ts (1)
apps/desktop/src/shared/types.ts (1)
TabType(23-30)
apps/desktop/src/renderer/stores/tabs/drag-logic.test.ts (3)
apps/desktop/src/renderer/stores/tabs/types.ts (1)
Tab(25-25)apps/desktop/src/shared/types.ts (1)
TabType(23-30)apps/desktop/src/renderer/stores/tabs/drag-logic.ts (1)
handleDragTabToTab(127-326)
🔇 Additional comments (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx (1)
90-97: Close button cursor tweak is good UXAdding
cursor-pointerto the close icon aligns cursor feedback with its clickable behavior; no issues with the existing keyboard-accessiblebuttonsemantics.apps/desktop/src/renderer/stores/tabs/store.test.ts (1)
1-178: removeTab test coverage is solid and matches the new empty‑group semanticsThese tests nicely pin down the intended behavior for:
- Removing the last vs. one-of-many children from a group,
- Ensuring unrelated groups aren’t affected by top-level tab removal, and
- Correctly switching the active tab when a group disappears.
They line up with the
handleEmptyGroupRemovalhelper’s contract and should give good safety for future refactors.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (2)
21-35: Clean refactoring of tab resolution logic.The new
tabToRendercomputation properly handles the cascade: workspace → active tab → parent group fallback. The early returns withnullmake the logic clear and theuseMemodependencies are correct.
37-61: Well-structured drop zone integration.The integration of
useTabContentDropand conditionalDropOverlayrendering is clean. TheattachDropref callback properly wires the drop target, and the contextual messages ("Drop to create split view" vs "Drop to add to split view") provide good user feedback.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/useTabContentDrop.ts (1)
6-36: Excellent API refinement.The hook rename to
useTabContentDropbetter describes its purpose, and theattachDropwrapper provides a cleaner integration point than exposing the rawdropref. The logic correctly prevents self-drops and validatestabToRenderexistence.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/index.tsx (2)
16-16: Improved encapsulation by internalizing active tab tracking.Removing the
focusedChildIdprop and derivingactiveTabIdinternally fromuseActiveTabIds()centralizes the active tab logic and simplifies the component's API. This aligns well with the broader refactoring pattern across the tab management system.Also applies to: 21-23, 44-44
54-55: Active tab logic correctly implemented.The
activeTabIdderivation and usage inrenderPaneproperly identifies the active child tab for focus styling. The dependency array update (line 103) correctly includesactiveTabIdto ensure the callback stays fresh.Also applies to: 80-104
| interface SingleTabViewProps { | ||
| tab: SingleTab; | ||
| isDropZone: boolean; | ||
| } |
There was a problem hiding this comment.
Remove unused isDropZone parameter.
The isDropZone prop is declared in the interface but never destructured or used in the component (Line 8 only destructures tab). Since the drop overlay is now rendered by the parent TabsContent component (see index.tsx lines 51-52), this parameter serves no purpose here.
Apply this diff to remove the unused parameter:
interface SingleTabViewProps {
tab: SingleTab;
- isDropZone: boolean;
}📝 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.
| interface SingleTabViewProps { | |
| tab: SingleTab; | |
| isDropZone: boolean; | |
| } | |
| interface SingleTabViewProps { | |
| tab: SingleTab; | |
| } |
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SingleTabView.tsx
around lines 3 to 6, the SingleTabViewProps interface declares an unused
isDropZone prop; remove isDropZone from the interface declaration and delete any
references to it in this file (e.g., prop lists/destructuring or default
values). Ensure the component still types correctly with only the tab prop and
run TypeScript checks to confirm no remaining usages of isDropZone in this
module.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SingleTabView.tsx (1)
4-7: Remove unusedisDropZoneparameter.The
isDropZoneprop is declared but never destructured or used in the component (Line 9 only destructurestab). Since the drop overlay is now rendered by the parentTabsContentcomponent, this parameter serves no purpose here.Apply this diff to remove the unused parameter:
interface SingleTabViewProps { tab: SingleTab; - isDropZone: boolean; }
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsx (1)
11-21: Remove unusedtabIdprop.The
tabIdprop is declared in the interface but never destructured or used in the component. Since all tab-specific logic is handled via the callbacks, this parameter is unnecessary.Apply this diff to remove the unused parameter:
interface TabContextMenuProps { children: ReactNode; - tabId: string; tabType: TabType; hasParent?: boolean; onClose: () => void; onRename: () => void; onDuplicate?: () => void; onUngroup?: () => void; onMoveOutOfGroup?: () => void; }apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SingleTabView.tsx (1)
10-33: Track placeholder implementations.Multiple TODO comments indicate incomplete functionality for rename, duplicate, split, and close operations. These placeholders should be implemented or tracked in issues to ensure they're not forgotten.
Would you like me to open tracking issues for these unimplemented features?
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/index.tsx (1)
81-104: Track placeholder implementations.The stub implementations for rename, duplicate, split, and close pane operations should be completed or tracked in issues.
Would you like me to open tracking issues for these unimplemented features?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/index.tsx(4 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SingleTabView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx(4 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/useTabRename.ts(1 hunks)apps/desktop/src/renderer/stores/tabs/store.ts(12 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/useTabRename.ts (1)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
useRenameTab(619-619)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsx (2)
apps/desktop/src/shared/types.ts (1)
TabType(23-30)packages/ui/src/components/context-menu.tsx (5)
ContextMenu(241-241)ContextMenuTrigger(242-242)ContextMenuContent(243-243)ContextMenuItem(244-244)ContextMenuSeparator(248-248)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx (1)
packages/ui/src/components/context-menu.tsx (5)
ContextMenu(241-241)ContextMenuTrigger(242-242)ContextMenuContent(243-243)ContextMenuItem(244-244)ContextMenuSeparator(248-248)
apps/desktop/src/renderer/stores/tabs/store.ts (3)
apps/desktop/src/renderer/stores/tabs/types.ts (1)
Tab(25-25)apps/desktop/src/shared/types.ts (1)
TabType(23-30)apps/desktop/src/renderer/stores/tabs/utils.ts (1)
getChildTabIds(6-8)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/index.tsx (4)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
useActiveTabIds(614-615)apps/desktop/src/renderer/stores/tabs/drag-logic.ts (1)
cleanLayout(48-75)apps/desktop/src/shared/types.ts (1)
MosaicNode(35-35)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx (1)
TabContentContextMenu(19-47)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SingleTabView.tsx (2)
apps/desktop/src/renderer/stores/tabs/types.ts (1)
SingleTab(16-18)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx (1)
TabContentContextMenu(19-47)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx (3)
apps/desktop/src/renderer/stores/tabs/store.ts (3)
useUngroupTabs(628-628)useUngroupTab(627-627)useTabs(613-613)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/useTabRename.ts (1)
useTabRename(4-61)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/TabContextMenu.tsx (1)
TabContextMenu(23-69)
🔇 Additional comments (9)
apps/desktop/src/renderer/stores/tabs/store.ts (4)
119-173: LGTM! Well-structured helper for empty group removal.The
handleEmptyGroupRemovalhelper effectively centralizes the logic for removing empty groups and updating related state (active tabs and history). The fallback chain for selecting the next active tab is sensible and comprehensive.
263-269: LGTM! Clean rename implementation.The
renameTabimplementation is straightforward and correctly updates the tab title immutably.
208-218: LGTM! Improved control flow with early exit.The early exit pattern and delegation to
removeChildTabFromGroupfor child tabs improves code clarity and ensures consistent empty group cleanup.
512-592: LGTM! Comprehensive ungroupTabs implementation.The
ungroupTabsimplementation correctly handles the multi-step process of ungrouping all child tabs, removing the group, reordering tabs to preserve position, and updating active tab state.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/useTabRename.ts (1)
1-61: LGTM! Well-designed rename hook.The
useTabRenamehook effectively encapsulates rename state management with excellent UX patterns: auto-select on edit, keyboard shortcuts (Enter/Escape), and proper validation with trimming. The synchronization with externaltabTitlechanges ensures the component stays in sync.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx (1)
1-47: LGTM! Clean context menu implementation.The
TabContentContextMenucomponent provides a well-structured menu with logical grouping (separators) and appropriate styling (destructive variant for Close Pane).apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupTabView/index.tsx (1)
16-16: LGTM! Improved active tab tracking.The refactor to derive
activeTabIdinternally usinguseActiveTabIdsremoves the need for prop drilling and simplifies the component's API. TherenderPanecallback correctly includes all necessary dependencies in its dependency array.Also applies to: 45-56, 106-146
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx (2)
70-83: LGTM! Correct parent index calculation.The
handleMoveOutOfGrouplogic correctly finds the parent group's index and places the ungrouped tab immediately after it (parentIndex + 1).
140-153: LGTM! Clean rename UI integration.The conditional rendering of the inline input during rename mode is well-implemented, with proper event handling (stopPropagation, onBlur, onKeyDown) to manage the rename flow.
| <Button | ||
| ref={attachRef} | ||
| variant="ghost" | ||
| onClick={handleTabClick} | ||
| onDoubleClick={rename.startRename} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| e.preventDefault(); | ||
| handleTabClick(); | ||
| } | ||
| }} | ||
| tabIndex={0} | ||
| className={` | ||
| w-full text-start group px-3 py-2 rounded-md cursor-pointer flex items-center justify-between | ||
| ${isActive ? "bg-sidebar-accent" : ""} | ||
| ${isDragging ? "opacity-50" : ""} | ||
| ${isDragOver ? "bg-sidebar-accent/50" : ""} | ||
| `} | ||
| > | ||
| <div className="flex items-center gap-1 flex-1 min-w-0"> | ||
| {isGroupTab && hasChildren && ( | ||
| > | ||
| <div className="flex items-center gap-1 flex-1 min-w-0"> | ||
| {isGroupTab && hasChildren && ( | ||
| <button | ||
| type="button" | ||
| onClick={handleToggleExpand} | ||
| onDoubleClick={(e) => { | ||
| e.stopPropagation(); | ||
| e.preventDefault(); | ||
| }} | ||
| className="shrink-0 cursor-pointer hover:opacity-80" | ||
| > | ||
| <HiChevronRight | ||
| className={`size-4 transition-transform ${isExpanded ? "rotate-90" : ""}`} | ||
| /> | ||
| </button> | ||
| )} | ||
| {rename.isRenaming ? ( | ||
| <input | ||
| ref={rename.inputRef} | ||
| type="text" | ||
| value={rename.renameValue} | ||
| onChange={(e) => rename.setRenameValue(e.target.value)} | ||
| onBlur={rename.submitRename} | ||
| onKeyDown={rename.handleKeyDown} | ||
| onClick={(e) => e.stopPropagation()} | ||
| className="flex-1 bg-sidebar-accent border border-primary rounded px-1 py-0.5 text-sm outline-none" | ||
| /> | ||
| ) : ( | ||
| <span className="truncate flex-1">{tab.title}</span> | ||
| )} | ||
| </div> | ||
| {!isGroupTab && ( | ||
| <button | ||
| type="button" | ||
| onClick={handleToggleExpand} | ||
| className="shrink-0 cursor-pointer hover:opacity-80" | ||
| onClick={handleRemoveTab} | ||
| className="cursor-pointer opacity-0 group-hover:opacity-100 ml-2 text-xs shrink-0" | ||
| > | ||
| <HiChevronRight | ||
| className={`size-4 transition-transform ${isExpanded ? "rotate-90" : ""}`} | ||
| /> | ||
| <HiMiniXMark className="size-4" /> | ||
| </button> | ||
| )} | ||
| <span className="truncate flex-1">{tab.title}</span> | ||
| </div> | ||
| {!isGroupTab && ( | ||
| <button | ||
| type="button" | ||
| onClick={handleRemoveTab} | ||
| className="opacity-0 group-hover:opacity-100 ml-2 text-xs shrink-0" | ||
| > | ||
| <HiMiniXMark className="size-4" /> | ||
| </button> | ||
| )} | ||
| </Button> | ||
| </Button> |
There was a problem hiding this comment.
Fix nested button elements.
Lines 155-163 render a <button> element inside the <Button> component (which also renders as a button). Nested buttons are invalid HTML and cause accessibility issues, unpredictable click behavior, and keyboard navigation problems.
Apply this diff to fix the nesting issue by using a div instead of the inner button:
{!isGroupTab && (
- <button
+ <div
type="button"
onClick={handleRemoveTab}
className="cursor-pointer opacity-0 group-hover:opacity-100 ml-2 text-xs shrink-0"
>
<HiMiniXMark className="size-4" />
- </button>
+ </div>
)}Alternatively, you can use role="button" on a span/div for better semantics, or extract the close button outside the main Button component.
📝 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.
| <Button | |
| ref={attachRef} | |
| variant="ghost" | |
| onClick={handleTabClick} | |
| onDoubleClick={rename.startRename} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| handleTabClick(); | |
| } | |
| }} | |
| tabIndex={0} | |
| className={` | |
| w-full text-start group px-3 py-2 rounded-md cursor-pointer flex items-center justify-between | |
| ${isActive ? "bg-sidebar-accent" : ""} | |
| ${isDragging ? "opacity-50" : ""} | |
| ${isDragOver ? "bg-sidebar-accent/50" : ""} | |
| `} | |
| > | |
| <div className="flex items-center gap-1 flex-1 min-w-0"> | |
| {isGroupTab && hasChildren && ( | |
| > | |
| <div className="flex items-center gap-1 flex-1 min-w-0"> | |
| {isGroupTab && hasChildren && ( | |
| <button | |
| type="button" | |
| onClick={handleToggleExpand} | |
| onDoubleClick={(e) => { | |
| e.stopPropagation(); | |
| e.preventDefault(); | |
| }} | |
| className="shrink-0 cursor-pointer hover:opacity-80" | |
| > | |
| <HiChevronRight | |
| className={`size-4 transition-transform ${isExpanded ? "rotate-90" : ""}`} | |
| /> | |
| </button> | |
| )} | |
| {rename.isRenaming ? ( | |
| <input | |
| ref={rename.inputRef} | |
| type="text" | |
| value={rename.renameValue} | |
| onChange={(e) => rename.setRenameValue(e.target.value)} | |
| onBlur={rename.submitRename} | |
| onKeyDown={rename.handleKeyDown} | |
| onClick={(e) => e.stopPropagation()} | |
| className="flex-1 bg-sidebar-accent border border-primary rounded px-1 py-0.5 text-sm outline-none" | |
| /> | |
| ) : ( | |
| <span className="truncate flex-1">{tab.title}</span> | |
| )} | |
| </div> | |
| {!isGroupTab && ( | |
| <button | |
| type="button" | |
| onClick={handleToggleExpand} | |
| className="shrink-0 cursor-pointer hover:opacity-80" | |
| onClick={handleRemoveTab} | |
| className="cursor-pointer opacity-0 group-hover:opacity-100 ml-2 text-xs shrink-0" | |
| > | |
| <HiChevronRight | |
| className={`size-4 transition-transform ${isExpanded ? "rotate-90" : ""}`} | |
| /> | |
| <HiMiniXMark className="size-4" /> | |
| </button> | |
| )} | |
| <span className="truncate flex-1">{tab.title}</span> | |
| </div> | |
| {!isGroupTab && ( | |
| <button | |
| type="button" | |
| onClick={handleRemoveTab} | |
| className="opacity-0 group-hover:opacity-100 ml-2 text-xs shrink-0" | |
| > | |
| <HiMiniXMark className="size-4" /> | |
| </button> | |
| )} | |
| </Button> | |
| </Button> | |
| <Button | |
| ref={attachRef} | |
| variant="ghost" | |
| onClick={handleTabClick} | |
| onDoubleClick={rename.startRename} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| handleTabClick(); | |
| } | |
| }} | |
| tabIndex={0} | |
| className={` | |
| w-full text-start group px-3 py-2 rounded-md cursor-pointer flex items-center justify-between | |
| ${isActive ? "bg-sidebar-accent" : ""} | |
| ${isDragging ? "opacity-50" : ""} | |
| ${isDragOver ? "bg-sidebar-accent/50" : ""} | |
| `} | |
| > | |
| <div className="flex items-center gap-1 flex-1 min-w-0"> | |
| {isGroupTab && hasChildren && ( | |
| <button | |
| type="button" | |
| onClick={handleToggleExpand} | |
| onDoubleClick={(e) => { | |
| e.stopPropagation(); | |
| e.preventDefault(); | |
| }} | |
| className="shrink-0 cursor-pointer hover:opacity-80" | |
| > | |
| <HiChevronRight | |
| className={`size-4 transition-transform ${isExpanded ? "rotate-90" : ""}`} | |
| /> | |
| </button> | |
| )} | |
| {rename.isRenaming ? ( | |
| <input | |
| ref={rename.inputRef} | |
| type="text" | |
| value={rename.renameValue} | |
| onChange={(e) => rename.setRenameValue(e.target.value)} | |
| onBlur={rename.submitRename} | |
| onKeyDown={rename.handleKeyDown} | |
| onClick={(e) => e.stopPropagation()} | |
| className="flex-1 bg-sidebar-accent border border-primary rounded px-1 py-0.5 text-sm outline-none" | |
| /> | |
| ) : ( | |
| <span className="truncate flex-1">{tab.title}</span> | |
| )} | |
| </div> | |
| {!isGroupTab && ( | |
| <div | |
| type="button" | |
| onClick={handleRemoveTab} | |
| className="cursor-pointer opacity-0 group-hover:opacity-100 ml-2 text-xs shrink-0" | |
| > | |
| <HiMiniXMark className="size-4" /> | |
| </div> | |
| )} | |
| </Button> |
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx
around lines 105-164 there is a nested <button> rendered inside the outer
<Button> component (lines ~155-163), which is invalid HTML and breaks
accessibility and keyboard behavior; replace the inner button used for expand
toggle and the remove/close control with non-button elements (e.g., a div or
span) and add role="button", tabIndex={0}, onKeyDown handlers that trigger the
same actions for Enter/Space, and preserve existing onClick, onDoubleClick,
stopPropagation, aria-labels, and classes so semantics and keyboard
accessibility remain intact. Ensure any visual cursor and hover styles remain
and remove the nested button tag only.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.