feat: configurable recording-indicator position (top vs near cursor)#242
feat: configurable recording-indicator position (top vs near cursor)#242mvanhorn wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughAdds a "near cursor" placement mode for the recording overlay. A new ChangesNear-cursor recording overlay placement
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsView
participant UserDefaults
participant RecordingOverlayManager
participant AXUIElement
participant NSScreen
User->>SettingsView: selects "Near Cursor" in Show at picker
SettingsView->>UserDefaults: overlay_vertical_position = 1
User->>RecordingOverlayManager: starts dictation
RecordingOverlayManager->>RecordingOverlayManager: usesNearCursorOverlayPlacement = true
RecordingOverlayManager->>RecordingOverlayManager: nearCursorAnchorPoint()
RecordingOverlayManager->>AXUIElement: AXFocusedUIElement → AXSelectedTextBounds
alt AX bounds available
AXUIElement-->>RecordingOverlayManager: selection rect → anchor point
else fallback
RecordingOverlayManager->>RecordingOverlayManager: NSEvent.mouseLocation
end
RecordingOverlayManager->>NSScreen: screen(containing: anchorPoint)
RecordingOverlayManager->>RecordingOverlayManager: clampedFrame to visibleFrame
RecordingOverlayManager->>User: overlay appears near cursor/selection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/RecordingOverlay.swift (1)
464-464: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
usesNearCursorOverlayPlacementfor consistency.Elsewhere (
showOverlayPanelL326,makeOverlayContentL386) the placement decision usesusesNearCursorOverlayPlacement, but here it's the rawoverlayVerticalPosition == 1. They're equivalent only because theuseWingedLayoutearly-return at L449 already excluded the winged case; using the same computed property avoids a future refactor reintroducing a winged near-cursor frame by accident.♻️ Use the shared flag
- let usesNearCursorPosition = overlayVerticalPosition == 1 + let usesNearCursorPosition = usesNearCursorOverlayPlacement🤖 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` at line 464, Replace the raw comparison `overlayVerticalPosition == 1` with the existing computed property `usesNearCursorOverlayPlacement` on line 464 to maintain consistency with how the same placement decision is made in other methods like `showOverlayPanel` and `makeOverlayContent`. This ensures the same logic is used throughout and reduces the risk of reintroducing bugs in future refactoring.
🤖 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/RecordingOverlay.swift`:
- Around line 325-337: The entrance frame is being clamped to overlayScreen
derived from the mouse pointer location, but the final frame is positioned
relative to the anchor point which may be on a different screen. When the caret
and mouse pointer are on different displays, this causes incorrect clamping. Fix
this by ensuring overlayScreen is computed using the same screen logic as the
final frame, rather than using NSEvent.mouseLocation. Recompute the anchor point
or derive the screen from the anchor's location instead of the mouse location in
the screen(containing:fallback:) call when usesNearCursorOverlayPlacement is
true.
- Around line 157-172: The nearCursorAnchorPoint() function makes synchronous
Accessibility Framework calls on the main thread that can block indefinitely if
the focused application is unresponsive. To prevent UI freezing, use
AXUIElementSetMessagingTimeout to establish timeout limits on the AX messaging
calls. Set the messaging timeout on both the systemWide element immediately
after creating it and also on the focusedElement after retrieving it, ensuring
that the timeout applies throughout the entire call chain and prevents the main
thread from being blocked by unresponsive target applications.
---
Nitpick comments:
In `@Sources/RecordingOverlay.swift`:
- Line 464: Replace the raw comparison `overlayVerticalPosition == 1` with the
existing computed property `usesNearCursorOverlayPlacement` on line 464 to
maintain consistency with how the same placement decision is made in other
methods like `showOverlayPanel` and `makeOverlayContent`. This ensures the same
logic is used throughout and reduces the risk of reintroducing bugs in future
refactoring.
🪄 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: f84aca0d-1313-4710-88ea-51d64e232318
📒 Files selected for processing (2)
Sources/RecordingOverlay.swiftSources/SettingsView.swift
| private func nearCursorAnchorPoint() -> NSPoint { | ||
| let mouseLocation = NSEvent.mouseLocation | ||
| let systemWide = AXUIElementCreateSystemWide() | ||
| var focusedValue: CFTypeRef?, rangeValue: CFTypeRef?, boundsValue: CFTypeRef? | ||
| guard AXUIElementCopyAttributeValue(systemWide, kAXFocusedUIElementAttribute as CFString, &focusedValue) == .success, | ||
| let focusedRaw = focusedValue, CFGetTypeID(focusedRaw) == AXUIElementGetTypeID() else { return mouseLocation } | ||
| let focusedElement = unsafeBitCast(focusedRaw, to: AXUIElement.self) | ||
| guard AXUIElementCopyAttributeValue(focusedElement, kAXSelectedTextRangeAttribute as CFString, &rangeValue) == .success, | ||
| let rangeRaw = rangeValue, CFGetTypeID(rangeRaw) == AXValueGetTypeID(), | ||
| AXUIElementCopyParameterizedAttributeValue(focusedElement, kAXBoundsForRangeParameterizedAttribute as CFString, rangeRaw, &boundsValue) == .success, | ||
| let boundsRaw = boundsValue, CFGetTypeID(boundsRaw) == AXValueGetTypeID() else { return mouseLocation } | ||
| var rect = CGRect.zero | ||
| guard AXValueGetValue(unsafeBitCast(boundsRaw, to: AXValue.self), .cgRect, &rect), rect.height > 0, rect != .zero else { return mouseLocation } | ||
| let mainHeight = NSScreen.screens.first?.frame.height ?? mouseLocation.y + rect.maxY | ||
| return NSPoint(x: rect.midX, y: mainHeight - rect.maxY) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Does AXUIElementSetMessagingTimeout on the system-wide AXUIElement apply to child elements, and can AXUIElementCopyAttributeValue block the calling thread without a timeout?
💡 Result:
The function AXUIElementSetMessagingTimeout does not automatically apply to child elements when called on the system-wide AXUIElement [1][2]. According to official technical documentation, setting the timeout on an accessibility object (including the system-wide one) applies specifically to that object instance [1][2]. When you call AXUIElementSetMessagingTimeout on the system-wide accessibility object, it sets a global messaging timeout for the current process, which acts as a default for other accessibility objects that do not have their own specific timeout set [1][2]. If an individual accessibility element has had a specific timeout set on it, that element will use its own timeout rather than the global one [1][2]. Regarding AXUIElementCopyAttributeValue, this function can indeed block the calling thread [3]. The accessibility API performs inter-process communication (IPC) to retrieve information from other applications [4]. If the target application is unresponsive, busy, or otherwise unable to respond to the accessibility request in a timely manner, the calling thread will block while waiting for the operation to complete or time out [3]. Because these calls are synchronous, it is common practice to perform accessibility queries on a background thread to avoid hanging the main application thread [3]. Setting a messaging timeout is one way to define how long the API will wait before returning an error (such as kAXErrorCannotComplete) instead of blocking indefinitely [1][2][5].
Citations:
- 1: https://github.com/phracker/MacOSX-SDKs/blob/master/MacOSX10.5.sdk/System/Library/Frameworks/ApplicationServices.framework/Versions/A/Frameworks/HIServices.framework/Versions/A/Headers/AXUIElement.h
- 2: https://github.com/cdelouya/champ/blob/master/sdk/MacOSX10.9.sdk/System/Library/Frameworks/ApplicationServices.framework/Versions/A/Frameworks/HIServices.framework/Versions/A/Headers/AXUIElement.h
- 3: 3s+ lag when switching to space with unity editor nikitabobko/AeroSpace#497
- 4: https://fazm.ai/t/macos-accessibility-automation
- 5: https://stackoverflow.com/questions/27694912/axuielementcopyattributevalue-cannot-complete
🏁 Script executed:
# First, examine the file structure and find the relevant functions
cd Sources
cat -n RecordingOverlay.swift | head -200Repository: zachlatta/freeflow
Length of output: 9470
🏁 Script executed:
# Continue reading the file to find where nearCursorAnchorPoint is called
cd Sources
sed -n '174,400p' RecordingOverlay.swiftRepository: zachlatta/freeflow
Length of output: 8870
🏁 Script executed:
# Continue to find the overlayFrame property
cd Sources
sed -n '400,600p' RecordingOverlay.swiftRepository: zachlatta/freeflow
Length of output: 8152
Synchronous AX queries run on the main thread during overlay show — risk of UI hang.
nearCursorAnchorPoint() issues cross-process Accessibility calls (AXUIElementCopyAttributeValue, AXUIElementCopyParameterizedAttributeValue). It's reached via overlayFrame inside showOverlayPanel/updateOverlayLayout, which run on DispatchQueue.main. If the focused application is busy or unresponsive, these synchronous AX messages can block the main run loop until the system timeout elapses, freezing the app at the exact moment recording starts.
Bound the wait with a per-request timeout so a stalled target app can't freeze the overlay:
🔒 Bound AX messaging time
private func nearCursorAnchorPoint() -> NSPoint {
let mouseLocation = NSEvent.mouseLocation
let systemWide = AXUIElementCreateSystemWide()
+ // Don't let an unresponsive focused app stall the main thread.
+ AXUIElementSetMessagingTimeout(systemWide, 0.25)
var focusedValue: CFTypeRef?, rangeValue: CFTypeRef?, boundsValue: CFTypeRef?Note that AXUIElementSetMessagingTimeout on the system-wide element sets a default timeout for the current process; individual elements like focusedElement will use their own timeout if one is set explicitly. Consider setting the timeout on focusedElement as well to ensure the messaging limit applies throughout the entire call chain.
📝 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.
| private func nearCursorAnchorPoint() -> NSPoint { | |
| let mouseLocation = NSEvent.mouseLocation | |
| let systemWide = AXUIElementCreateSystemWide() | |
| var focusedValue: CFTypeRef?, rangeValue: CFTypeRef?, boundsValue: CFTypeRef? | |
| guard AXUIElementCopyAttributeValue(systemWide, kAXFocusedUIElementAttribute as CFString, &focusedValue) == .success, | |
| let focusedRaw = focusedValue, CFGetTypeID(focusedRaw) == AXUIElementGetTypeID() else { return mouseLocation } | |
| let focusedElement = unsafeBitCast(focusedRaw, to: AXUIElement.self) | |
| guard AXUIElementCopyAttributeValue(focusedElement, kAXSelectedTextRangeAttribute as CFString, &rangeValue) == .success, | |
| let rangeRaw = rangeValue, CFGetTypeID(rangeRaw) == AXValueGetTypeID(), | |
| AXUIElementCopyParameterizedAttributeValue(focusedElement, kAXBoundsForRangeParameterizedAttribute as CFString, rangeRaw, &boundsValue) == .success, | |
| let boundsRaw = boundsValue, CFGetTypeID(boundsRaw) == AXValueGetTypeID() else { return mouseLocation } | |
| var rect = CGRect.zero | |
| guard AXValueGetValue(unsafeBitCast(boundsRaw, to: AXValue.self), .cgRect, &rect), rect.height > 0, rect != .zero else { return mouseLocation } | |
| let mainHeight = NSScreen.screens.first?.frame.height ?? mouseLocation.y + rect.maxY | |
| return NSPoint(x: rect.midX, y: mainHeight - rect.maxY) | |
| } | |
| private func nearCursorAnchorPoint() -> NSPoint { | |
| let mouseLocation = NSEvent.mouseLocation | |
| let systemWide = AXUIElementCreateSystemWide() | |
| // Don't let an unresponsive focused app stall the main thread. | |
| AXUIElementSetMessagingTimeout(systemWide, 0.25) | |
| var focusedValue: CFTypeRef?, rangeValue: CFTypeRef?, boundsValue: CFTypeRef? | |
| guard AXUIElementCopyAttributeValue(systemWide, kAXFocusedUIElementAttribute as CFString, &focusedValue) == .success, | |
| let focusedRaw = focusedValue, CFGetTypeID(focusedRaw) == AXUIElementGetTypeID() else { return mouseLocation } | |
| let focusedElement = unsafeBitCast(focusedRaw, to: AXUIElement.self) | |
| guard AXUIElementCopyAttributeValue(focusedElement, kAXSelectedTextRangeAttribute as CFString, &rangeValue) == .success, | |
| let rangeRaw = rangeValue, CFGetTypeID(rangeRaw) == AXValueGetTypeID(), | |
| AXUIElementCopyParameterizedAttributeValue(focusedElement, kAXBoundsForRangeParameterizedAttribute as CFString, rangeRaw, &boundsValue) == .success, | |
| let boundsRaw = boundsValue, CFGetTypeID(boundsRaw) == AXValueGetTypeID() else { return mouseLocation } | |
| var rect = CGRect.zero | |
| guard AXValueGetValue(unsafeBitCast(boundsRaw, to: AXValue.self), .cgRect, &rect), rect.height > 0, rect != .zero else { return mouseLocation } | |
| let mainHeight = NSScreen.screens.first?.frame.height ?? mouseLocation.y + rect.maxY | |
| return NSPoint(x: rect.midX, y: mainHeight - rect.maxY) | |
| } |
🤖 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 157 - 172, The
nearCursorAnchorPoint() function makes synchronous Accessibility Framework calls
on the main thread that can block indefinitely if the focused application is
unresponsive. To prevent UI freezing, use AXUIElementSetMessagingTimeout to
establish timeout limits on the AX messaging calls. Set the messaging timeout on
both the systemWide element immediately after creating it and also on the
focusedElement after retrieving it, ensuring that the timeout applies throughout
the entire call chain and prevents the main thread from being blocked by
unresponsive target applications.
| let hiddenFrame: NSRect | ||
| if usesNearCursorOverlayPlacement { | ||
| let overlayScreen = self.screen(containing: NSEvent.mouseLocation, fallback: screen) | ||
| let entranceFrame = NSRect( | ||
| x: frame.origin.x, | ||
| y: frame.origin.y + frame.height, | ||
| width: frame.width, | ||
| height: frame.height | ||
| ) | ||
| hiddenFrame = clampedFrame(entranceFrame, to: overlayScreen.visibleFrame) | ||
| } else { | ||
| hiddenFrame = NSRect(x: frame.origin.x, y: screen.frame.maxY, width: frame.width, height: frame.height) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Entrance frame clamps to the pointer's screen, not the anchor's screen.
frame (from overlayFrame) is positioned relative to nearCursorAnchorPoint() — which prefers the focused caret over the pointer — and is clamped to the caret's screen. Here the entrance/hidden frame is instead clamped to screen(containing: NSEvent.mouseLocation, …). When the caret and the mouse pointer sit on different displays, the entrance frame is clamped to the wrong screen, producing a janky cross-monitor slide before settling at frame.
Recompute the anchor (or reuse it) so the entrance screen matches the final frame's screen.
🤖 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 325 - 337, The entrance frame is
being clamped to overlayScreen derived from the mouse pointer location, but the
final frame is positioned relative to the anchor point which may be on a
different screen. When the caret and mouse pointer are on different displays,
this causes incorrect clamping. Fix this by ensuring overlayScreen is computed
using the same screen logic as the final frame, rather than using
NSEvent.mouseLocation. Recompute the anchor point or derive the screen from the
anchor's location instead of the mouse location in the
screen(containing:fallback:) call when usesNearCursorOverlayPlacement is true.
1 participant
What
Adds an optional setting to choose where the recording / voice-input indicator appears: keep the current top-of-screen drop-down (default), or show it near the cursor / text caret while dictating.
Closes #212.
Why
On slow or unstable connections the recording indicator only lives at the top of the screen, so the user has to keep glancing up to confirm dictation is still active. For people dictating into a text field lower on the display, that means looking away from where the text is actually landing. This adds a "Show at" choice so the indicator can follow the active input instead.
How
Follows the same pattern already used for the multi-display "Show on" picker.
Sources/SettingsView.swift— new@AppStorage("overlay_vertical_position")key (0= top, default;1= near cursor) and a small "Show at"Pickeradded to the overlay settings group, mirroring the existingoverlayDisplaySectionlayout and accessibility wiring.Sources/RecordingOverlay.swift— the drop-down pill'soverlayFramebranches on the new key. Position0keeps the existingscreen.frame.maxY-anchored placement byte-for-byte (including notch handling), so existing users see no change. Position1anchors the pill just below the active text caret, resolved via the Accessibility API (focused element → selected-text-range bounds), and falls back toNSEvent.mouseLocationwhen AX bounds are unavailable. The result is clamped fully inside the target screen'svisibleFrameso it never clips at an edge, the entrance animation slides locally instead of dropping from the menu bar, and the pill rounds all four corners since it is no longer flush with the top. The minimalist notch / winged layout stays top-anchored.Behavior
overlay_display_idsetting.UserDefaults(@AppStorage) and survives relaunch.Testing
Built locally with
make CODESIGN_IDENTITY=-(ad-hoc signed) against the macOS SDK — clean compile of all sources including the two changed files.Summary by CodeRabbit