-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add drag-and-drop project reordering to the sidebar #185
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
b727592
c937f6b
f32db73
3ba50c0
5b0f6c0
de1a251
2c6bbc3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,7 @@ const initialState: AppState = { | |
| threadsHydrated: false, | ||
| }; | ||
| const persistedExpandedProjectCwds = new Set<string>(); | ||
| const persistedProjectOrderCwds: string[] = []; | ||
|
|
||
| // ── Persist helpers ────────────────────────────────────────────────── | ||
|
|
||
|
|
@@ -51,13 +52,22 @@ function readPersistedState(): AppState { | |
| try { | ||
| const raw = window.localStorage.getItem(PERSISTED_STATE_KEY); | ||
| if (!raw) return initialState; | ||
| const parsed = JSON.parse(raw) as { expandedProjectCwds?: string[] }; | ||
| const parsed = JSON.parse(raw) as { | ||
| expandedProjectCwds?: string[]; | ||
| projectOrderCwds?: string[]; | ||
| }; | ||
| persistedExpandedProjectCwds.clear(); | ||
| persistedProjectOrderCwds.length = 0; | ||
| for (const cwd of parsed.expandedProjectCwds ?? []) { | ||
| if (typeof cwd === "string" && cwd.length > 0) { | ||
| persistedExpandedProjectCwds.add(cwd); | ||
| } | ||
| } | ||
| for (const cwd of parsed.projectOrderCwds ?? []) { | ||
| if (typeof cwd === "string" && cwd.length > 0 && !persistedProjectOrderCwds.includes(cwd)) { | ||
| persistedProjectOrderCwds.push(cwd); | ||
| } | ||
| } | ||
| return { ...initialState }; | ||
| } catch { | ||
| return initialState; | ||
|
|
@@ -75,6 +85,7 @@ function persistState(state: AppState): void { | |
| expandedProjectCwds: state.projects | ||
| .filter((project) => project.expanded) | ||
| .map((project) => project.cwd), | ||
| projectOrderCwds: state.projects.map((project) => project.cwd), | ||
| }), | ||
| ); | ||
| if (!legacyKeysCleanedUp) { | ||
|
|
@@ -110,10 +121,17 @@ function mapProjectsFromReadModel( | |
| incoming: OrchestrationReadModel["projects"], | ||
| previous: Project[], | ||
| ): Project[] { | ||
| return incoming.map((project) => { | ||
| const existing = | ||
| previous.find((entry) => entry.id === project.id) ?? | ||
| previous.find((entry) => entry.cwd === project.workspaceRoot); | ||
| const previousById = new Map(previous.map((project) => [project.id, project] as const)); | ||
| const previousByCwd = new Map(previous.map((project) => [project.cwd, project] as const)); | ||
| const previousOrderById = new Map(previous.map((project, index) => [project.id, index] as const)); | ||
| const previousOrderByCwd = new Map(previous.map((project, index) => [project.cwd, index] as const)); | ||
| const persistedOrderByCwd = new Map( | ||
| persistedProjectOrderCwds.map((cwd, index) => [cwd, index] as const), | ||
| ); | ||
| const usePersistedOrder = previous.length === 0; | ||
|
|
||
| const mappedProjects = incoming.map((project) => { | ||
| const existing = previousById.get(project.id) ?? previousByCwd.get(project.workspaceRoot); | ||
| return { | ||
| id: project.id, | ||
| name: project.title, | ||
|
|
@@ -127,8 +145,25 @@ function mapProjectsFromReadModel( | |
| ? persistedExpandedProjectCwds.has(project.workspaceRoot) | ||
| : true), | ||
| scripts: project.scripts.map((script) => ({ ...script })), | ||
| }; | ||
| } satisfies Project; | ||
| }); | ||
|
|
||
| return mappedProjects | ||
| .map((project, incomingIndex) => { | ||
| const previousIndex = previousOrderById.get(project.id) ?? previousOrderByCwd.get(project.cwd); | ||
| const persistedIndex = usePersistedOrder ? persistedOrderByCwd.get(project.cwd) : undefined; | ||
| const orderIndex = | ||
| previousIndex ?? | ||
| persistedIndex ?? | ||
| (usePersistedOrder ? persistedProjectOrderCwds.length : previous.length) + incomingIndex; | ||
|
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. Operator precedence bug in order index fallback calculationLow Severity The |
||
| return { project, incomingIndex, orderIndex }; | ||
| }) | ||
| .toSorted((a, b) => { | ||
| const byOrder = a.orderIndex - b.orderIndex; | ||
| if (byOrder !== 0) return byOrder; | ||
| return a.incomingIndex - b.incomingIndex; | ||
| }) | ||
| .map((entry) => entry.project); | ||
| } | ||
|
|
||
| function toLegacySessionStatus( | ||
|
|
@@ -349,6 +384,22 @@ export function setProjectExpanded( | |
| return changed ? { ...state, projects } : state; | ||
| } | ||
|
|
||
| export function reorderProjects( | ||
| state: AppState, | ||
| draggedProjectId: Project["id"], | ||
| targetProjectId: Project["id"], | ||
| ): AppState { | ||
| if (draggedProjectId === targetProjectId) return state; | ||
| const draggedIndex = state.projects.findIndex((project) => project.id === draggedProjectId); | ||
| const targetIndex = state.projects.findIndex((project) => project.id === targetProjectId); | ||
| if (draggedIndex < 0 || targetIndex < 0) return state; | ||
| const projects = [...state.projects]; | ||
| const [draggedProject] = projects.splice(draggedIndex, 1); | ||
| if (!draggedProject) return state; | ||
| projects.splice(targetIndex, 0, draggedProject); | ||
| return { ...state, projects }; | ||
| } | ||
|
|
||
| export function setError(state: AppState, threadId: ThreadId, error: string | null): AppState { | ||
| const threads = updateThread(state.threads, threadId, (t) => { | ||
| if (t.error === error) return t; | ||
|
|
@@ -384,6 +435,7 @@ interface AppStore extends AppState { | |
| markThreadUnread: (threadId: ThreadId) => void; | ||
| toggleProject: (projectId: Project["id"]) => void; | ||
| setProjectExpanded: (projectId: Project["id"], expanded: boolean) => void; | ||
| reorderProjects: (draggedProjectId: Project["id"], targetProjectId: Project["id"]) => void; | ||
| setError: (threadId: ThreadId, error: string | null) => void; | ||
| setThreadBranch: (threadId: ThreadId, branch: string | null, worktreePath: string | null) => void; | ||
| } | ||
|
|
@@ -397,6 +449,8 @@ export const useStore = create<AppStore>((set) => ({ | |
| toggleProject: (projectId) => set((state) => toggleProject(state, projectId)), | ||
| setProjectExpanded: (projectId, expanded) => | ||
| set((state) => setProjectExpanded(state, projectId, expanded)), | ||
| reorderProjects: (draggedProjectId, targetProjectId) => | ||
| set((state) => reorderProjects(state, draggedProjectId, targetProjectId)), | ||
| setError: (threadId, error) => set((state) => setError(state, threadId, error)), | ||
| setThreadBranch: (threadId, branch, worktreePath) => | ||
| set((state) => setThreadBranch(state, threadId, branch, worktreePath)), | ||
|
|
||


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.
Ambiguous operator precedence in fallback order calculation
Low Severity
The
orderIndexfallback expression relies on+having higher precedence than??without explicit parentheses. While the current behavior is correct (the addition only applies when bothpreviousIndexandpersistedIndexare nullish), a reader could easily misinterpret this as addingincomingIndexto the result of the entire??chain. Wrapping the fallback arithmetic in parentheses would make the intent unambiguous and prevent accidental breakage if the expression is modified later.