Skip to content

Fix mini-card close cleanup and confirm popover placement#15

Merged
jcanizalez merged 2 commits intomainfrom
fix/minicard-close-flow
Mar 14, 2026
Merged

Fix mini-card close cleanup and confirm popover placement#15
jcanizalez merged 2 commits intomainfrom
fix/minicard-close-flow

Conversation

@jcanizalez
Copy link
Copy Markdown
Owner

Summary

  • keep the close confirmation popover inside the viewport and flip it above the trigger when needed
  • remove sessions from the UI immediately when closing so dead mini cards do not linger
  • add a small position helper test for the new popover placement logic

Testing

  • yarn vitest run tests/popover-position.test.ts
  • yarn eslint src/renderer/components/ConfirmPopover.tsx src/renderer/components/AgentCard.tsx src/renderer/components/FocusedTerminal.tsx src/renderer/components/TabView.tsx src/renderer/lib/popover-position.ts src/renderer/lib/terminal-close.ts src/main/pty-manager.ts tests/popover-position.test.ts

Copilot AI review requested due to automatic review settings March 14, 2026 01:08
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves session-close UX and confirmation popover behavior by (1) centralizing terminal close/cleanup so sessions disappear from the UI immediately and (2) adding viewport-aware positioning logic for confirmation popovers (including flipping when needed).

Changes:

  • Add a shared closeTerminalSession helper to immediately remove sessions from UI and request backend termination.
  • Introduce calculatePopoverPosition to clamp/flip popover placement within the viewport and wire it into ConfirmPopover.
  • Add Vitest coverage for the new popover positioning helper.

Reviewed changes

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

Show a summary per file
File Description
tests/popover-position.test.ts Adds unit tests for horizontal clamping and vertical flip behavior.
src/renderer/lib/terminal-close.ts New shared close helper + pending-close tracking used by exit handler.
src/renderer/lib/popover-position.ts New pure helper to compute popover top/left and placement within viewport constraints.
src/renderer/components/TabView.tsx Switches tab close flow to use closeTerminalSession.
src/renderer/components/ProjectSidebar.tsx Switches sidebar session close flow to use closeTerminalSession.
src/renderer/components/FocusedTerminal.tsx Switches focused session close flow to use closeTerminalSession.
src/renderer/components/ConfirmPopover.tsx Uses viewport-aware positioning and adds flip + max width constraints.
src/renderer/components/AgentCard.tsx Switches mini-card close flow to use closeTerminalSession.
src/renderer/App.tsx Uses consumePendingTerminalClose to handle close-intent exit cleanup.
src/main/pty-manager.ts Ensures a terminal-exit IPC event is emitted even if the PTY is already missing.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/renderer/components/ConfirmPopover.tsx Outdated
Comment thread src/renderer/App.tsx Outdated
Comment thread src/renderer/components/ConfirmPopover.tsx
Copy link
Copy Markdown
Owner Author

@jcanizalez jcanizalez left a comment

Choose a reason for hiding this comment

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

Addressing review

Comment thread src/renderer/App.tsx Outdated
@jcanizalez
Copy link
Copy Markdown
Owner Author

Review Summary

All 3 Copilot review comments have been addressed:

  1. ConfirmPopover.tsx — closest('button') anchor issue — Fixed in b17be7a. Now uses a dedicated <span ref={anchorRef}> wrapper instead of walking up with closest("button").

  2. App.tsx — Double removeTerminal call — Already guarded at line 165 with if (state.terminals.has(id)), preventing redundant re-renders.

  3. ConfirmPopover.tsx — Stale anchorRect on scroll/resize — Fixed. anchorRef stores the actual DOM element, and handleViewportChange re-reads getBoundingClientRect() via readAnchorRect() on every scroll/resize.

Merge Conflict

The PR has a merge conflict in src/renderer/App.tsx. The conflict is trivial — main added three unused variables (_terminals, _activeProject, _taskCount) that should be dropped. Rebase onto main and resolve by keeping HEAD's version (without the _ prefixed vars).

@jcanizalez jcanizalez force-pushed the fix/minicard-close-flow branch from b17be7a to f0f90e9 Compare March 14, 2026 02:13
@jcanizalez jcanizalez merged commit aa31a7f into main Mar 14, 2026
1 check passed
jcanizalez added a commit that referenced this pull request Mar 14, 2026
- feat: extract standalone server for multi-client architecture (#18)
- feat: add workspaces to group projects (#17)
- feat: redesign kanban board cards and columns (#16)
- feat: add landing page with OS-detected download buttons (#14, #19)
- fix: mini-card close cleanup and confirm popover placement (#15)
jcanizalez added a commit that referenced this pull request Mar 14, 2026
…age (#20)

- feat: extract standalone server for multi-client architecture (#18)
- feat: add workspaces to group projects (#17)
- feat: redesign kanban board cards and columns (#16)
- feat: add landing page with OS-detected download buttons (#14, #19)
- fix: mini-card close cleanup and confirm popover placement (#15)
jcanizalez added a commit that referenced this pull request Mar 14, 2026
* v0.7.1: server extraction, workspaces, kanban redesign, and landing page

- feat: extract standalone server for multi-client architecture (#18)
- feat: add workspaces to group projects (#17)
- feat: redesign kanban board cards and columns (#16)
- feat: add landing page with OS-detected download buttons (#14, #19)
- fix: mini-card close cleanup and confirm popover placement (#15)

* v0.7.2: fix workspace switching and server startup

- Sync server database.ts with workspaces schema (was missing from #17)
- Broadcast config:changed events to WS clients on config save
- Add ws to root dependencies (fixes bufferutil resolve error)
- Fix server entry path resolution in dev mode
- Use npx tsx instead of Electron binary to spawn server in dev
jcanizalez added a commit that referenced this pull request Mar 14, 2026
* v0.7.1: server extraction, workspaces, kanban redesign, and landing page

- feat: extract standalone server for multi-client architecture (#18)
- feat: add workspaces to group projects (#17)
- feat: redesign kanban board cards and columns (#16)
- feat: add landing page with OS-detected download buttons (#14, #19)
- fix: mini-card close cleanup and confirm popover placement (#15)

* v0.7.2: fix workspace switching and server startup

- Sync server database.ts with workspaces schema (was missing from #17)
- Broadcast config:changed events to WS clients on config save
- Add ws to root dependencies (fixes bufferutil resolve error)
- Fix server entry path resolution in dev mode
- Use npx tsx instead of Electron binary to spawn server in dev

* feat: extract MCP server into standalone @vibegrid/mcp package

- Create @vibegrid/mcp package with stdio MCP server (npx @vibegrid/mcp)
- Move all 6 tool categories (21 tools) from src/main/mcp-tools/ to packages/mcp/
- Tools import from @vibegrid/server instead of Electron main process
- Remove --mcp mode, mcp-socket-server, mcp-proxy from Electron
- Update MCP settings UI to show npx @vibegrid/mcp commands
- Fix Zod 4 + MCP SDK compatibility for workflow tool schemas

* chore: remove duplicate manager files from src/main

Delete 18 old manager files that are now in packages/server/src/.
Update 4 test files to import from packages/server/src/ instead.

Remaining in src/main/: index.ts (thin shell), ipc-handlers.ts
(bridge delegation), menu.ts, update-manager.ts, logger.ts,
ipc-safe-handle.ts, server/ (bridge + launcher).

* fix: address Copilot review comments

- Replace dialog.showMessageBox with log.warn in server database.ts
  (server package has no Electron dependency)
- Remove private:true from @vibegrid/mcp so it can be published to npm
- Redirect console.info and console.debug to stderr in MCP entry point
  (prevents stdout corruption of JSON-RPC stream)
jcanizalez added a commit that referenced this pull request Mar 14, 2026
* v0.7.1: server extraction, workspaces, kanban redesign, and landing page

- feat: extract standalone server for multi-client architecture (#18)
- feat: add workspaces to group projects (#17)
- feat: redesign kanban board cards and columns (#16)
- feat: add landing page with OS-detected download buttons (#14, #19)
- fix: mini-card close cleanup and confirm popover placement (#15)

* feat: redesign task board UI with polished kanban, list, and inline task creation

- Kanban cards: task short IDs, status icons, date footer, top-right menu on hover
- List view: collapsible colored section bars with chevron toggle and animations
- Pill-style segmented view toggle for List/Board and Grid/Tabs
- Inline floating form for task creation replacing centered modal
- Add task buttons in kanban columns and list sections

* fix: address PR review comments

- Fix nested button in list view section headers (use div + proper button)
- Plumb status through to AddTaskDialog so column/section + buttons create
  tasks with the correct initial status
- Fix useEffect deps for dialog init (include editingTask)
- Use stable task ID suffix for short IDs instead of order field
@jcanizalez jcanizalez deleted the fix/minicard-close-flow branch March 14, 2026 15:04
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