feat(overlay): let users drag the bottom recording overlay - #974
schneidermilanistvan-hash wants to merge 8 commits into
Conversation
The bottom overlay is anchored to the centre of the screen edge, so it can cover whatever is underneath it — a text field, a toolbar, a button — with no way to move it aside. Overlay Position only offers top or bottom, and Bottom Offset only shifts it vertically. This makes the overlay draggable and remembers where it was put: - BottomOverlayPanel gains `allowsUserDragging`, which bypasses AppKit's keep-on-screen clamping for user-driven moves, so the overlay can be pushed past a screen edge and out of the way. - `isMovableByWindowBackground` is enabled on the panel. Clicks on the pill's own controls still reach them; only the inert background starts a drag. - The resting origin is persisted to `OverlayCustomOrigin`. `didMove` fires on every step of a drag, so the write is coalesced and only the final position is stored. An `isApplyingProgrammaticFrame` guard keeps the app's own repositioning from being recorded as a user drag. - `positionWindow()` honours a stored origin instead of re-centring, which otherwise overwrote the dragged position on every presentation. - Settings grows a "Reset Position" row, shown only once a custom position exists. Without it, dragging the overlay fully off screen would be unrecoverable, since the grab area is then off screen too. The row lives in its own view so SettingsView stays within the type_body_length limit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Greptile SummaryThe latest changes pair the in-memory dragged origin with the display arrangement on which it was selected, preventing an in-progress drag from being evaluated against stale persisted screen metadata.
Confidence Score: 5/5The PR appears safe to merge; the latest placement changes address the previous race and multi-display concerns without introducing a new actionable failure. The current implementation synchronously cancels pending saves on external position changes, keeps each live origin paired with the arrangement sampled during its drag, and validates visibility against individual screens when that arrangement changes. All previous threads are resolved, and no blocking or non-blocking findings remain. Reviews (8): Last reviewed commit: "fix(overlay): judge the live origin by t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 739fdbb571
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| SettingsStore.shared.overlayCustomOrigin = origin | ||
| } | ||
| } | ||
| self.pendingOriginSave = save | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.2, execute: save) |
There was a problem hiding this comment.
When transcription changes the dynamic preview size while the user is dragging, refreshDynamicPreviewSizeIfNeeded can run positionWindow() after 80 ms, while this work item does not update overlayCustomOrigin until the pointer has been still for 200 ms. Consequently positionWindow() reads the previous origin—or nil on the first drag—and snaps the panel back to its old/anchored position during the drag. Track the current drag origin in memory immediately and debounce only the UserDefaults write, or avoid programmatic repositioning while a drag is active.
Useful? React with 👍 / 👎.
| /// Origin the user dragged the recording overlay to, in screen coordinates. | ||
| /// `nil` means the overlay uses its default anchored placement. | ||
| var overlayCustomOrigin: CGPoint? { |
There was a problem hiding this comment.
When a user exports settings after positioning the overlay and later imports that backup, SettingsBackupPayload, makeBackupPayload(), and restore(from:) omit this newly persisted preference even though they handle the adjacent overlay position, offset, and size settings. A fresh installation therefore loses the saved placement, while importing over an existing installation can retain an unrelated local origin—including an off-screen one—despite reporting that settings were restored. Add an optional backward-compatible backup field, or explicitly clear the local origin during restore if coordinates should not migrate.
Useful? React with 👍 / 👎.
The first commit stored the dragged origin as an absolute point and always reused it. Drag the overlay onto an external display, unplug that display, and the overlay reopens outside the visible desktop — where the grab area is also off screen, so the only way back is knowing that Reset Position exists. Clamping the stored origin onto the visible desktop would be the obvious fix, but it would also undo a position the user deliberately dragged past a screen edge to get the overlay out of the way, which is the point of the feature. So the display arrangement is stored next to the origin: - `OverlayCustomOriginDesktopFrame` records the union of all screen frames at the moment the position is saved. - On the same arrangement the stored origin is honoured exactly, including a position dragged fully past an edge. - Once the arrangement differs, the origin is only reused while the overlay would still intersect the visible desktop; otherwise positioning falls back to the anchored default. - The stored origin is kept rather than cleared, so reconnecting the display restores the chosen position. Also mentions dragging in the Bottom Offset description, since nothing else signals that the overlay can be moved. Verified against the decision table: same-display on-screen, same-display dragged past the edge, external unplugged and stranded, external unplugged but still visible, no stored arrangement (on and off screen), and external reattached. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Pushed a follow-up commit ( The gap: the dragged origin was stored as an absolute point and always reused. Drag the overlay onto an external display, unplug it, and the overlay reopens outside the visible desktop — where the grab area is off screen too, so the only way back is already knowing that Reset Position exists. Clamping the stored origin onto the visible desktop would be the obvious fix, but it would also undo a position deliberately dragged past a screen edge, which is the point of the feature. So the display arrangement is stored next to the origin:
Verified against this decision table:
Also added a mention of dragging to the Bottom Offset description, since nothing else signalled the overlay could be moved. Still true from the original PR: builds clean, 🤖 Generated with Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dce5665ea0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// The whole desktop area, i.e. the union of every attached screen's frame. | ||
| private static func currentDesktopFrame() -> CGRect { | ||
| NSScreen.screens.reduce(CGRect.null) { $0.union($1.frame) } | ||
| } |
There was a problem hiding this comment.
currentDesktopFrame() discards the actual screen topology, so the recovery check can still restore the overlay into an area where no display exists. For example, with staggered displays, unplugging a monitor that occupied an interior gap may leave the bounding union unchanged; framesMatch then returns true unconditionally, and even when it differs, desktop.intersects treats that gap as visible. Persist the individual screen frames and require intersection with at least one currently attached screen before restoring the origin.
Useful? React with 👍 / 👎.
Three problems the automated reviews on altic-dev#974 found, all real. **The overlay snapped back mid-drag.** Only the write to settings was meant to be coalesced, but the origin itself lived nowhere else, so anything that repositioned the panel during a drag read the pre-drag origin — or nil, on the first drag ever — and moved the overlay out from under the pointer. That is not a corner case: the preview text growing during dictation calls `refreshSizeForContent()` after 80 ms, well inside the 200 ms save window, and dictation is exactly when the overlay is on screen to be dragged. The origin is now held in memory the moment the pointer moves; only the write is debounced. **Reset Position could undo itself.** A save queued by a drag survived the reset that followed it and wrote the origin back afterwards. The reset now cancels the pending save and clears the in-memory origin. **A gap between displays counted as desktop.** The previous commit stored the union of all screen frames, which loses the topology twice over: unplugging a display that sat inside the bounding rectangle of the others leaves the union unchanged, so the arrangement looked identical; and in an irregular arrangement the union covers space no display draws, so an overlay stranded in that gap was treated as reachable. Both left the overlay invisible with nothing to grab. Each screen's frame is now stored separately (`OverlayCustomOriginScreenFrames`), arrangements are compared as sets, and the recovery check requires the overlay to overlap a screen that is actually attached. Also carries the custom origin and its arrangement through settings backup/restore, which had been left out while the adjacent overlay position, offset and size were included. Both fields are optional, so older backups still decode, and the arrangement is restored before the origin — clearing the origin also clears the arrangement, so the two cannot disagree. Decision table, extended with the flagged cases and re-verified (12/12): same display on-screen, same display dragged past the edge, external unplugged and stranded, external unplugged but still visible, no stored arrangement (on and off screen), external reattached, screens reported in a different order, interior display unplugged with the union unchanged, irregular arrangement with the position in a covered gap, irregular arrangement with the position on an attached screen, and no screens attached. The last three fail under the union logic this replaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Pushed Snap-back mid-drag (Codex, Reset can be undone (Greptile P2, same line) — a save queued by a drag survived the reset that followed it. Reset now cancels the pending save and clears the in-memory origin. Reaching it by hand means clicking Reset within 200 ms of the last pixel of movement, so this is insurance rather than a bug anyone would hit, but the guard costs three lines. Display gaps appear reachable (Greptile P1) and track individual screens instead of their bounding union (Codex, Settings backup (Codex, Decision table re-verified against the new logic, 12/12:
The three bold-or-last rows are the ones the union logic got wrong. Build succeeds, 🤖 Generated with Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1731f83c17
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if let customOrigin = self.liveCustomOrigin ?? SettingsStore.shared.overlayCustomOrigin, | ||
| Self.storedOriginIsUsable(customOrigin, size: window.frame.size) |
There was a problem hiding this comment.
After the overlay has been dragged once in the current process, liveCustomOrigin remains non-nil and always wins over SettingsStore.shared.overlayCustomOrigin. Importing a backup with a different non-nil custom origin therefore updates UserDefaults but not the displayed position, and every subsequent presentation continues using the pre-import location until the user resets or restarts the app. Synchronize or clear the live cache when the origin-change notification represents an externally restored value.
Useful? React with 👍 / 👎.
| let save = DispatchWorkItem { | ||
| MainActor.assumeIsolated { | ||
| SettingsStore.shared.overlayCustomOriginScreenFrames = Self.currentScreenFrames() | ||
| SettingsStore.shared.overlayCustomOrigin = origin |
There was a problem hiding this comment.
Although individual screen frames are now persisted, they are sampled when the delayed work item executes rather than when origin is captured. If a display is disconnected or its configuration changes during this 200 ms window, the old origin is paired with the new arrangement; arrangementsMatch then accepts that arrangement unconditionally and can restore the overlay to the now-disconnected display instead of recovering it to the anchored position. Capture currentScreenFrames() alongside origin before scheduling the work item.
Useful? React with 👍 / 👎.
Two follow-ups from the second review pass. **A restored backup did not move the overlay.** Once the overlay had been dragged in the current process, the in-memory origin always outranked settings, so importing a backup with a different position wrote UserDefaults and left the overlay where it was — for the rest of the process. The origin-changed observer now takes the stored value as the new truth rather than only clearing it on reset. A drag in flight is unaffected: the notification is posted by the coalesced write itself, which stores exactly what is already in memory. **The arrangement was sampled 200 ms late.** Screen frames were read when the debounced write ran, not when the drag happened, so a display unplugged inside that window paired the old origin with the new arrangement. That reads as a match, which is the one case that skips the reachability check — precisely the stranding this is meant to prevent. Both are captured together now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Refresh the live origin when restoring settings — once the overlay had been dragged in the current process, the in-memory origin outranked settings for the rest of that process, so importing a backup wrote UserDefaults and left the overlay where it was. The origin-changed observer now takes the stored value as the new truth instead of only clearing it on reset. A drag in flight is unaffected, since the notification that reaches the observer is posted by the coalesced write itself, which stores exactly what is already in memory. Capture screen frames before debouncing the save — worth calling out as the sharper of the two, because it lands on the one path that skips the reachability check. Frames were read when the write ran rather than when the drag happened, so a display unplugged inside the 200 ms window paired the old origin with the new arrangement, Builds clean, lint unchanged. Greptile came back 5/5 on Unchanged and still worth a maintainer's eye rather than a bot's: this covers the bottom overlay only — the notch overlay is a separate controller and deliberately untouched — and I have not tested on Intel or run the test suite. Happy to move this to a Discussion per CONTRIBUTING if tying it to the closed #284 is not the route you want. 🤖 Generated with Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd7f203750
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| NotificationCenter.default.addObserver(forName: NSNotification.Name("OverlayCustomOriginChanged"), object: nil, queue: .main) { [weak self] _ in | ||
| Task { @MainActor [weak self] in |
There was a problem hiding this comment.
When Reset Position or a backup restore changes the origin during the 200 ms persistence window, this notification handler defers cancellation to an unstructured Task. The already-queued pendingOriginSave can therefore run first and write the dragged origin back, undoing the reset or restored value. The fresh evidence is that the new handler performs the intended cancellation only inside this asynchronously scheduled task; handle the main-queue notification synchronously, as the drag observer does, before returning to the run loop.
Useful? React with 👍 / 👎.
The origin-changed observer treated every notification as external, including the one posted by its own coalesced write. Because the observer runs a turn later, a drag that resumed in that gap had already installed a newer save, which the observer then cancelled while rolling the in-memory origin back to the value just stored — losing the end of the drag, or snapping the panel backwards. The controller now claims the single notification its own write posts and the observer consumes one per write, so only a genuinely external change — Reset Position, a restored backup — cancels a pending save. Nothing is inferred from state the pointer may already have moved past. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The hole was that the observer treated every notification as external, including the one its own coalesced write posts. Running a turn later, it could find a newer save already installed by a drag that had resumed, cancel it, and roll the in-memory origin back to the value just stored. The controller now claims the single notification its own write posts, and the observer consumes one per write. Only a genuinely external change — Reset Position, a restored backup — cancels a pending save. Nothing is inferred from state the pointer may already have moved past, which was the actual defect in the previous shape rather than the width of the window. 🤖 Generated with Claude Code |
The origin-changed handler deferred its work into an unstructured `Task`, so a save scheduled for the tail of the 200 ms debounce window could run before the cancellation did and write the dragged origin back over a Reset Position or a restored backup — the failure the cancellation exists to prevent. It now runs on the run loop turn that posted the notification, the way the drag observer already does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The handler deferred its work into an unstructured The self-posted notification count stays. It is what makes "my own write" and "someone else's write" distinguishable rather than inferred from state, and it does not depend on the handler's timing. Build and lint unchanged. 🤖 Generated with Claude Code |
|
Correction to something I have now repeated in four comments on this PR: "I have not run the test suite" was a bad disclaimer. I have run it.
The remaining caveat is real and unchanged: still untested on Intel. The 🤖 Generated with Claude Code |
The previous commit told the controller's own coalesced write apart from an external one with a counter of notifications it was about to post. That is the wrong shape for the problem: an unbounded `Int` that assumes exactly one post per write and one delivery per post, forever, whose drift would be silent and whose symptom would be Reset Position quietly doing nothing. `OverlayCustomOriginChanged` has exactly one poster and one observer, and it exists solely to tell the controller a position was changed from somewhere else. So the controller's own write should never post it. Drag persistence now goes through `SettingsStore.storeDraggedOverlayOrigin(_:screenFrames:)`, which stores the origin and the arrangement together and deliberately posts nothing — the overlay is by definition already at that position. `objectWillChange` is still sent, so the Reset Position row appears on the first drag. The observer then needs no branch and no state: every notification it receives is external, so it always cancels a pending save and takes the stored value. Net one member and one code path fewer than before the race existed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd03bee54d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if let customOrigin = self.liveCustomOrigin ?? SettingsStore.shared.overlayCustomOrigin, | ||
| Self.storedOriginIsUsable(customOrigin, size: window.frame.size) |
There was a problem hiding this comment.
When the first continuous drag moves the overlay completely beyond every screen edge, customOrigin comes from liveCustomOrigin, but storedOriginIsUsable still validates it using the persisted screen frames, which are empty or stale until the 200 ms save executes. If transcription changes the preview size during that interval, positionWindow() rejects the live origin and snaps the panel back to its anchored position while the pointer is still dragging. The fresh evidence after the earlier live-origin fix is that this new validation path still reads the old arrangement from SettingsStore; retain the current arrangement alongside the live origin or bypass stored-origin recovery checks for an active live drag.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
f101c32. The fix is right; the reasoning attached to it needed one correction.
storedOriginIsUsable read SettingsStore.overlayCustomOriginScreenFrames no matter whose origin it was handed, which for a live origin is the arrangement before the one it was chosen on — or none at all on a first drag, since the matching write is still in the 200 ms debounce. The origin now travels with its arrangement, originIsUsable takes both and reads no global state, and the same-arrangement rule applies to a live origin from the moment it is set rather than 200 ms later.
Where I would push back is the failure it is hung on. A drag alone cannot reach it: the grab point stays under the pointer, the pointer is confined to display space, so the panel always intersects an attached screen and screens.contains { $0.intersects(overlay) } holds even against an empty stored arrangement. Reaching the snap-back needs the preview to shrink mid-drag far enough that what is left of the panel clears the edge — dictation ending while the panel is held mostly past the left edge. Narrow. The inconsistency underneath it was real on its own terms, though: a known-current position was being judged against a known-stale arrangement, and that is worth removing whether or not a user can walk into it.
Build clean, 437 tests, 0 failures.
`storedOriginIsUsable` read `SettingsStore.overlayCustomOriginScreenFrames` regardless of which origin it was handed. For a live origin that is the wrong arrangement: the matching write is still sitting in the 200 ms debounce, so settings holds the previous arrangement, or none at all on the first drag. The origin now travels with the arrangement it was chosen on. `originIsUsable` takes both and reads no global state, so the same-arrangement rule — a position dragged past a screen edge is honoured exactly — applies to a live origin from the moment it is set, not 200 ms later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Description
The bottom recording overlay is anchored to the centre of the screen edge, so it sits wherever the layout puts it — which is sometimes directly on top of the thing you are dictating into. Today the only controls are Overlay Position (top or bottom) and Bottom Offset (vertical only), so an overlay covering a text field, a toolbar or a send button cannot be moved aside.
This makes the overlay draggable and remembers where you put it.
BottomOverlayPanelgainsallowsUserDragging, which bypasses AppKit's keep-on-screen clamping for user-driven moves. The panel already had this hook for offscreen parking; this reuses it so the overlay can be pushed past a screen edge and fully out of the way.isMovableByWindowBackgroundis enabled on the panel. Clicks on the pill's own controls (prompt picker, mode menu, actions menu) still reach them — only the inert background begins a drag.OverlayCustomOrigin.didMovefires on every step of a drag, so the write is coalesced and only the final position is stored. AnisApplyingProgrammaticFrameguard stops the app's own repositioning from being recorded as a user drag.positionWindow()honours a stored origin instead of re-centring. Without this the dragged position was overwritten on every presentation and on every hover-driven resize.That last point is not cosmetic: once the overlay is dragged mostly off screen, the grab area is off screen too, so there is no way to drag it back. Reset Position is the way out.
Type of Change
Related Issue or Discussion
Relates to #284 ("Free-Floating Overlay, Tiny Size, and Visibility on the Active Screen"). That issue asked for an overlay that "behaves like the bottom bar stack but is draggable (sensible default position, persisted position)" and is closed as completed — the pill and sizing landed, but the draggable/persisted part did not, which is what this PR adds. It deliberately does not touch the other two requests in that issue (tiny visualiser-only size, active-screen following).
Happy to move this to a Discussion in Ideas first if you would prefer that per CONTRIBUTING — I opened the PR because the change is small and already implemented, not to skip the process.
Testing
swiftlint --strict --config .swiftlint.ymlon the three changed files — 0 violationsswiftformat --lint --config .swiftformat— no changes to any added lineVerified by hand on a real build, measuring the panel's frame via
CGWindowListCopyWindowInfoat each step:OverlayCustomOriginwritten once, at the resting positionAlso checked in an isolated harness reproducing the same panel setup (borderless non-activating
NSPanel+NSHostingView+ a SwiftUIButton):isMovableByWindowBackgrounddoes survive the hosting view, a drag past the left screen edge reaches a negative origin,didMovefires, and a click on the embedded button still registers without moving the window.Two caveats I'd rather state than hide: I could not test on an Intel Mac, and I did not run the test suite locally — only the linter, the formatter and the manual verification above.
Screenshots / Video
Captured against a neutral backdrop so nothing unrelated is in frame.
1. Default — anchored bottom-centre
2. Dragged into the middle of the screen
3. Dragged up against the top edge
4. Reopened after stopping and restarting dictation — still where it was dragged
Notes
OverlayCustomOrigin, positioning is exactly as before, so existing users see no difference until they drag.OverlayCustomPositionRowview rather than added inline, because inlining pushedSettingsViewpast thetype_body_lengthlimit of 2000 lines.Bottom Offsetstill applies while no custom position is set; the new row explains that resetting restores it.legacy_swiftui_aspect_ratioviolation inBottomOverlayView.swiftis present onmainas well and is not touched here.🤖 Generated with Claude Code