Skip to content

Fix workspace delete confirmation focus - #22

Merged
trydis merged 2 commits into
mainfrom
delete-workspace-prompt-focus
Mar 14, 2026
Merged

Fix workspace delete confirmation focus#22
trydis merged 2 commits into
mainfrom
delete-workspace-prompt-focus

Conversation

@trydis

@trydis trydis commented Mar 14, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Refactor

    • Streamlined workspace deletion: removed the in-app destructive delete alert and replaced the multi-step pending-delete flow with an immediate confirmation route (now presented via the system confirmation dialog).
  • Tests

    • Updated tests to exercise the new immediate confirmation flow and verify workspace deletion, selection repair, and related behaviors.

@coderabbitai

coderabbitai Bot commented Mar 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7ef2c54f-3c8f-4ffd-bbd4-3246a4cdb1d5

📥 Commits

Reviewing files that changed from the base of the PR and between ad4cc63 and 37953d1.

📒 Files selected for processing (2)
  • Sources/Shellraiser/Services/Workspaces/WorkspaceManager+WorkspaceLifecycle.swift
  • Sources/Shellraiser/Services/Workspaces/WorkspaceManager.swift

📝 Walkthrough

Walkthrough

Refactors workspace deletion from a pending-state flow to an immediate confirmation callback. Removes the UI alert binding in ContentView and adds a WorkspaceDeletionConfirmer dependency on WorkspaceManager; tests updated to use the confirmation closure.

Changes

Cohort / File(s) Summary
Workspace Manager Core
Sources/Shellraiser/Services/Workspaces/WorkspaceManager.swift, Sources/Shellraiser/Services/Workspaces/WorkspaceManager+WorkspaceLifecycle.swift
Replaces published pendingWorkspaceDeletion and confirm/cancel methods with a WorkspaceDeletionConfirmer callback property and direct-confirm delete path. Adds presentWorkspaceDeletionConfirmation(_:) static helper and updates initializer to accept the confirmer. Removes public confirm/cancel pending-deletion APIs.
UI Layer
Sources/Shellraiser/App/ContentView.swift
Removes the destructive delete workspace alert presentation and the pendingWorkspaceDeletionIsPresented binding and related alert actions/messages.
Tests & Test Helpers
Tests/ShellraiserTests/WorkspaceManagerLifecycleTests.swift, Tests/ShellraiserTests/WorkspaceManagerShortcutTests.swift, Tests/ShellraiserTests/WorkspaceTestSupport.swift
Updates test helper makeWorkspaceManager(...) to accept confirmWorkspaceDeletion closure; refactors tests to capture and assert on the deletion request via the confirmer closure rather than inspecting pending state. Adds/renames tests to match the new confirmation flow.

Sequence Diagram(s)

sequenceDiagram
    participant UI as "ContentView (UI)"
    participant WM as "WorkspaceManager"
    participant Confirmer as "confirmWorkspaceDeletion\n(callback)"
    participant AppKit as "AppKit Dialog"
    participant Persist as "WorkspacePersisting"

    UI->>WM: requestDeleteWorkspace(workspaceId)
    WM->>Confirmer: invoke(request)
    Confirmer->>AppKit: present dialog (optional)
    AppKit-->>Confirmer: Bool (confirmed?)
    Confirmer-->>WM: Bool
    alt confirmed
        WM->>Persist: deleteWorkspace(workspaceId)
        Persist-->>WM: deletion result
        WM->>UI: update selection / notify
    else cancelled
        WM-->>UI: no-op / cancellation acknowledged
    end
Loading

Possibly related PRs

  • Tests #4: Modifies workspace deletion/confirmation flow and tests; strong overlap with the removal of pending-deletion state and introduction of a confirmation callback.
  • Add focused Git metadata to workspace sidebar #12: Changes WorkspaceManager initializer and lifecycle related to deletion; directly related to the API/signature updates here.
  • Codex started heuristics #21: Alters workspace deletion lifecycle and confirmation handling; relevant for code-level similarities.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fix workspace delete confirmation focus' is vague and does not clearly convey the actual changes made, which involve refactoring the deletion confirmation mechanism from a pending-state pattern to a callback-based approach. Consider a more descriptive title that captures the main architectural change, such as 'Refactor workspace deletion confirmation to callback-based pattern' or 'Replace pending deletion state with confirmation callback'
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch delete-workspace-prompt-focus
📝 Coding Plan
  • Generate coding plan for human review comments

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

@trydis
trydis merged commit 1b7e332 into main Mar 14, 2026
2 checks passed
@trydis
trydis deleted the delete-workspace-prompt-focus branch March 14, 2026 14:32
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.

1 participant