[codex] Improve sidebar group management UX#3745
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughIntroduces a shared rename-request provider for sidebar sections, centralizes rename initiation and auto-rename on creation, extracts section action menu items into reusable context/dropdown components, updates UI copy from "Section" → "group", and refactors sidebar ordering to treat ungrouped workspaces and sections as a single top-level sequence. Changes
Sequence DiagramsequenceDiagram
participant User
participant ProjectSection as ProjectSectionComponent
participant Hook as useDashboardSidebarProjectSectionActions
participant Provider as RenameProvider
participant API as ServerAPI
User->>ProjectSection: Click "New group"
ProjectSection->>Hook: handleNewSection()
Hook->>API: createSection("New group")
API-->>Hook: { sectionId }
Hook->>Provider: requestSectionRename(sectionId)
Provider-->>ProjectSection: pendingRenameSectionId set
ProjectSection->>ProjectSection: Start rename UI for sectionId
User->>ProjectSection: Submit new name
ProjectSection->>API: updateSectionName(sectionId, name)
API-->>ProjectSection: success
ProjectSection->>Provider: clearPendingSectionRename(sectionId)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx (1)
41-62:⚠️ Potential issue | 🟡 MinorKeyboard activation on the actions trigger will also toggle the section.
The header
<div>handlesEnter/Spaceto callonToggleCollapse. The actions dropdown trigger button (rendered inside this div) only stops propagation onclick/pointerdown/contextmenu— not onkeydown. When a keyboard user focuses the ellipsis button and presses Enter (or Space) to open the menu, thekeydownstill bubbles to this header and toggles collapse alongside opening the dropdown.Two options:
- Stop
keydownpropagation on the dropdown trigger button (inDashboardSidebarSectionActionsDropdown.tsx), or- Guard this handler so it ignores events whose
event.targetis not the header itself (e.g.,if (event.currentTarget !== event.target) return;).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx` around lines 41 - 62, The header's onKeyDown in DashboardSidebarSectionHeader is catching Enter/Space from child controls (like the ellipsis actions button) and toggling collapse; update the onKeyDown handler in DashboardSidebarSectionHeader to ignore key events not targeted at the header itself by returning early when event.currentTarget !== event.target before handling Enter/Space, so only direct key activation of the header toggles collapse; alternatively, if you prefer handling in the actions component, add an onKeyDown on the dropdown trigger in DashboardSidebarSectionActionsDropdown to stop propagation for Enter/Space (i.e., call event.stopPropagation() for those keys) so opening the menu doesn't bubble to onToggleCollapse.
🧹 Nitpick comments (4)
apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts (1)
304-347: Optional: unify theinsertAfterWorkspaceIdbranch with the new top-level ordering helpers.The other mutations (
moveWorkspaceToSection,deleteSection, no-anchorcreateSection) all funnel throughgetProjectTopLevelItems+writeProjectTopLevelOrder. This branch still uses the older "manually bump tabOrder by 1" approach, which works but mixes models. Consider building the unified list, splicing the new section after the anchor, and rewriting viawriteProjectTopLevelOrder— it deduplicates the in-section vs. top-level anchor logic and avoids the dual update loops.Sketch (illustrative):
const anchorWorkspace = collections.v2WorkspaceLocalState.get(insertAfterWorkspaceId); const anchorId = anchorWorkspace?.sidebarState.sectionId ?? insertAfterWorkspaceId; const anchorType: ProjectTopLevelItem["type"] = anchorWorkspace?.sidebarState.sectionId ? "section" : "workspace"; const items = getProjectTopLevelItems(collections, projectId); const anchorIndex = items.findIndex(i => i.type === anchorType && i.id === anchorId); const insertIndex = anchorIndex === -1 ? items.length : anchorIndex + 1; items.splice(insertIndex, 0, { type: "section", id: sectionId, tabOrder: 0 }); writeProjectTopLevelOrder(collections, projectId, items); tabOrder = insertIndex + 1;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts` around lines 304 - 347, Unify the insertAfterWorkspaceId branch with the top-level ordering helpers: instead of manually incrementing tabOrder via two loops on collections.v2WorkspaceLocalState and collections.v2SidebarSections, build the project top-level list with getProjectTopLevelItems(collections, projectId), compute the anchor by mapping insertAfterWorkspaceId to either a section id or workspace id (mirroring the existing anchor logic), find anchorIndex, splice a new {type: "section", id: sectionId, tabOrder: 0} into items at anchorIndex+1 (or push if not found), then call writeProjectTopLevelOrder(collections, projectId, items) to persist the order and set tabOrder = insertIndex + 1; this keeps behavior consistent with moveWorkspaceToSection, deleteSection and the no-anchor createSection flow.apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/DashboardSidebarSection.tsx (1)
61-82: DRY: samecolor/onRename/onSetColor/onDeletewiring duplicated for context menu and dropdown.The four action handlers are passed identically to
DashboardSidebarSectionContextMenuandDashboardSidebarSectionActionsDropdown. Consider extracting a singlesectionActionsPropsobject (typed asDashboardSidebarSectionActionsProps) to spread into both, which also keeps them in lockstep going forward.♻️ Proposed refactor
+const sectionActionsProps: DashboardSidebarSectionActionsProps = { + color: section.color, + onRename: startRename, + onSetColor: (color) => setSectionColor(section.id, color), + onDelete: () => onDelete(section.id), +}; + return ( <div style={sectionBorderStyle}> - <DashboardSidebarSectionContextMenu - color={section.color} - onRename={startRename} - onSetColor={(color) => setSectionColor(section.id, color)} - onDelete={() => onDelete(section.id)} - > + <DashboardSidebarSectionContextMenu {...sectionActionsProps}> <DashboardSidebarSectionHeader ... - actions={ - <DashboardSidebarSectionActionsDropdown - color={section.color} - onRename={startRename} - onSetColor={(color) => setSectionColor(section.id, color)} - onDelete={() => onDelete(section.id)} - /> - } + actions={<DashboardSidebarSectionActionsDropdown {...sectionActionsProps} />} /> </DashboardSidebarSectionContextMenu>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/DashboardSidebarSection.tsx` around lines 61 - 82, Extract the repeated props into a single object so the ContextMenu and ActionsDropdown stay in sync: create a const sectionActionsProps typed as DashboardSidebarSectionActionsProps containing color: section.color, onRename: startRename, onSetColor: (color) => setSectionColor(section.id, color), and onDelete: () => onDelete(section.id), then spread {...sectionActionsProps} into both DashboardSidebarSectionContextMenu and DashboardSidebarSectionActionsDropdown instead of passing the four props twice.apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx (1)
63-71: Decorative chevron/grip lack a discernible accessibility hint.The chevron previously lived in a dedicated toggle button with implicit semantics; now the header
divcarries thebuttonrole and the chevron is purely decorative. Consider addingaria-expanded={!section.isCollapsed}to the headerdivso screen readers announce the collapsed/expanded state. (Currently only the rotating icon conveys it.)♿ Proposed change
<div ref={ref} role={isRenaming ? undefined : "button"} tabIndex={isRenaming ? undefined : 0} + aria-expanded={isRenaming ? undefined : !section.isCollapsed} onClick={isRenaming ? undefined : onToggleCollapse}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx` around lines 63 - 71, In DashboardSidebarSectionHeader, the header div that acts as the toggle (the container rendering HiChevronRight and LuGripVertical and referencing section.isCollapsed) lacks a machine-readable expanded state; add aria-expanded={!section.isCollapsed} to that header div (the element that currently carries role="button") so assistive tech can announce collapse/expand changes for the toggle implemented in DashboardSidebarSectionHeader and tied to section.isCollapsed.apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx (1)
87-113: Consider memoizingcolorOptions/colorItems.
colorOptionsand the resultingcolorItemsJSX are rebuilt on every render even though their inputs (PROJECT_COLORS,PROJECT_COLOR_DEFAULT, pluscolor/handlers) rarely change. Wrapping them inuseMemo(or hoistingcolorOptionsto module scope, since it's static) avoids needless work when the parent re-renders for unrelated reasons. Optional polish.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx` around lines 87 - 113, The colorOptions array and the colorItems JSX are being recreated on every render; wrap their creation in React.useMemo (or hoist static PROJECT_COLORS/PROJECT_COLOR_DEFAULT to module scope) to avoid unnecessary recomputation inside the DashboardSidebarSectionContextMenu component: memoize the computation that maps PROJECT_COLORS to colorOptions and the map that produces colorItems (which uses selectedValue, onSetColor, renderItem) so they only recompute when their real dependencies change (PROJECT_COLORS/PROJECT_COLOR_DEFAULT, selectedValue, onSetColor, renderItem).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/DashboardSidebarSectionActionsDropdown.tsx`:
- Around line 1-47: Create a barrel export for the
DashboardSidebarSectionActionsDropdown component by adding an index.ts in the
same folder that re-exports the component; ensure it exports the named symbol
DashboardSidebarSectionActionsDropdown so imports elsewhere can use the folder
path rather than the full file path and update any imports if necessary to use
the barrel.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx`:
- Around line 152-165: The trash icon coloring is inconsistent because in the
context menu you add text-destructive directly to LuTrash2 but in the dropdown
you rely on DropdownMenuItem variant="destructive" to color descendants; update
the rendering in SectionActionsMenuItems so LuTrash2 always receives the
destructive class when the action is destructive (use the existing destructive
prop or check kind) so the icon gets text-destructive in both flows (look for
renderItem usage, the onDelete action, LuTrash2, and DropdownMenuItem
variant="destructive" to apply the change).
---
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx`:
- Around line 41-62: The header's onKeyDown in DashboardSidebarSectionHeader is
catching Enter/Space from child controls (like the ellipsis actions button) and
toggling collapse; update the onKeyDown handler in DashboardSidebarSectionHeader
to ignore key events not targeted at the header itself by returning early when
event.currentTarget !== event.target before handling Enter/Space, so only direct
key activation of the header toggles collapse; alternatively, if you prefer
handling in the actions component, add an onKeyDown on the dropdown trigger in
DashboardSidebarSectionActionsDropdown to stop propagation for Enter/Space
(i.e., call event.stopPropagation() for those keys) so opening the menu doesn't
bubble to onToggleCollapse.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx`:
- Around line 87-113: The colorOptions array and the colorItems JSX are being
recreated on every render; wrap their creation in React.useMemo (or hoist static
PROJECT_COLORS/PROJECT_COLOR_DEFAULT to module scope) to avoid unnecessary
recomputation inside the DashboardSidebarSectionContextMenu component: memoize
the computation that maps PROJECT_COLORS to colorOptions and the map that
produces colorItems (which uses selectedValue, onSetColor, renderItem) so they
only recompute when their real dependencies change
(PROJECT_COLORS/PROJECT_COLOR_DEFAULT, selectedValue, onSetColor, renderItem).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx`:
- Around line 63-71: In DashboardSidebarSectionHeader, the header div that acts
as the toggle (the container rendering HiChevronRight and LuGripVertical and
referencing section.isCollapsed) lacks a machine-readable expanded state; add
aria-expanded={!section.isCollapsed} to that header div (the element that
currently carries role="button") so assistive tech can announce collapse/expand
changes for the toggle implemented in DashboardSidebarSectionHeader and tied to
section.isCollapsed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/DashboardSidebarSection.tsx`:
- Around line 61-82: Extract the repeated props into a single object so the
ContextMenu and ActionsDropdown stay in sync: create a const sectionActionsProps
typed as DashboardSidebarSectionActionsProps containing color: section.color,
onRename: startRename, onSetColor: (color) => setSectionColor(section.id,
color), and onDelete: () => onDelete(section.id), then spread
{...sectionActionsProps} into both DashboardSidebarSectionContextMenu and
DashboardSidebarSectionActionsDropdown instead of passing the four props twice.
In
`@apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts`:
- Around line 304-347: Unify the insertAfterWorkspaceId branch with the
top-level ordering helpers: instead of manually incrementing tabOrder via two
loops on collections.v2WorkspaceLocalState and collections.v2SidebarSections,
build the project top-level list with getProjectTopLevelItems(collections,
projectId), compute the anchor by mapping insertAfterWorkspaceId to either a
section id or workspace id (mirroring the existing anchor logic), find
anchorIndex, splice a new {type: "section", id: sectionId, tabOrder: 0} into
items at anchorIndex+1 (or push if not found), then call
writeProjectTopLevelOrder(collections, projectId, items) to persist the order
and set tabOrder = insertIndex + 1; this keeps behavior consistent with
moveWorkspaceToSection, deleteSection and the no-anchor createSection flow.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 453272d3-6f62-44be-8662-7e09bff53a19
📒 Files selected for processing (18)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectContextMenu/DashboardSidebarProjectContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/hooks/useDashboardSidebarProjectSectionActions/useDashboardSidebarProjectSectionActions.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/DashboardSidebarSection.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/DashboardSidebarSectionContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/DashboardSidebarSectionActionsDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/types.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionRenameContext/DashboardSidebarSectionRenameContext.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionRenameContext/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceContextMenu/DashboardSidebarWorkspaceContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/SortableSectionHeader/SortableSectionHeader.tsxapps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts
Comment on lines
+1
to
+47
| import { | ||
| DropdownMenu, | ||
| DropdownMenuContent, | ||
| DropdownMenuTrigger, | ||
| } from "@superset/ui/dropdown-menu"; | ||
| import { LuEllipsis } from "react-icons/lu"; | ||
| import type { DashboardSidebarSectionActionsProps } from "../../types"; | ||
| import { SectionActionsMenuItems } from "../SectionActionsMenuItems"; | ||
|
|
||
| export function DashboardSidebarSectionActionsDropdown({ | ||
| color, | ||
| onRename, | ||
| onSetColor, | ||
| onDelete, | ||
| }: DashboardSidebarSectionActionsProps) { | ||
| return ( | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <button | ||
| type="button" | ||
| onPointerDown={(event) => event.stopPropagation()} | ||
| onClick={(event) => event.stopPropagation()} | ||
| onContextMenu={(event) => event.stopPropagation()} | ||
| className="flex size-5 shrink-0 items-center justify-center rounded text-muted-foreground/80 opacity-0 transition-[opacity,color,background-color] hover:bg-muted hover:text-foreground group-hover:opacity-100 focus-visible:opacity-100 data-[state=open]:opacity-100" | ||
| aria-label="Group actions" | ||
| > | ||
| <LuEllipsis className="size-3.5" /> | ||
| </button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent | ||
| align="end" | ||
| className="w-44" | ||
| onCloseAutoFocus={(event) => event.preventDefault()} | ||
| onClick={(event) => event.stopPropagation()} | ||
| onPointerDown={(event) => event.stopPropagation()} | ||
| > | ||
| <SectionActionsMenuItems | ||
| color={color} | ||
| kind="dropdown" | ||
| onRename={onRename} | ||
| onSetColor={onSetColor} | ||
| onDelete={onDelete} | ||
| /> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'index.ts' --full-path '*DashboardSidebarSectionActionsDropdown*'
fd -t f -a 'DashboardSidebarSectionActionsDropdown'Repository: superset-sh/superset
Length of output: 490
Create missing index.ts barrel export for this component folder.
The component file exists at DashboardSidebarSectionActionsDropdown/DashboardSidebarSectionActionsDropdown.tsx, but the required index.ts barrel export is missing. Per coding guidelines, all components must use the structure: ComponentName/ComponentName.tsx + index.ts for barrel export.
Add index.ts:
export { DashboardSidebarSectionActionsDropdown } from "./DashboardSidebarSectionActionsDropdown";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/DashboardSidebarSectionActionsDropdown.tsx`
around lines 1 - 47, Create a barrel export for the
DashboardSidebarSectionActionsDropdown component by adding an index.ts in the
same folder that re-exports the component; ensure it exports the named symbol
DashboardSidebarSectionActionsDropdown so imports elsewhere can use the folder
path rather than the full file path and update any imports if necessary to use
the barrel.
Kitenite
force-pushed
the
sections-sidebar-ux
branch
from
f44d41b to
434f3bf
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR improves the v2 dashboard sidebar group management UX by: adding hover-only action dropdowns on section headers, unifying context-menu and dropdown actions through a shared Confidence Score: 5/5Safe to merge; one minor dead-code comment is the only finding. All findings are P2. The state-management refactor is logically sound — apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts — dead
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts | Core state-management refactor; new helper functions correctly handle top-level reordering, but the insertAfterWorkspaceId option in createSection is now dead code with no callers. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionRenameContext/DashboardSidebarSectionRenameContext.tsx | New React context for cross-component rename signalling; clean implementation with correct functional setState in clearPendingSectionRename. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/SortableSectionHeader/SortableSectionHeader.tsx | Correctly consumes rename context via useEffect; startRename is memoized with section.name as a dep; no infinite-loop risk in the effect. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx | New shared component driving both context-menu and dropdown surfaces; key props on colorItems are correctly applied inside the renderItem helper. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/DashboardSidebarSectionActionsDropdown.tsx | New hover-only actions dropdown; event propagation is correctly stopped on onPointerDown, onClick, and onContextMenu to prevent triggering collapse or context menu. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx | Header refactored to make the entire row the collapse toggle target; drag handle and chevron share the same grid cell with opacity cross-fade on hover; actions slot replaces onStartRename callback. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts | "New group from workspace" now creates the section then immediately moves the workspace into it and requests rename — cleaner than the prior insertAfterWorkspaceId approach. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/hooks/useDashboardSidebarProjectSectionActions/useDashboardSidebarProjectSectionActions.ts | Project-level "New group" now requests rename and auto-expands the project if collapsed — correct ordering since the section isn't rendered until the project expands. |
Sequence Diagram
sequenceDiagram
participant User
participant ProjectActions as useDashboardSidebarProjectSectionActions
participant WorkspaceActions as useDashboardSidebarWorkspaceItemActions
participant SidebarState as useDashboardSidebarState
participant RenameCtx as DashboardSidebarSectionRenameContext
participant Header as SortableSectionHeader
Note over User,Header: "New group" from project context menu
User->>ProjectActions: onCreateSection()
ProjectActions->>SidebarState: createSection(projectId)
SidebarState-->>ProjectActions: sectionId
ProjectActions->>RenameCtx: requestSectionRename(sectionId)
ProjectActions->>SidebarState: toggleProjectCollapsed(projectId) [if collapsed]
Note over Header: Section renders in expanded project
RenameCtx-->>Header: pendingRenameSectionId === section.id
Header->>Header: useEffect → startRename()
Header->>RenameCtx: clearPendingSectionRename(sectionId)
Note over User,Header: "New group from workspace" from workspace context menu
User->>WorkspaceActions: handleCreateSection()
WorkspaceActions->>SidebarState: createSection(projectId)
SidebarState-->>WorkspaceActions: sectionId
WorkspaceActions->>SidebarState: moveWorkspaceToSection(workspaceId, projectId, sectionId)
WorkspaceActions->>RenameCtx: requestSectionRename(sectionId)
RenameCtx-->>Header: pendingRenameSectionId === section.id
Header->>Header: useEffect → startRename()
Header->>RenameCtx: clearPendingSectionRename(sectionId)
Note over User,Header: "Delete group" / "Ungroup"
User->>Header: onDelete / onMoveToSection(null)
Header->>SidebarState: deleteSection(sectionId) / moveWorkspaceToSection(id, pid, null)
SidebarState->>SidebarState: getProjectTopLevelItems (excl. deleted)
SidebarState->>SidebarState: splice workspaces before first section
SidebarState->>SidebarState: writeProjectTopLevelOrder (full reindex)
Comments Outside Diff (1)
-
apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts, line 293-362 (link)Dead code:
insertAfterWorkspaceIdoptionThe
insertAfterWorkspaceIdoption is no longer passed by any caller —handleCreateSectioninuseDashboardSidebarWorkspaceItemActionswas its only call site and was updated to usecreateSection+moveWorkspaceToSectioninstead. The entireif (insertAfterWorkspaceId) { … }branch (lines 305–342) is now unreachable. Consider removing the option and the branch to avoid confusion about supported behaviour.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts Line: 293-362 Comment: **Dead code: `insertAfterWorkspaceId` option** The `insertAfterWorkspaceId` option is no longer passed by any caller — `handleCreateSection` in `useDashboardSidebarWorkspaceItemActions` was its only call site and was updated to use `createSection` + `moveWorkspaceToSection` instead. The entire `if (insertAfterWorkspaceId) { … }` branch (lines 305–342) is now unreachable. Consider removing the option and the branch to avoid confusion about supported behaviour. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts
Line: 293-362
Comment:
**Dead code: `insertAfterWorkspaceId` option**
The `insertAfterWorkspaceId` option is no longer passed by any caller — `handleCreateSection` in `useDashboardSidebarWorkspaceItemActions` was its only call site and was updated to use `createSection` + `moveWorkspaceToSection` instead. The entire `if (insertAfterWorkspaceId) { … }` branch (lines 305–342) is now unreachable. Consider removing the option and the branch to avoid confusion about supported behaviour.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Improve sidebar group management UX" | Re-trigger Greptile
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts (2)
88-110:writeProjectTopLevelOrderforcessectionId = nullandprojectId = projectIdon every workspace item — make sure callers only pass workspaces that should be ungrouped.This silently moves any passed-in workspace to ungrouped/
projectId. Today both call sites (moveWorkspaceToSectionungroup,deleteSection) only pass top-level/orphaned workspaces, so this is fine, but the function name doesn't communicate that it also mutates sectionId/projectId. A short JSDoc clarifying this contract would help future callers avoid accidentally re-parenting workspaces.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts` around lines 88 - 110, writeProjectTopLevelOrder currently resets workspace sidebar state (it sets draft.sidebarState.projectId = projectId and draft.sidebarState.sectionId = null) which re-parents workspaces to the provided project; add a short JSDoc above writeProjectTopLevelOrder stating its contract: that it mutates workspace sidebarState to assign the given projectId and clear sectionId (i.e., unnests/ungroups the workspace), and therefore callers such as moveWorkspaceToSection and deleteSection must only pass workspaces that are intended to be ungrouped; ensure the JSDoc names the mutated fields (sidebarState.projectId and sidebarState.sectionId) so future callers won’t accidentally re-parent workspaces.
290-347: Optional: anchor branch ofcreateSectioncould reuse the new top-level helpers.The
elsebranch (Lines 343-347) now derivestabOrderfromgetProjectTopLevelItems, but theinsertAfterWorkspaceIdbranch (Lines 305-342) still hand-rolls two parallel loops to bump workspace and section orders. You can collapse that to a single splice into the combined list followed bywriteProjectTopLevelOrder, which would unify the two code paths and remove the manual+1bumping. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts` around lines 290 - 347, The insertAfterWorkspaceId branch inside createSection currently manually iterates v2WorkspaceLocalState and v2SidebarSections to bump tabOrder; replace that with the same top-level helpers used in the else branch: call getProjectTopLevelItems(collections, projectId) to build the combined ordered list, find the anchor index using insertAfterWorkspaceId (by matching workspaceId or sectionId as appropriate), splice the new sectionId into that array at anchorIndex+1, then call writeProjectTopLevelOrder(collections, projectId, newOrderedArray) to persist all tabOrders in one pass; remove the two for-loops and the manual +1 increment logic and set tabOrder to the computed position from the newOrderedArray (or use getNextTabOrder on the updated array if needed).apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts (1)
80-86: Minor: drop the redundantname: "New group"override.
createSection's defaultnamewas just changed to"New group"inuseDashboardSidebarState.ts, so passing it explicitly here duplicates the literal. If the default ever changes again, both call sites need to be kept in sync.♻️ Proposed cleanup
const handleCreateSection = () => { - const sectionId = createSection(projectId, { - name: "New group", - }); + const sectionId = createSection(projectId); moveWorkspaceToSection(workspaceId, projectId, sectionId); requestSectionRename(sectionId); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts` around lines 80 - 86, Remove the redundant explicit name override when creating a section: in useDashboardSidebarWorkspaceItemActions, update handleCreateSection so it calls createSection(projectId) without passing { name: "New group" } (since createSection already defaults name to "New group" in useDashboardSidebarState.ts), then continue to call moveWorkspaceToSection(workspaceId, projectId, sectionId) and requestSectionRename(sectionId) as before.apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx (1)
2-17: ImportReactNodedirectly instead of using theReact.*namespace.
React.ReactNoderelies on a globalReactnamespace that isn't imported here (onlyComponentPropsWithoutRefandforwardRefare). This works only if the project's tsconfig/global types still expose theReactnamespace — fragile under modernjsx: "react-jsx"setups and inconsistent with the rest of this file.♻️ Proposed fix
-import { type ComponentPropsWithoutRef, forwardRef } from "react"; +import { type ComponentPropsWithoutRef, forwardRef, type ReactNode } from "react"; @@ - actions?: React.ReactNode; + actions?: ReactNode;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx` around lines 2 - 17, The props type uses the global React namespace for the actions prop (actions?: React.ReactNode) but React isn't imported; update the import list from "react" to also pull in ReactNode and change the actions type to use that imported symbol (e.g., actions?: ReactNode) inside the DashboardSidebarSectionHeaderProps interface in DashboardSidebarSectionHeader.tsx so the type does not rely on a global React namespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx`:
- Around line 86-93: The header's outer interactive region (role="button",
onClick={onToggleCollapse}, Space/Enter handlers) allows clicks and key events
from the actions slot to bubble up and toggle collapse; to fix, render the
actions slot (the actions variable) inside a non-interactive container that
stops propagation of mouse and keyboard events (e.g., wrap actions in a <div>
and call event.stopPropagation() on onClick and onKeyDown, specifically
preventing Space/Enter from bubbling) so callers don't need to handle
propagation; update the DashboardSidebarSectionHeader component where isRenaming
and actions are used and ensure onToggleCollapse and the header's keyboard
handlers remain unchanged.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx`:
- Around line 2-17: The props type uses the global React namespace for the
actions prop (actions?: React.ReactNode) but React isn't imported; update the
import list from "react" to also pull in ReactNode and change the actions type
to use that imported symbol (e.g., actions?: ReactNode) inside the
DashboardSidebarSectionHeaderProps interface in
DashboardSidebarSectionHeader.tsx so the type does not rely on a global React
namespace.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.ts`:
- Around line 80-86: Remove the redundant explicit name override when creating a
section: in useDashboardSidebarWorkspaceItemActions, update handleCreateSection
so it calls createSection(projectId) without passing { name: "New group" }
(since createSection already defaults name to "New group" in
useDashboardSidebarState.ts), then continue to call
moveWorkspaceToSection(workspaceId, projectId, sectionId) and
requestSectionRename(sectionId) as before.
In
`@apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts`:
- Around line 88-110: writeProjectTopLevelOrder currently resets workspace
sidebar state (it sets draft.sidebarState.projectId = projectId and
draft.sidebarState.sectionId = null) which re-parents workspaces to the provided
project; add a short JSDoc above writeProjectTopLevelOrder stating its contract:
that it mutates workspace sidebarState to assign the given projectId and clear
sectionId (i.e., unnests/ungroups the workspace), and therefore callers such as
moveWorkspaceToSection and deleteSection must only pass workspaces that are
intended to be ungrouped; ensure the JSDoc names the mutated fields
(sidebarState.projectId and sidebarState.sectionId) so future callers won’t
accidentally re-parent workspaces.
- Around line 290-347: The insertAfterWorkspaceId branch inside createSection
currently manually iterates v2WorkspaceLocalState and v2SidebarSections to bump
tabOrder; replace that with the same top-level helpers used in the else branch:
call getProjectTopLevelItems(collections, projectId) to build the combined
ordered list, find the anchor index using insertAfterWorkspaceId (by matching
workspaceId or sectionId as appropriate), splice the new sectionId into that
array at anchorIndex+1, then call writeProjectTopLevelOrder(collections,
projectId, newOrderedArray) to persist all tabOrders in one pass; remove the two
for-loops and the manual +1 increment logic and set tabOrder to the computed
position from the newOrderedArray (or use getNextTabOrder on the updated array
if needed).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9e90f017-6609-4149-8d4a-1d7385de853c
📒 Files selected for processing (21)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectContextMenu/DashboardSidebarProjectContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/hooks/useDashboardSidebarProjectSectionActions/useDashboardSidebarProjectSectionActions.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/DashboardSidebarSection.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContent/DashboardSidebarSectionContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContent/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/DashboardSidebarSectionContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/DashboardSidebarSectionActionsDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/types.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionRenameContext/DashboardSidebarSectionRenameContext.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionRenameContext/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceContextMenu/DashboardSidebarWorkspaceContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/SortableSectionHeader/SortableSectionHeader.tsxapps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts
💤 Files with no reviewable changes (4)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContent/DashboardSidebarSectionContent.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContent/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/DashboardSidebarSection.tsx
✅ Files skipped from review due to trivial changes (6)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectContextMenu/DashboardSidebarProjectContextMenu.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceContextMenu/DashboardSidebarWorkspaceContextMenu.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/DashboardSidebarSectionActionsDropdown.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/types.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionRenameContext/DashboardSidebarSectionRenameContext.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionRenameContext/index.ts
Kitenite
force-pushed
the
sections-sidebar-ux
branch
from
434f3bf to
a3c7651
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx (1)
67-75: Minor:cursor-grabon the chevron/grip wrapper, but the area also toggles collapse on click.The inner
divadvertises grab affordance viacursor-grab/active:cursor-grabbing, but since{...listeners}/{...attributes}are spread onto the outer header (inSortableSectionHeader), the entire header is draggable and a click on the chevron/grip area will bubble to the header'sonClick={onToggleCollapse}. In practice, dnd-kit'sMouseSensoractivation distance (8px) makes a quick click toggle collapse — which is probably the intent — but the cursor hint can read as "drag-only" when this region also toggles. Consider whether this should be visually presented as a chevron toggle (e.g.,cursor-pointer) so users don't expect drag-only behavior in that 5x5 area.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx` around lines 67 - 75, The chevron/grip wrapper in DashboardSidebarSectionHeader currently uses "cursor-grab" / "active:cursor-grabbing" which suggests drag-only but the outer SortableSectionHeader spreads {...listeners} / {...attributes} and also handles onClick={onToggleCollapse}; update the inner div's className to use a click affordance (e.g., "cursor-pointer") instead of "cursor-grab"/"active:cursor-grabbing" so the small 5x5 area clearly indicates it toggles collapse; adjust the class on the div containing HiChevronRight and LuGripVertical (and any conditional classes referencing section.isCollapsed) accordingly (or make it conditional: use cursor-pointer when the header handles toggle, and cursor-grab only if you detect a pure drag-only mode).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsx`:
- Around line 67-75: The chevron/grip wrapper in DashboardSidebarSectionHeader
currently uses "cursor-grab" / "active:cursor-grabbing" which suggests drag-only
but the outer SortableSectionHeader spreads {...listeners} / {...attributes} and
also handles onClick={onToggleCollapse}; update the inner div's className to use
a click affordance (e.g., "cursor-pointer") instead of
"cursor-grab"/"active:cursor-grabbing" so the small 5x5 area clearly indicates
it toggles collapse; adjust the class on the div containing HiChevronRight and
LuGripVertical (and any conditional classes referencing section.isCollapsed)
accordingly (or make it conditional: use cursor-pointer when the header handles
toggle, and cursor-grab only if you detect a pure drag-only mode).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e79af165-7717-4bd3-a8bd-381b68f1d0de
📒 Files selected for processing (21)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/DashboardSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectContextMenu/DashboardSidebarProjectContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/hooks/useDashboardSidebarProjectSectionActions/useDashboardSidebarProjectSectionActions.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/DashboardSidebarSection.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContent/DashboardSidebarSectionContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContent/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/DashboardSidebarSectionContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/DashboardSidebarSectionActionsDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/types.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionHeader/DashboardSidebarSectionHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionRenameContext/DashboardSidebarSectionRenameContext.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionRenameContext/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceContextMenu/DashboardSidebarWorkspaceContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/SortableSectionHeader/SortableSectionHeader.tsxapps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts
💤 Files with no reviewable changes (4)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContent/DashboardSidebarSectionContent.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContent/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/DashboardSidebarSection.tsx
✅ Files skipped from review due to trivial changes (7)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/components/DashboardSidebarProjectContextMenu/DashboardSidebarProjectContextMenu.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/types.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSectionRenameContext/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/DashboardSidebarSectionActionsDropdown/DashboardSidebarSectionActionsDropdown.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceContextMenu/DashboardSidebarWorkspaceContextMenu.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarSection/components/DashboardSidebarSectionContextMenu/components/SectionActionsMenuItems/SectionActionsMenuItems.tsx
- apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
7 tasks
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 26, 2026
3 PR (#442, #443, #444) で取り込み済みの 9 commits を git 履歴上もマージ済みに記録する。 内容差分は無し (merge -s ours)。 取り込み内容: - 6b96acd Improve sidebar group management UX (superset-sh#3745) - a4079e7 Update dashboard sidebar workspace icons (superset-sh#3755) - d3753d0 [codex] Use dynamic footer copyright years (superset-sh#3754) - ce606be Handle browser passthrough during v2 resize (superset-sh#3744) - b1e1eb7 Refactor v2 workspace page (superset-sh#3747) - 8693869 [codex] move v2 toggle to experimental settings (superset-sh#3748) - ef3f381 Revert "fix(desktop): refit v2 terminal after font settle (superset-sh#3742)" (superset-sh#3750) - 手動移植 (vibrancy patch 維持) - 25b2d52 Show terminal sessions from all workspaces in dropdown (superset-sh#3751) - 62737db fix v1 terminal resize repaint (superset-sh#3756)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root Cause
Sections visually own the following workspace rows. Some actions were assigning workspace or section
tabOrderusing only one side of that top-level lane, which could place ungrouped workspaces after a section header and make them appear grouped again.Validation
bunx @biomejs/biome@2.4.2 check --write apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebarbun run --cwd apps/desktop typecheckgit diff --cached --checkSummary by cubic
Improves the v2 dashboard sidebar group management with a unified actions menu, an auto-rename flow, and safer ordering. New groups auto-start rename (including from a workspace) and expand the project if collapsed; ungroup/delete now place workspaces correctly.
New Features
DashboardSidebarSectionRenameProvider; creating a group from a workspace moves that workspace into it and opens the project if collapsed.Bug Fixes
Written for commit a3c7651. Summary will update on new commits.
Summary by CodeRabbit
New Features
UI/UX Improvements
Bug Fixes