Repository navigation
fix(desktop): sidebar folder drags without layout shift, draggable Sessions tag folders #7112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4bac499
2d3b10d
d85b66f
74926c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { | |
| verticalListSortingStrategy, | ||
| } from "@dnd-kit/sortable"; | ||
| import { useLingui } from "@lingui/react/macro"; | ||
| import { useMemo } from "react"; | ||
| import { useSidebarSectionsCollapseStore } from "renderer/stores/sidebar-sections-collapse"; | ||
| import { | ||
| dropZoneId, | ||
|
|
@@ -42,6 +43,13 @@ export function DashboardSidebarPinnedSection({ | |
| const { pinnedItems, workspacesById, projectsById, activeType } = | ||
| useDashboardSidebarDnd(); | ||
| const isDraggingWorkspace = activeType === "workspace"; | ||
| // Captured once per drag, on the render where the drag flag flips — the | ||
| // only render where "empty" still means "empty at pickup". | ||
| // biome-ignore lint/correctness/useExhaustiveDependencies: pinnedItems is read at that moment on purpose | ||
| const emptyAtPickup = useMemo( | ||
| () => isDraggingWorkspace && pinnedItems.length === 0, | ||
| [isDraggingWorkspace], | ||
| ); | ||
| const isSectionCollapsed = useSidebarSectionsCollapseStore( | ||
| (s) => s.collapsed.pinned, | ||
| ); | ||
|
|
@@ -63,10 +71,35 @@ export function DashboardSidebarPinnedSection({ | |
| ); | ||
| } | ||
|
|
||
| // Keep the section (and its drop targets) mounted through the whole drag, | ||
| // even when the last pinned row is dragged out mid-drag. | ||
| if (pinnedItems.length === 0 && !isDraggingWorkspace) return null; | ||
|
|
||
| // With nothing pinned, the section must not appear in-flow at pickup: | ||
| // mounting a header and drop zone above the dragged row shifts it under | ||
| // the pointer, and when the sidebar has no scroll room for dnd-kit to | ||
| // compensate the ghost sits that far off the cursor for the whole drag. | ||
| // Float the target instead — zero height in the list, stuck to the top of | ||
| // the scroller so it stays reachable however far the list is scrolled. | ||
| // A drag that started with pins keeps the in-flow section even after its | ||
| // last row transfers out, so the layout under the pointer holds still. | ||
| if (pinnedItems.length === 0 && emptyAtPickup) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When a no-pin drag enters the floating pin target, Prompt for AI agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional. The float exists to keep the pickup itself shift-free; once the row transfers into Pinned, the in-flow section is what shows the row landing there, and the DragOverlay is unaffected by mid-drag layout. Keeping the float would leave the transferred row with nowhere to render. Measured A/B against main: the transfer path costs the same on both sides. |
||
| return ( | ||
| // -mb-3 cancels the `mt-3 first:mt-0` the next section gains once | ||
| // it is no longer the scroller's first child — otherwise the float | ||
| // still nudges the list by that margin. | ||
| <div className="-mb-3 sticky top-0 z-20 h-0"> | ||
| <div className="bg-sidebar/90 py-1 backdrop-blur-sm dark:bg-muted/90"> | ||
| <SidebarDropZone | ||
| dropZoneId={dropZoneId(PINNED_CONTAINER)} | ||
| label={t({ | ||
| id: "dashboard.sidebar.pinnedSection.dropToPin", | ||
| message: "Drop to pin", | ||
| })} | ||
| /> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <div className="mt-3 pb-1 first:mt-0"> | ||
| <DashboardSidebarSectionHeader | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,34 +1,54 @@ | ||
| import { | ||
| SortableContext, | ||
| verticalListSortingStrategy, | ||
| } from "@dnd-kit/sortable"; | ||
| import { SortableContext } from "@dnd-kit/sortable"; | ||
| import { useLingui } from "@lingui/react/macro"; | ||
| import { AnimatePresence, motion } from "framer-motion"; | ||
| import { useMemo } from "react"; | ||
| import { | ||
| dropZoneId, | ||
| parseId, | ||
| SESSIONS_CONTAINER, | ||
| useDashboardSidebarDnd, | ||
| } from "../../../../hooks/useSidebarDnd"; | ||
| import { useDashboardSidebarSelection } from "../../../../providers/DashboardSidebarSelectionProvider"; | ||
| import type { | ||
| DashboardSidebarWorkspace, | ||
| DashboardSidebarWorkspaceIndentation, | ||
| } from "../../../../types"; | ||
| import { WorkspaceBulkMenuScope } from "../../../DashboardSidebarWorkspaceItem/components/WorkspaceBulkMenuScope"; | ||
| import { SidebarDropZone } from "../../../SidebarDropZone"; | ||
| import { SortableSectionHeader } from "../../../SortableSectionHeader"; | ||
| import { SortableWorkspaceItem } from "../../../SortableWorkspaceItem"; | ||
|
|
||
| /** Main rows are never bulk-selected; sessions and worktrees are. */ | ||
| const isBulkSelectable = (workspace: DashboardSidebarWorkspace) => | ||
| workspace.type !== "main" && workspace.pendingTransaction?.type !== "insert"; | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
|
|
||
| interface DashboardSidebarExpandedProjectContentProps { | ||
| projectId: string; | ||
| /** DnD container: a project id, or SESSIONS_CONTAINER for the Sessions lane. */ | ||
| containerId: string; | ||
| /** Lane the rows are filed under: the project id, or null for sessions. */ | ||
| projectId: string | null; | ||
| isCollapsed: boolean; | ||
| /** Row indentation overrides; the Sessions lane sits flush with its header. */ | ||
| topLevelIndentation?: DashboardSidebarWorkspaceIndentation; | ||
| groupedIndentation?: DashboardSidebarWorkspaceIndentation; | ||
| workspaceShortcutLabels: Map<string, string>; | ||
| onWorkspaceHover: (workspaceId: string) => void | Promise<void>; | ||
| onDeleteSection: (sectionId: string) => void; | ||
| onRenameSection: (sectionId: string, name: string) => void; | ||
| onToggleSectionCollapse: (sectionId: string) => void; | ||
| } | ||
|
|
||
| /** | ||
| * The sortable row list of one DnD container — every project, and the | ||
| * Sessions lane. Ungrouped rows and folder headers are flat siblings so a | ||
| * folder drag can move whole groups as units (see useSidebarDnd). | ||
| */ | ||
| export function DashboardSidebarExpandedProjectContent({ | ||
| containerId, | ||
| projectId, | ||
| isCollapsed, | ||
| topLevelIndentation, | ||
| groupedIndentation, | ||
| workspaceShortcutLabels, | ||
| onWorkspaceHover, | ||
| onDeleteSection, | ||
|
|
@@ -38,41 +58,40 @@ export function DashboardSidebarExpandedProjectContent({ | |
| const { t } = useLingui(); | ||
| const { | ||
| projectItems, | ||
| getProjectSortableItems, | ||
| activeType, | ||
| activeContainer, | ||
| sessionItems, | ||
| getContainerSortingStrategy, | ||
| activeSectionId, | ||
| activeWorkspaceHome, | ||
| groupInfo, | ||
| collapsedSectionIds, | ||
| workspacesById, | ||
| sectionsById, | ||
| } = useDashboardSidebarDnd(); | ||
| const flatItems = useMemo( | ||
| () => projectItems[projectId] ?? [], | ||
| [projectItems, projectId], | ||
| () => | ||
| containerId === SESSIONS_CONTAINER | ||
| ? sessionItems | ||
| : (projectItems[containerId] ?? []), | ||
| [projectItems, sessionItems, containerId], | ||
| ); | ||
| const sortableItems = getProjectSortableItems(projectId); | ||
| const sortingStrategy = getContainerSortingStrategy(containerId); | ||
| const { isWorkspaceSelected, selectWorkspaceFromEvent } = | ||
| useDashboardSidebarSelection(); | ||
|
|
||
| // A pinned workspace can only return to its home project; when every row | ||
| // of that project is pinned, the empty list needs an explicit drop target. | ||
| // A pinned workspace can only return to its home container; when every row | ||
| // of that container is pinned, the empty list needs an explicit drop target. | ||
| const dropZoneEligible = | ||
| !isCollapsed && flatItems.length === 0 && activeWorkspaceHome === projectId; | ||
| !isCollapsed && | ||
| flatItems.length === 0 && | ||
| activeWorkspaceHome === containerId; | ||
|
|
||
| const selectableWorkspaceIds = useMemo( | ||
| () => | ||
| flatItems.flatMap((id) => { | ||
| const parsed = parseId(id); | ||
| if (!parsed || parsed.type !== "workspace") return []; | ||
| const workspace = workspacesById.get(parsed.realId); | ||
| if ( | ||
| !workspace || | ||
| workspace.type !== "worktree" || | ||
| workspace.pendingTransaction?.type === "insert" | ||
| ) { | ||
| return []; | ||
| } | ||
| if (!workspace || !isBulkSelectable(workspace)) return []; | ||
| const group = groupInfo.get(parsed.realId); | ||
| if (group && collapsedSectionIds.has(group.sectionId)) return []; | ||
| return [parsed.realId]; | ||
|
|
@@ -96,10 +115,7 @@ export function DashboardSidebarExpandedProjectContent({ | |
| workspacesById={workspacesById} | ||
| groupInfo={groupInfo} | ||
| > | ||
| <SortableContext | ||
| items={sortableItems} | ||
| strategy={verticalListSortingStrategy} | ||
| > | ||
| <SortableContext items={flatItems} strategy={sortingStrategy}> | ||
| {flatItems.map((id) => { | ||
| const parsed = parseId(id); | ||
| if (!parsed) return null; | ||
|
|
@@ -125,25 +141,25 @@ export function DashboardSidebarExpandedProjectContent({ | |
| const isInSection = !!group; | ||
| const isInCollapsedSection = | ||
| isInSection && collapsedSectionIds.has(group.sectionId); | ||
| const sectionDragActive = | ||
| activeType === "section" && activeContainer === projectId; | ||
| const hidden = | ||
| isInCollapsedSection || (sectionDragActive && isInSection); | ||
| const canBulkSelect = | ||
| workspace.type === "worktree" && | ||
| workspace.pendingTransaction?.type !== "insert"; | ||
| const inDraggedSection = | ||
| isInSection && group.sectionId === activeSectionId; | ||
| const canBulkSelect = isBulkSelectable(workspace); | ||
|
|
||
| // The zero-height collapse lives inside the sortable wrapper | ||
| // (see SortableWorkspaceItem) so the clip box moves with the | ||
| // dnd translate — wrapping here would clip displaced rows | ||
| // out of view mid-drag. | ||
| // out of view mid-drag. Rows stay mounted and full-height | ||
| // during a section drag: the group travels as a block. | ||
| return ( | ||
| <SortableWorkspaceItem | ||
| key={String(id)} | ||
| sortableId={String(id)} | ||
| workspace={workspace} | ||
| accentColor={group?.color} | ||
| isInSection={isInSection} | ||
| indentation={ | ||
| isInSection ? groupedIndentation : topLevelIndentation | ||
| } | ||
| onHoverCardOpen={onWorkspaceHover} | ||
| shortcutLabel={workspaceShortcutLabels.get(parsed.realId)} | ||
| isSelected={ | ||
|
|
@@ -154,15 +170,15 @@ export function DashboardSidebarExpandedProjectContent({ | |
| ? (event) => | ||
| selectWorkspaceFromEvent(event, { | ||
| workspaceId: parsed.realId, | ||
| projectId, | ||
| projectId: containerId, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When a user Ctrl/Shift-clicks a session row, the selection stores Prompt for AI agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pre-existing: the Sessions section passed |
||
| orderedWorkspaceIds: selectableWorkspaceIds, | ||
| }) | ||
| : undefined | ||
| } | ||
| collapsed={hidden} | ||
| collapseInstantly={sectionDragActive} | ||
| collapsed={isInCollapsedSection} | ||
| isDragPlaceholder={inDraggedSection} | ||
| disabled={ | ||
| hidden || | ||
| isInCollapsedSection || | ||
| (workspace.type === "main" && | ||
| workspace.hostType === "local-device") | ||
| } | ||
|
|
@@ -172,7 +188,7 @@ export function DashboardSidebarExpandedProjectContent({ | |
| </SortableContext> | ||
| {dropZoneEligible && ( | ||
| <SidebarDropZone | ||
| dropZoneId={dropZoneId(projectId)} | ||
| dropZoneId={dropZoneId(containerId)} | ||
| label={t({ | ||
| id: "dashboard.sidebar.projectContent.dropToUnpin", | ||
| message: "Drop to unpin", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: When a session is bulk-selected, any subsequent project list or order update clears it because
selectableWorkspaceIdsstill excludes all session rows. Add the session workspaces (including grouped rows) to the available-ID set now that Sessions uses the shared bulk-selectable lane.Prompt for AI agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-existing: main's
selectableWorkspaceIdsalso excluded session rows while the Sessions section already offered bulk selection withprojectId: "sessions". Unchanged by this PR; noted as a follow-up.