Prevent accidental ESC from destroying recording#190
Prevent accidental ESC from destroying recording#190michaellustig-shopify wants to merge 1 commit intokitlangton:mainfrom
Conversation
When ESC is pressed during recording, instead of immediately destroying the recording, a compact confirmation overlay appears with two options: - "Continue Recording" (green, press C) — dismisses the dialog - "Destroy" (red, press ESC again) — cancels and deletes the recording The confirmation state is shared via @shared(.isConfirmingCancel) so the key event handler can gate the C-key shortcut. The dialog uses .ultraThinMaterial with bouncy scale transitions matching Hex's style. Also handles edge cases: transcription completing auto-dismisses the dialog, hotkey press while confirming treats as confirmed cancel, and re-pressing the hotkey after Continue correctly stops the recording. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughIntroduces an escape confirmation dialog to prevent accidental recording cancellation. On first ESC press, a confirmation overlay displays with "Continue Recording" and "Destroy" options. Pressing ESC again or clicking Destroy cancels the recording; continuing resumes recording uninterrupted. Changes
Sequence DiagramsequenceDiagram
participant User
participant TranscriptionView
participant TranscriptionFeature as Feature Logic
participant SoundClient
participant SettingsStore as State Store
User->>TranscriptionView: Press ESC (first time)
TranscriptionView->>TranscriptionFeature: .confirmCancel
TranscriptionFeature->>SoundClient: Play cancelWarning sound
TranscriptionFeature->>SettingsStore: Set isConfirmingCancel = true
SettingsStore-->>TranscriptionView: Update state
TranscriptionView->>TranscriptionView: Show CancelConfirmationView
User->>TranscriptionView: Press ESC (second time) or Click Destroy
TranscriptionView->>TranscriptionFeature: .confirmCancel again
TranscriptionFeature->>TranscriptionFeature: handleConfirmedCancel
TranscriptionFeature->>SettingsStore: Set isConfirmingCancel = false
SettingsStore-->>TranscriptionView: Update state
TranscriptionView->>TranscriptionView: Hide CancelConfirmationView & stop recording
alt User clicks Continue Recording or presses C
User->>TranscriptionView: Click Continue or Press C
TranscriptionView->>TranscriptionFeature: .dismissCancelConfirm
TranscriptionFeature->>SettingsStore: Set isConfirmingCancel = false
SettingsStore-->>TranscriptionView: Update state
TranscriptionView->>TranscriptionView: Hide CancelConfirmationView & resume recording
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip You can disable sequence diagrams in the walkthrough.Disable the |
|
Hey kit, this is certainly AI slop, but I'm using it and it works. Feel free to close it.
I think the only thing you may want to change is the sound effect, but I don't know how you make your cute little noises, so all the best, sir. |
|
the demo - Well, I was recording that demo, but I realized that the floating pill does not show up in recordings, so here's a screenshot. 
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/escape-confirmation.md:
- Line 5: Update the changeset summary line "Add escape confirmation dialog to
prevent accidental recording loss." to include the GitHub issue/PR tracking
token in the format "(... (`#NNN`))" (e.g., add " (`#190`)" or the actual PR/issue
number for this change) so the summary follows the guideline "Include
user-facing impact and GitHub issue/PR number".
In `@Hex/Features/Transcription/TranscriptionFeature.swift`:
- Around line 217-223: The SwiftLint `opening_brace` violation is caused by the
newline before the brace in the key handling conditional; update the conditional
that checks `keyEvent.key == .c, keyEvent.modifiers.isEmpty, isConfirmingCancel`
so the opening brace `{` is on the same line as the final condition (i.e., end
the `if` line with `{`), keeping the body that calls `Task { await
send(.dismissCancelConfirm) }` and `return true` unchanged; this touches the
`isConfirmingCancel` check and the `send(.dismissCancelConfirm)` call sites.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0602041b-1dbf-48f7-9ba0-14d41032c4c4
⛔ Files ignored due to path filters (1)
Hex/Resources/Audio/cancelWarning.mp3is excluded by!**/*.mp3
📒 Files selected for processing (5)
.changeset/escape-confirmation.mdHex/Clients/SoundEffect.swiftHex/Features/Settings/SettingsFeature.swiftHex/Features/Transcription/CancelConfirmationView.swiftHex/Features/Transcription/TranscriptionFeature.swift
| "hex-app": minor | ||
| --- | ||
|
|
||
| Add escape confirmation dialog to prevent accidental recording loss. First ESC shows a Destroy/Continue overlay; second ESC or clicking Destroy cancels the recording. |
There was a problem hiding this comment.
Add the issue/PR reference to the changeset summary.
The summary should include the tracking reference token (e.g., (#190)).
Proposed edit
-Add escape confirmation dialog to prevent accidental recording loss. First ESC shows a Destroy/Continue overlay; second ESC or clicking Destroy cancels the recording.
+Add escape confirmation dialog to prevent accidental recording loss (`#190`). First ESC shows a Destroy/Continue overlay; second ESC or clicking Destroy cancels the recording.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Add escape confirmation dialog to prevent accidental recording loss. First ESC shows a Destroy/Continue overlay; second ESC or clicking Destroy cancels the recording. | |
| Add escape confirmation dialog to prevent accidental recording loss (`#190`). First ESC shows a Destroy/Continue overlay; second ESC or clicking Destroy cancels the recording. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/escape-confirmation.md at line 5, Update the changeset summary
line "Add escape confirmation dialog to prevent accidental recording loss." to
include the GitHub issue/PR tracking token in the format "(... (`#NNN`))" (e.g.,
add " (`#190`)" or the actual PR/issue number for this change) so the summary
follows the guideline "Include user-facing impact and GitHub issue/PR number".
| // If "C" is pressed with no modifiers while confirming cancel, continue recording. | ||
| if keyEvent.key == .c, keyEvent.modifiers.isEmpty, | ||
| isConfirmingCancel | ||
| { | ||
| Task { await send(.dismissCancelConfirm) } | ||
| return true | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'if keyEvent\.key == \.c, keyEvent\.modifiers\.isEmpty,' Hex/Features/Transcription/TranscriptionFeature.swiftRepository: kitlangton/Hex
Length of output: 277
Move opening brace to same line as condition to resolve SwiftLint opening_brace rule.
The opening brace on line 220 should be moved to the same line as the final condition on line 219.
Proposed edit
- if keyEvent.key == .c, keyEvent.modifiers.isEmpty,
- isConfirmingCancel
- {
+ if keyEvent.key == .c, keyEvent.modifiers.isEmpty,
+ isConfirmingCancel {
Task { await send(.dismissCancelConfirm) }
return true
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If "C" is pressed with no modifiers while confirming cancel, continue recording. | |
| if keyEvent.key == .c, keyEvent.modifiers.isEmpty, | |
| isConfirmingCancel | |
| { | |
| Task { await send(.dismissCancelConfirm) } | |
| return true | |
| } | |
| if keyEvent.key == .c, keyEvent.modifiers.isEmpty, | |
| isConfirmingCancel { | |
| Task { await send(.dismissCancelConfirm) } | |
| return true | |
| } |
🧰 Tools
🪛 SwiftLint (0.63.2)
[Warning] 220-220: Opening braces should be preceded by a single space and on the same line as the declaration
(opening_brace)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 217 -
223, The SwiftLint `opening_brace` violation is caused by the newline before the
brace in the key handling conditional; update the conditional that checks
`keyEvent.key == .c, keyEvent.modifiers.isEmpty, isConfirmingCancel` so the
opening brace `{` is on the same line as the final condition (i.e., end the `if`
line with `{`), keeping the body that calls `Task { await
send(.dismissCancelConfirm) }` and `return true` unchanged; this touches the
`isConfirmingCancel` check and the `send(.dismissCancelConfirm)` call sites.
2 participants
Summary
Ckey) dismisses the dialogESCagain) cancels and deletes the recordingTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit