Skip to content

feat(WorktreeRow): add loading state and safety guards to worktree deletion - #128

Open
gedeagas wants to merge 1 commit into
mainfrom
workspace-improvement-delete
Open

feat(WorktreeRow): add loading state and safety guards to worktree deletion#128
gedeagas wants to merge 1 commit into
mainfrom
workspace-improvement-delete

Conversation

@gedeagas

Copy link
Copy Markdown
Owner

Summary

  • Add visual loading state (spinner + disabled interactions) while a worktree deletion is in progress
  • Prevent double-delete via deleteInFlightRef guard and disabled UI controls
  • Add comprehensive test suite for deletion flow (loading state, double-click prevention, error handling)

Layers touched

  • Renderer (src/renderer/) — components, stores, lib

Changes

Sidebar / Center / Right panel:

  • WorktreeRow.tsx: Added isDeleting state to reducer, deleteInFlightRef to prevent concurrent deletes, aria-busy attribute during deletion, disabled click/drag/context-menu interactions while deleting, replaced raw <button> with <Button> component in the confirmation dialog, show <Spinner> in place of PR icon and pin button while deleting
  • __tests__/WorktreeRow.test.tsx: New test suite covering delete-confirmation dialog flow, spinner/aria-busy presence during async deletion, double-click guard, error flash on failure, and "don't ask again" persistence

How to test

  1. yarn dev
  2. Right-click a non-main worktree and select Delete
  3. Confirm deletion - verify the spinner appears, the row is not clickable/draggable, and the dialog buttons are disabled during the operation
  4. Trigger a delete error (e.g. delete a worktree with active processes) - verify the error toast appears and the row returns to normal state
  5. yarn test WorktreeRow - all new tests pass

Checklist

  • Self-reviewed the diff
  • Tested locally with yarn dev
  • Types pass — yarn typecheck
  • No console errors or warnings in DevTools
  • New state is added to the correct Zustand store

…letion

- Prevent duplicate deletion requests via in-flight ref and early returns
- Show spinner during deletion; disable interactions (drag, click, context menu)
- Persist "don't ask again" only on successful deletion, not on failure
- Replace raw buttons with Button component for consistent loading indicator
Copilot AI review requested due to automatic review settings July 28, 2026 15:01
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a safer UX for worktree deletion in the sidebar by introducing an in-progress (“deleting”) state that disables interactions and shows a spinner, plus tests for the delete flow.

Changes:

  • Add isDeleting UI state + deleteInFlightRef guard to prevent concurrent deletes and disable row interactions during deletion.
  • Update delete confirmation dialog to use the shared <Button> component and disable controls while deleting.
  • Add a new Vitest test suite for the deletion flow (confirmation dialog behavior, loading state, and failure handling).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/renderer/components/Sidebar/WorktreeRow.tsx Adds deletion in-flight UI state/guards, disables interactions, and updates confirmation dialog controls.
src/renderer/components/Sidebar/tests/WorktreeRow.test.tsx Adds new tests for delete confirmation + loading state + error behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +136 to 140
await removeWorktree(worktree.projectId, worktree.id)
if (skipFutureConfirmations) setSkipDeleteConfirm(true)
rowDispatch({ type: 'HIDE_DELETE_CONFIRM' })
} catch (err) {
console.error('[WorktreeRow] removeWorktree failed:', err)
Comment on lines +157 to +173
it('shows a row-level spinner when confirmation is skipped', async () => {
const removal = deferred<void>()
mocks.removeWorktree.mockReturnValue(removal.promise)
mocks.skipDeleteWorktreeConfirm = true
renderRow()

const row = screen.getByRole('option')
fireEvent.keyDown(row, { key: 'Delete' })

await waitFor(() => expect(row.getAttribute('aria-busy')).toBe('true'))
expect(within(row).getByRole('status')).toBeTruthy()
expect(row.getAttribute('draggable')).toBe('false')

await act(async () => removal.resolve())

await waitFor(() => expect(row.getAttribute('aria-busy')).toBe('false'))
})
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