Skip to content

Fix: persist sidebar reorder on drag end#1050

Merged
Kitenite merged 3 commits into
superset-sh:mainfrom
onevcat:fix/persist-sidebar-reorder
Feb 2, 2026
Merged

Fix: persist sidebar reorder on drag end#1050
Kitenite merged 3 commits into
superset-sh:mainfrom
onevcat:fix/persist-sidebar-reorder

Conversation

@onevcat
Copy link
Copy Markdown
Contributor

@onevcat onevcat commented Jan 29, 2026

Hi maintainers,

Summary

  • Sidebar reorder for project groups and workspace items could appear to work but reset after restart.
  • Root cause: persistence only happened on React-DnD drop. When the mouse was released outside a registered drop target, didDrop was false, so the reorder never hit the database.
  • Fix: persist reorders in the drag end handler when indices actually changed, while keeping the existing drop path intact.

Reproduction steps

  1. Open the desktop app with multiple projects/workspaces.
  2. Drag a project group (or a workspace item) to a new position.
  3. Release the mouse in the list area (not necessarily on another item).
  4. Restart the app.

Actual result

Order changes only in UI and resets after restart.

Expected result

Order should be persisted and remain after restart.

Testing

Not run (manual drag/drop verified locally).

Thank you for reviewing!

Summary by CodeRabbit

  • Improvements
    • Enhanced drag-and-drop reordering for projects and workspaces with more reliable completion handling.
    • Added optimistic in-memory updates so index changes appear instantly during drag.
    • Ensures cache/state is refreshed after reorder operations settle.
  • Bug Fixes
    • Guarded end-of-drag logic to avoid spurious reorders and prevent duplicate operations.
  • User-facing
    • Preserves existing component interfaces; no public API changes.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 29, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds originalIndex tracking and end handlers to project and workspace drag sources; performs optimistic in-memory reordering on hover and calls reorderProjects / reorderWorkspaces mutations on drag end when index changed, with error toasts and cache invalidation on settle.

Changes

Cohort / File(s) Summary
Project Reordering
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx
Adds drag end handler that early-returns if item missing or monitor.didDrop(); when index changed and not dropped on valid target, calls reorderProjects.mutate with onSettled invalidation and error toast. Adds reorderProjects to hook deps.
Workspace Reordering
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx
Includes originalIndex in drag payload; hover performs optimistic in-memory reorder when moving within same project and updates item.index; drop/end handler compares original vs current index and calls reorderWorkspaces.mutate only on change, with error toast and cache invalidation; updates drag deps to include reorderWorkspaces.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant UI as ProjectSection/<br/>WorkspaceListItem
    participant Drag as Drag Monitor
    participant Memory as In-Memory Data
    participant API as reorderProjects/<br/>reorderWorkspaces

    User->>Drag: Start drag (store originalIndex)
    Drag->>UI: Provide drag item & monitor
    UI->>Drag: Track hover position
    alt Index changed while hovering
        UI->>Memory: Optimistically reorder grouped data
        Memory-->>UI: Update visuals
    end
    User->>Drag: Release (drop)
    Drag->>UI: Trigger end handler
    alt monitor.didDrop() is false AND index changed
        UI->>API: Call reorder mutation (from originalIndex to current index)
        alt Success
            API-->>UI: Mutation success
            UI->>Memory: Ensure persisted order / invalidate cache
        else Failure
            UI-->>User: Show error toast
            UI->>Memory: Revert optimistic changes
        end
    else monitor.didDrop() true OR no index change
        UI->>Memory: Discard or keep optimistic state (no server call)
    end
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I tracked an OriginalIndex from the start,
Hopped and hovered, moved with heart,
Optimistic flips before the call,
If the server frowns, I roll it all,
A joyful reorder — hop, drop, restart! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description covers the problem, root cause, fix approach, and reproduction steps, but lacks structured sections matching the provided template (Type of Change, Testing, etc.). Reformat the description to follow the template structure with labeled sections (Description, Related Issues, Type of Change, Testing, etc.) for consistency and clarity.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix: persist sidebar reorder on drag end' accurately captures the main change: implementing persistence of sidebar reordering when the drag ends, fixing the issue where reorders were lost on app restart.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx`:
- Around line 66-79: The drop handler in the drag source is not returning a
value so monitor.didDrop() stays false and the end handler in ProjectSection
(which calls reorderProjects.mutate) fires a second time; update the drop
handler for the relevant drag source to return a non-undefined result object
(e.g., { fromIndex, toIndex, handled: true }) so monitor.didDrop() will be true
and the end handler in ProjectSection (the end: (item, monitor) => { ...
reorderProjects.mutate(...) }) will skip its mutate call; ensure the returned
shape is minimal and documented where reorderProjects.mutate is invoked.

…order

Return value from drop handlers so didDrop() returns true, preventing
both drop and end handlers from firing mutate(). Add onSettled callback
to invalidate query cache, ensuring UI syncs with server state on
mutation failure.
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for solving this @onevcat! Sorry it took me a while to review

@Kitenite Kitenite merged commit 3f4d8be into superset-sh:main Feb 2, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants