Repository navigation
feat(desktop): place workspaces moved into a group at the top - #6137
prayagtushar wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughWorkspace moves now place items at the top of the target section. Bulk moves reverse selected workspace IDs before insertion to preserve selection order. ChangesSidebar workspace ordering
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR changes workspace moves into sidebar groups to prepend workspaces rather than append them, and reverses bulk iteration to retain visual order.
Confidence Score: 4/5The mixed-selection ordering defect should be fixed before merging because moving a selection into a group can visibly reverse part of that selection. Filtering out selected workspaces already in the target section before reversing and prepending the remainder cannot preserve relative visual order across the complete selection. Files Needing Attention: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useBulkWorkspaceMoveActions/useBulkWorkspaceMoveActions.ts
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useBulkWorkspaceMoveActions/useBulkWorkspaceMoveActions.ts | Reverses bulk move iteration correctly for wholly incoming selections, but mixed selections with existing target-section members do not retain their full visual order. |
| apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts | Changes section moves from append to prepend using the existing tab-order helper; the assignment itself is consistent with the requested behavior. |
Sequence Diagram
sequenceDiagram
participant User
participant Bulk as Bulk move action
participant Filter as workspaceIdsForSectionMove
participant State as Sidebar state
User->>Bulk: Move visually ordered selection to group
Bulk->>Filter: Filter selection for target group
Filter-->>Bulk: Exclude members already in target
Bulk->>Bulk: Reverse remaining IDs
loop Each remaining workspace
Bulk->>State: Move and prepend
end
Note over State: Incoming items precede existing selected target members
Prompt To Fix All With AI
### Issue 1
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useBulkWorkspaceMoveActions/useBulkWorkspaceMoveActions.ts:63-67
**Mixed selection order breaks**
When a selection includes workspaces already in the target section, `workspaceIdsForSectionMove` removes those IDs before this loop reverses and prepends the remainder, causing incoming workspaces to appear ahead of selected target-section workspaces that originally preceded them.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(desktop): place workspaces moved in..." | Re-trigger Greptile
There was a problem hiding this comment.
No issues found across 2 files
Architecture diagram
sequenceDiagram
participant UI as Desktop UI
participant Hook as useBulkWorkspaceMoveActions
participant SectionHook as useDashboardSidebarState
participant LocalState as v2WorkspaceLocalState (collections)
Note over UI,LocalState: Workspace movement into a group
UI->>Hook: Trigger move (single or bulk)
alt Single workspace move
UI->>SectionHook: moveWorkspaceToSection(workspaceId, projectId, sectionId)
SectionHook->>SectionHook: CHANGED: getPrependTabOrder(siblingRows) instead of getNextTabOrder
SectionHook->>LocalState: update(workspaceId, { tabOrder: prependedOrder, ... })
LocalState-->>SectionHook: Updated
SectionHook-->>UI: Workspace lands at top of group
else Bulk workspace move (moveSelectionToSection)
UI->>Hook: moveSelectionToSection(sectionId)
Hook->>Hook: CHANGED: Reverse selection order (bottom-to-top iteration)
loop For each workspaceId in reversed selection
Hook->>SectionHook: moveWorkspaceToSection(workspaceId, projectId, sectionId)
SectionHook->>SectionHook: getPrependTabOrder(siblingRows)
SectionHook->>LocalState: update(workspaceId, { tabOrder, ... })
LocalState-->>SectionHook: Updated
end
Hook->>Hook: clearSelection()
Note over UI,LocalState: Result: original order preserved, items at top of group
else Bulk create group (createGroupFromSelection)
UI->>Hook: createGroupFromSelection()
Hook->>SectionHook: createSection(projectId)
SectionHook-->>Hook: sectionId
Hook->>Hook: CHANGED: Reverse selectedIds iteration (bottom-to-top)
loop For each workspaceId in reversed selectedIds
Hook->>SectionHook: moveWorkspaceToSection(workspaceId, projectId, sectionId)
SectionHook->>SectionHook: getPrependTabOrder(siblingRows)
SectionHook->>LocalState: update(workspaceId, { tabOrder, ... })
LocalState-->>SectionHook: Updated
end
Hook->>Hook: clearSelection()
end
53a15ad to
6210bb0
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
This PR has had no activity for 53 days and will close in 7 days unless there is new activity. If this contribution is still important or relevant, please leave a comment to keep it open. If it closes, you are welcome to reopen it or ask a maintainer to reopen it. Thank you for contributing! |
What & why
Moving a workspace into a group put it at the bottom, so in a large group you
have to scroll to find what you just moved. It now goes to the top, using the
getPrependTabOrder()helper already sitting next togetNextTabOrder(). Thismatches how new workspaces and projects already enter the sidebar.
Bulk moves needed the loops reversed. Prepend is relative to the group's current
lowest tab order, so each item lands above the previous one and a multi-select
walked top-to-bottom would land backwards.
The optional top/bottom preference from the issue is not included.
Fixes #5342
How I tested it
bun run lintclean;bun teston the sidebar dirs, 88 passbun run typecheckfails in@superset/uimulti-select keeps its order at the top.
Checklist
type(scope): subject)bun run lintandbun run typecheckpass (CI fails on lint warnings too)Screen recording:
Before fix:
Screen.Recording.2026-08-03.at.7.01.23.PM.mov
After fix:
Screen.Recording.2026-08-03.at.6.59.30.PM.mov
Summary by cubic
Moving a workspace into a group now puts it at the top; multi-select moves keep original order, including mixed selections and when creating a new group. Fixes #5342; no top/bottom preference.
Written for commit 6210bb0. Summary will update on new commits.
Summary by CodeRabbit