Skip to content

fix(macos): replace setup terminal modal with terminal drawer - #16

Merged
roodboi merged 1 commit into
mainfrom
fix/issue-15-macos-close-button
Feb 26, 2026
Merged

fix(macos): replace setup terminal modal with terminal drawer#16
roodboi merged 1 commit into
mainfrom
fix/issue-15-macos-close-button

Conversation

@roodboi

@roodboi roodboi commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove Setup Assistant's embedded terminal modal state/sheet flow
  • route "Run here" actions to the built-in terminal drawer via global-shell terminal notifications
  • delete unused ShellView now that no modal terminal surface remains

Testing

  • bun run macos:test
  • bun x ultracite check

Closes #15

Summary by CodeRabbit

  • Bug Fixes

    • Setup assistant terminal execution now uses the global terminal panel instead of an embedded sheet for consistency with standard terminal behavior.
  • Refactor

    • Removed the embedded terminal component from the setup process. Commands now route through the global terminal panel for a unified user experience.

@changeset-bot

changeset-bot Bot commented Feb 26, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 0d5bfba

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Feb 26, 2026

Copy link
Copy Markdown

Walkthrough

Removed the embedded terminal sheet flow from SetupAssistantView and deleted the ShellView component entirely. Command execution now routes through a global terminal panel invocation instead of displaying an internal sheet-based terminal view.

Changes

Cohort / File(s) Summary
Embedded Terminal Flow Removal
apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/SetupAssistantView.swift
Removed showEmbeddedTerminal and embeddedCommand state properties. Modified runInEmbeddedTerminalIfAvailable to validate input and invoke openGlobalCommandInTerminalPanel directly instead of setting state for a sheet presentation.
ShellView Component Deletion
apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ShellView.swift
Deleted entire ShellView.swift file, removing the ShellView SwiftUI component that previously hosted embedded terminal rendering with GhosttyTerminalSession, lifecycle management, and internal sheet UI.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A bunny hops through code so clean,
Removing sheets that blocked the scene,
No more trapped in modal's hold,
Global panels brave and bold!
Freedom flows through every line,
SwiftUI paths now shine! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing an embedded terminal modal/sheet with a terminal drawer, matching the core refactoring in SetupAssistantView and removal of ShellView.
Linked Issues check ✅ Passed The PR eliminates the blocking modal terminal surface by removing the embeddedTerminalSheet and ShellView, routing commands to a global terminal panel instead, directly addressing issue #15's requirement to prevent users from being trapped in a non-dismissible modal.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the reported issue: removing the embedded terminal modal flow from SetupAssistantView and deleting the ShellView component that comprised the problematic modal surface.

✏️ 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 fix/issue-15-macos-close-button

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.

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (1)
apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/SetupAssistantView.swift (1)

217-217: Consider renaming function to reflect its new behavior.

The function name runInEmbeddedTerminalIfAvailable suggests it runs commands in an embedded terminal, but it now opens a global terminal panel instead. Consider renaming to something like runInGlobalTerminalPanel or openCommandInTerminalDrawer for clarity.

♻️ Proposed rename
-  private func runInEmbeddedTerminalIfAvailable(_ command: String) {
+  private func runInGlobalTerminalPanel(_ command: String) {
     let normalizedCommand = command.trimmingCharacters(in: .whitespacesAndNewlines)
     guard !normalizedCommand.isEmpty else { return }
     openGlobalCommandInTerminalPanel(command: normalizedCommand, title: "Setup")

Also update all call sites (lines 82, 94, 108, 125, 139, 151):

-      onRunInEmbeddedTerminal: supportsEmbeddedTerminal ? runInEmbeddedTerminalIfAvailable : nil,
+      onRunInEmbeddedTerminal: supportsEmbeddedTerminal ? runInGlobalTerminalPanel : nil,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/SetupAssistantView.swift`
at line 217, The helper function runInEmbeddedTerminalIfAvailable was changed to
open the global terminal panel so rename it and its call sites to match the new
behavior (e.g., runInGlobalTerminalPanel or openCommandInTerminalDrawer): rename
the function declaration private func runInEmbeddedTerminalIfAvailable(_
command: String) to the chosen name and update every invocation (currently at
call sites referenced in the review: lines 82, 94, 108, 125, 139, 151) to call
the new function name; ensure any documentation/comments and visibility remain
correct and run a project-wide search/replace for
runInEmbeddedTerminalIfAvailable to avoid missing references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/SetupAssistantView.swift`:
- Line 217: The helper function runInEmbeddedTerminalIfAvailable was changed to
open the global terminal panel so rename it and its call sites to match the new
behavior (e.g., runInGlobalTerminalPanel or openCommandInTerminalDrawer): rename
the function declaration private func runInEmbeddedTerminalIfAvailable(_
command: String) to the chosen name and update every invocation (currently at
call sites referenced in the review: lines 82, 94, 108, 125, 139, 151) to call
the new function name; ensure any documentation/comments and visibility remain
correct and run a project-wide search/replace for
runInEmbeddedTerminalIfAvailable to avoid missing references.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8393ddf and 0d5bfba.

📒 Files selected for processing (2)
  • apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/SetupAssistantView.swift
  • apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ShellView.swift
💤 Files with no reviewable changes (1)
  • apps/macos/Packages/Features/DashboardFeature/Sources/DashboardFeature/ShellView.swift

@roodboi
roodboi merged commit 7116db7 into main Feb 26, 2026
4 checks passed
@roodboi
roodboi deleted the fix/issue-15-macos-close-button branch February 26, 2026 22:42
roodboi pushed a commit that referenced this pull request Feb 26, 2026
## <small>1.13.3 (2026-02-26)</small>

* fix(macos): replace setup terminal modal with terminal drawer (#16) ([7116db7](7116db7)), closes [#16](#16) [#15](#15)
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.

macOS 1.13.1: missing close button can trap user in modal/screen

1 participant