Show a subtle "still recording" sheen every 30s during long takes#215
Open
ojhurst wants to merge 1 commit into
Open
Show a subtle "still recording" sheen every 30s during long takes#215ojhurst wants to merge 1 commit into
ojhurst wants to merge 1 commit into
Conversation
The mic waveform is small. When a recording is left running by accident — forgotten toggle, started by mistake — there isn't much in the menu bar to catch the eye. This adds a periodic sheen sweep across the recording overlay every 30 seconds: a quiet "yep, still here, still recording" that's easy to ignore when the recording is intentional and hard to miss when it isn't. The sheen widens the overlay slightly, sweeps a soft highlight across left and right wings with a small stagger, then returns. No sound, no modal, no notification. The cadence resets when recording is toggled on, so the first sheen lands at the 30-second mark and repeats from there. Implementation: - AppState: per-recording timer, reset on toggle start/stop - RecordingOverlay: sheen sweep animation, staggered left/right wings, expansion and collapse around the recording overlay's shape Notes from the design pass: - Cadence: shorter intervals felt like a notification; 30 seconds reads as periodic reassurance. - Sheen-only: earlier passes tried widening or pulsing the overlay on its own; the sweep alone was the lightest touch that still caught the eye. - Brightness: the peak opacity was tuned between "flashing alert" and "easy to miss" — the committed value sits between. - Pill variants and non-notched Macs: the sheen geometry is anchored to the recording overlay's own bounds, not absolute pixel positions, so it scales naturally to wider pill layouts and to Macs without a notch — no per-layout code path needed. - First-sheen timing: the 30s timer resets on every recording toggle, so the first sheen always lands 30 seconds after start, never immediately. On by default; the sheen is gentle enough to leave running.
Contributor
📝 WalkthroughWalkthroughThis PR implements a visual reminder system for toggle-mode recording sessions. AppState now tracks per-session dictation duration via recordingSessionStartTime, and AppState manages a DispatchSourceTimer that periodically pulses an overlay reminder. RecordingOverlay extends the UI to show error/silence warnings, render an animated sheen effect on the stop button, and adapt layout width to accommodate the expanded reminder pill. ChangesToggle Recording Reminder Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/AppState.swift`:
- Around line 530-545: Add stored properties to AppState for
recordingSessionStartTime and wpmTrackedSeconds so the isRecording.didSet block
compiles and updates state correctly: declare recordingSessionStartTime as an
optional Date (default nil) and wpmTrackedSeconds as a TimeInterval/Double
(default 0) as members of AppState, ensure their access level matches existing
properties, and keep using
WordCountTracker.saveWPMTrackedSeconds(wpmTrackedSeconds) where present to
persist the value.
In `@Sources/RecordingOverlay.swift`:
- Around line 246-271: showError races with showSilenceWarning because each
schedules a delayed dismiss that only checks its own message key and phase; fix
by clearing the opposite feedback message immediately when presenting (in
showError clear overlayState.silenceWarningMessage; in showSilenceWarning clear
overlayState.errorMessage) and update the delayed closures in both showError and
showSilenceWarning to verify both the phase is .feedback and that both
overlayState.errorMessage and overlayState.silenceWarningMessage still match the
truncated/shown values before dismissing so an earlier callback cannot stomp a
newer feedback state (adjust references to overlayState, errorMessage,
silenceWarningMessage, showError, showSilenceWarning, and their dismiss
closures).
- Around line 12-14: The view is not reading the `@Published` state so setting
errorMessage and silenceWarningMessage has no visible effect; update
RecordingOverlay's SwiftUI body to consume those properties (errorMessage and
silenceWarningMessage) and conditionally render inline Text or a small
Banner/Overlay when they are non-nil, using the existing truncation/timer logic
that sets them; ensure state updates happen on the main thread
(DispatchQueue.main.async) when you set the published vars and consider using
.onChange(of: errorMessage) / .onChange(of: silenceWarningMessage) or .animation
to drive show/hide transitions, and remove/reuse isToggleReminderSheening only
if it controls visual state so the new message UI reflects the current boolean.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e4513937-f034-451e-99d6-ae05c87543be
📒 Files selected for processing (2)
Sources/AppState.swiftSources/RecordingOverlay.swift
Comment on lines
+530
to
+545
| // Build 119: capture per-session dictation duration so the WPM | ||
| // stat in MainWindow's right rail has real numerator/denominator | ||
| // material. Counts only the recording window; transcription and | ||
| // post-processing happen after isRecording flips back to false. | ||
| if isRecording { | ||
| recordingSessionStartTime = Date() | ||
| } else { | ||
| stopToggleRecordingReminder() | ||
| if let start = recordingSessionStartTime { | ||
| let elapsed = Date().timeIntervalSince(start) | ||
| if elapsed > 0 { | ||
| wpmTrackedSeconds += elapsed | ||
| WordCountTracker.saveWPMTrackedSeconds(wpmTrackedSeconds) | ||
| } | ||
| recordingSessionStartTime = nil | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify symbol declarations and usages for new WPM/session tracking references.
rg -n --type=swift '\b(recordingSessionStartTime|wpmTrackedSeconds)\b'
rg -n --type=swift '\b(var|let)\s+recordingSessionStartTime\b|\b(var|let)\s+wpmTrackedSeconds\b'Repository: zachlatta/freeflow
Length of output: 494
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=swift '\brecordingSessionStartTime\b' Sources/AppState.swift
rg -n --type=swift '\bwpmTrackedSeconds\b' Sources/AppState.swift
# Try to find likely stored-property declarations (var/let) and computed properties
rg -n --type=swift 'recordingSessionStartTime\s*[:=]' Sources/AppState.swift || true
rg -n --type=swift 'wpmTrackedSeconds\s*[:=]' Sources/AppState.swift || true
# Wider search across repo to see if declarations live elsewhere
rg -n --type=swift '\brecordingSessionStartTime\b' .
rg -n --type=swift '\bwpmTrackedSeconds\b' .
# If found in AppState.swift, show some surrounding context for the first declaration hit
python3 - <<'PY'
import subprocess, re, sys
def get_first_line(pattern):
p = subprocess.run(["bash","-lc",f"rg -n --type=swift '{pattern}' Sources/AppState.swift | head -n 1"],capture_output=True,text=True)
return p.stdout.strip()
for sym, pat in [("recordingSessionStartTime", r"recordingSessionStartTime\s*[:=]"),("wpmTrackedSeconds", r"wpmTrackedSeconds\s*[:=]")]:
line = get_first_line(pat)
if not line:
print(f"No likely declaration match in AppState.swift for {sym}")
continue
m = re.match(r"Sources/AppState\.swift:(\d+):", line)
ln = int(m.group(1))
# print 20 lines around
subprocess.run(["bash","-lc",f"nl -ba Sources/AppState.swift | sed -n '{max(1,ln-20)},{ln+20}p'"])
PYRepository: zachlatta/freeflow
Length of output: 1083
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== recordingSessionStartTime occurrences =="
rg -n --type=swift '\brecordingSessionStartTime\b' . || true
echo
echo "== wpmTrackedSeconds occurrences =="
rg -n --type=swift '\bwpmTrackedSeconds\b' . || true
echo
echo "== Potential declarations (var/let/typed) in repo =="
rg -n --type=swift '(\bvar\b|\blet\b)\s+recordingSessionStartTime\b' . || true
rg -n --type=swift '(\bvar\b|\blet\b)\s+wpmTrackedSeconds\b' . || true
echo
echo "== typed/explicit definitions =="
rg -n --type=swift 'recordingSessionStartTime\s*:' . || true
rg -n --type=swift 'wpmTrackedSeconds\s*:' . || true
echo
echo "== Count occurrences in AppState.swift =="
rg -n --type=swift '\brecordingSessionStartTime\b' Sources/AppState.swift | wc -l
rg -n --type=swift '\bwpmTrackedSeconds\b' Sources/AppState.swift | wc -lRepository: zachlatta/freeflow
Length of output: 720
🏁 Script executed:
#!/bin/bash
nl -ba Sources/AppState.swift | sed -n '480,575p'Repository: zachlatta/freeflow
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n Sources/AppState.swift | sed -n '480,575p'Repository: zachlatta/freeflow
Length of output: 4508
Critical: recordingSessionStartTime / wpmTrackedSeconds have no declarations in AppState scope
Sources/AppState.swift uses recordingSessionStartTime and wpmTrackedSeconds inside isRecording.didSet, but repo-wide there are no var/let (or typed) declarations for either symbol anywhere—so this code should not compile. Add these as stored properties of AppState (or otherwise make them accessible in this scope) before using them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/AppState.swift` around lines 530 - 545, Add stored properties to
AppState for recordingSessionStartTime and wpmTrackedSeconds so the
isRecording.didSet block compiles and updates state correctly: declare
recordingSessionStartTime as an optional Date (default nil) and
wpmTrackedSeconds as a TimeInterval/Double (default 0) as members of AppState,
ensure their access level matches existing properties, and keep using
WordCountTracker.saveWPMTrackedSeconds(wpmTrackedSeconds) where present to
persist the value.
Comment on lines
+12
to
+14
| @Published var errorMessage: String? | ||
| @Published var silenceWarningMessage: String? | ||
| @Published var isToggleReminderSheening = false |
Contributor
There was a problem hiding this comment.
Render path does not consume new error/silence message state.
errorMessage and silenceWarningMessage are set, truncated, and timed, but feedback UI still renders only generic failure icons. The inline message APIs currently have no visible effect.
Also applies to: 246-257, 279-285
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/RecordingOverlay.swift` around lines 12 - 14, The view is not reading
the `@Published` state so setting errorMessage and silenceWarningMessage has no
visible effect; update RecordingOverlay's SwiftUI body to consume those
properties (errorMessage and silenceWarningMessage) and conditionally render
inline Text or a small Banner/Overlay when they are non-nil, using the existing
truncation/timer logic that sets them; ensure state updates happen on the main
thread (DispatchQueue.main.async) when you set the published vars and consider
using .onChange(of: errorMessage) / .onChange(of: silenceWarningMessage) or
.animation to drive show/hide transitions, and remove/reuse
isToggleReminderSheening only if it controls visual state so the new message UI
reflects the current boolean.
Comment on lines
+246
to
+271
| func showError(_ message: String, dismissAfter: TimeInterval = 6.0) { | ||
| let truncated: String = { | ||
| if message.count <= Self.maxToastMessageLength { return message } | ||
| let cutoff = message.index(message.startIndex, offsetBy: Self.maxToastMessageLength - 1) | ||
| return String(message[..<cutoff]) + "…" | ||
| }() | ||
| DispatchQueue.main.async { | ||
| self.resetToggleRecordingReminder() | ||
| self.overlayState.errorMessage = truncated | ||
| self.lockedOverlayWidth = nil | ||
| self.overlayState.phase = .feedback | ||
| self.showOverlayPanel(animatedResize: true) | ||
| // Default 6s — 3s was too brief for rotation toasts to be read | ||
| // before they auto-dismissed. Callers that surface lower-stakes | ||
| // info (e.g. "Audio too short") may pass a shorter duration. | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + dismissAfter) { [weak self] in | ||
| guard let self = self else { return } | ||
| guard self.overlayState.phase == .feedback, | ||
| self.overlayState.errorMessage == truncated else { | ||
| return | ||
| } | ||
| self.overlayState.errorMessage = nil | ||
| self.dismissAll() | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Clear the opposite feedback message before scheduling dismiss callbacks.
showError and showSilenceWarning can race each other because each delayed closure only checks its own message key and .feedback phase. If one is shown after the other, the earlier callback can still flip phase back to .recording and stomp the newer feedback state.
💡 Proposed fix
func showError(_ message: String, dismissAfter: TimeInterval = 6.0) {
@@
DispatchQueue.main.async {
self.resetToggleRecordingReminder()
+ self.overlayState.silenceWarningMessage = nil
self.overlayState.errorMessage = truncated
@@
func showSilenceWarning(_ message: String) {
DispatchQueue.main.async {
self.resetToggleRecordingReminder()
+ self.overlayState.errorMessage = nil
self.overlayState.silenceWarningMessage = messageAlso applies to: 279-297
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/RecordingOverlay.swift` around lines 246 - 271, showError races with
showSilenceWarning because each schedules a delayed dismiss that only checks its
own message key and phase; fix by clearing the opposite feedback message
immediately when presenting (in showError clear
overlayState.silenceWarningMessage; in showSilenceWarning clear
overlayState.errorMessage) and update the delayed closures in both showError and
showSilenceWarning to verify both the phase is .feedback and that both
overlayState.errorMessage and overlayState.silenceWarningMessage still match the
truncated/shown values before dismissing so an earlier callback cannot stomp a
newer feedback state (adjust references to overlayState, errorMessage,
silenceWarningMessage, showError, showSilenceWarning, and their dismiss
closures).
Contributor
Author
|
I have had this running in my daily driver now for a little while, and it's definitely helped me at least three different times. It's helped me catch the fact that I accidentally left it on. |
Collaborator
|
i'm not against showing a still recording reminder but not sure the sheen matches the rest of the app |
Contributor
Author
|
Have you tried the sheen? I feel like it's just a really subtle thing. Do you have any other mechanisms you would propose instead? |
Collaborator
|
Hi, could you make sure the sheen is restricted to the shape of the actual overlay? In the GIF you added it seems to be restricted to a square Could you also address the coderabbit comments? Thanks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The mic waveform is small. When a recording is left running by accident — forgotten toggle, started by mistake — there isn't much in the menu bar to catch the eye. This adds a periodic sheen sweep across the recording overlay every 30 seconds: a quiet "yep, still here, still recording" that's easy to ignore when the recording is intentional and hard to miss when it isn't.
The sheen widens the overlay slightly, sweeps a soft highlight across left and right wings with a small stagger, then returns. No sound, no modal, no notification. The cadence resets when recording is toggled on, so the first sheen lands at the 30-second mark and repeats from there.
Implementation
AppState: per-recording timer, reset on toggle start/stopRecordingOverlay: sheen sweep animation, staggered left/right wings, expansion and collapse around the recording overlay's shapeNotes from the design pass
A few things considered along the way:
Defaults
On by default. The sheen is gentle enough to leave running, and the 30-second cadence was tuned to feel like a periodic acknowledgement rather than a notification.
Summary by CodeRabbit
Release Notes