Skip to content

fix: close selected canvas nodes and confirm macOS quit - #410

Merged
DeadWaveWave merged 6 commits into
mainfrom
codex/issue-405
Sep 10, 2026
Merged

fix: close selected canvas nodes and confirm macOS quit#410
DeadWaveWave merged 6 commits into
mainfrom
codex/issue-405

Conversation

@DeadWaveWave

@DeadWaveWave DeadWaveWave commented Sep 9, 2026

Copy link
Copy Markdown
Owner

💡 Change Scope

  • Small Change: Fast feedback, localized UI/logic, low-risk.
  • Large Change: New feature, cross-boundary logic, runtime-risk (persistence, IPC, lifecycle, recovery).

📝 What Does This PR Do?

Fixes #405. Cmd+W on macOS and Ctrl+W on Windows/Linux close the node receiving keyboard focus in the active canvas, including terminal/Agent and editor input without canvas selection. That focused node takes priority over stale canvas selection. Without node focus, one selected node is closed; ambiguous multiselection shows an in-app warning. With neither focus nor selection, the shortcut does nothing and the application stays open.

Cmd+Q on macOS requires two non-repeat presses within 1.5 seconds and shows a localized confirmation hint after the first press. Explicit menu Quit remains available; Windows/Linux Ctrl+Q retains its existing terminal semantics.

🏗️ Large Change Spec (Required if "Large Change" is checked)

1. Context & Business Logic

Application shortcuts are reserved at Electron's input boundary before native accelerators and terminal/page input. Chromium's deliberate quit confirmation is adapted to Electron: actual macOS Cmd+Q input can omit keyup, so confirmation uses two non-repeat keydowns.

Terminal body and editor clicks intentionally establish input focus without canvas selection. Close therefore resolves actual DOM focus in the active canvas first, then falls back to a single selected node. It never mutates global selection to make the shortcut work. Hidden nodes and focus in another canvas are excluded. Documents retain their existing save/conflict close flow for both focused and selected targets.

2. State Ownership & Invariants

Main owns transient quit confirmation and lifecycle cancellation. Renderer owns DOM focus, selection, and existing node close operations. Preload validates the one-way semantic event contract.

  • Close never closes the host application; only an eligible active-canvas target is closed.
  • Input focus takes priority over selection; document save-before-close remains authoritative and duplicate asynchronous closes are suppressed.
  • Autorepeat never confirms quit; confirmation enters the existing quit coordinator once. No new durable state or recovery contract is introduced.

Cancellation emits IPC only when a hint is pending. Cleanup retains the original WebContents reference so a destroyed BrowserWindow is not dereferenced from its closed callback. No executable-rule impact.

3. Verification Plan & Regression Layer

Final input-focus correction: OPENCOVE_REQUIRE_STAGED=1 pnpm pre-commit exited 0 on the final four staged files. Related tests: 33 passed; native terminal recovery: 8 passed; full macOS Electron E2E: 326 passed, 83 skipped (22.2 minutes), with zero retries and no crash fallback. No source changes occurred during this successful gate.

Focused unit regressions first failed against the old selection requirement, then passed after the correction. Coverage includes no selection, stale single/multiple selection, external-canvas focus, hidden nodes, disabled canvas, document focus/selection save delegation, duplicate closes, and late failures.

Three targeted close E2E cases passed after a fresh build (7.2 seconds), then passed again in the full gate: clicking and typing in an xterm textarea or note textarea with zero selected canvas nodes, plus existing empty-canvas/selected-node behavior. These tests use real UI clicks and typed input, then Electron webContents.sendInputEvent to simulate the native Main input stream and assert before-input-event delivery. They do not claim physical keyboard verification. Page-level Playwright keyboard injection did not exercise this Main shortcut path. The macOS double-press quit E2E also passed in the full gate.

Latest-head CI is green for b89674192aa18b9e7ca393aa69637c41296f535f (run). Windows passed 47 E2E tests / 4 skipped, explicitly including terminal and note input-focus Ctrl+W cases and empty-canvas/selected-node behavior. Linux modifier policy remains covered by unit tests. The initial local attempt stopped at a new test-variable lint error, corrected before the successful full gate.

The first CI attempt failed two existing macOS tests: a sidebar width sample near the end of its CSS transition and a drag fault-injection pending-state check across two asynchronous evaluations. Both passed once locally after fresh builds with zero retries; the failed CI shards then passed on one rerun without code changes. These initial failures are retained here rather than counted as a clean first CI attempt. The branch was synchronized with main; only CHANGELOG required manual conflict resolution, preserving all entries.

✅ Delivery & Compliance Checklist

  • My code passes the ultimate gatekeeper: pnpm pre-commit is completely green.
  • I have signed the CLA if required (see CLA.md).
  • I have included new tests to lock down the behavior (or explicitly stated why it's untestable).
  • I have strictly adhered to the DEVELOPMENT.md architectural boundaries.
  • I have attached a screenshot or screen recording (if this touches the UI).
  • I have updated the documentation accordingly (if adding a feature or changing a contract).

📸 Screenshots / Visual Evidence

Playwright assertions cover actual terminal/note input focus, target removal, sibling/app survival, and confirmed quit behavior. No screenshot is attached and no review-only media is committed. This PR remains unmerged for user testing.

@DeadWaveWave
DeadWaveWave merged commit 3a98e9e into main Sep 10, 2026
19 of 21 checks passed
@DeadWaveWave
DeadWaveWave deleted the codex/issue-405 branch September 10, 2026 17:48
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.

[BUG] 高危按键操作未拦截,无强提醒

1 participant