Keep quick-captured thoughts near the active board context - #128
Conversation
📝 WalkthroughWalkthroughQuick capture now places cards near active or visible board context, avoids occupied positions, reveals captured cards, and restores entry focus after activation. Live editor frames trigger a deferred reveal correction. ChangesQuick capture context
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ComposerCanvas
participant BoardViewModel
participant CanvasWorkspaceSession
participant PanelController
User->>ComposerCanvas: trigger quick capture
ComposerCanvas->>BoardViewModel: captureExternalText(text, viewport anchor)
BoardViewModel-->>ComposerCanvas: return captured card
ComposerCanvas->>CanvasWorkspaceSession: revealCard(captured card)
CanvasWorkspaceSession-->>ComposerCanvas: update retained pan
BoardViewModel-->>ComposerCanvas: post live frame update
ComposerCanvas->>CanvasWorkspaceSession: reveal updated frame
PanelController-->>ComposerCanvas: post composerEnterEditing after activation
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
the
vouch:trusted
![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: 60d823de57
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @discardableResult | ||
| func revealCard(_ cardID: UUID, in viewportSize: CGSize, margin: CGFloat = 48) -> Bool { | ||
| guard viewportSize.width > 0, viewportSize.height > 0, | ||
| let frame = board.cards.first(where: { $0.id == cardID })?.frame else { return false } |
There was a problem hiding this comment.
When an active text card has grown during the current edit, its current geometry exists only in BoardViewModel.liveTextFrames until editing ends, while this lookup reads the stale published cards frame. Refocusing the window can therefore pan only the card's old bounds into view and leave its expanded content or caret offscreen; expose and use the live frame for reveal calculations.
Useful? React with 👍 / 👎.
| DispatchQueue.main.async { | ||
| NotificationCenter.default.post(name: .composerEnterEditing, object: nil) |
There was a problem hiding this comment.
The board also becomes key when one of its own attached sheets ends, such as the NSSavePanel in BoardExporter or the NSOpenPanel in CanvasAgent. This unconditional post then makes enterEditingForEntry call beginEditing for a selected text card, so completing or canceling those sheets unexpectedly opens the inline editor and changes focus; restrict this notification to an actual board summon/refocus rather than every key-window transition.
Useful? React with 👍 / 👎.
| let id = board.editingCardID ?? board.primarySelectedCardID ?? board.cards.first?.id | ||
| guard let id, board.cards.first(where: { $0.id == id })?.elementKind == .text else { return } | ||
| board.beginEditing(id) | ||
| if board.editingCardID != id { board.beginEditing(id) } |
There was a problem hiding this comment.
When the board regains key status with a selected text card that is not already being edited, this only flips editingCardID; the newly mounted FreeWriteEditor never receives controller.focus(). Unlike the double-click and palette-return paths, there is no deferred focus call here, and PanelController.focusEditor is not run for an ordinary key-window refocus, so the card appears to enter editing while subsequent typing still goes to the previous first responder.
Useful? React with 👍 / 👎.
| /// ready to type. A capture remains the primary/editing card, so reopening the board returns to | ||
| /// that thought instead of preserving a viewport that can no longer see it. | ||
| private func enterEditingForEntry(reveal: Bool) { | ||
| guard !store.isSettingsOpen, store.compiledDraft == nil, !store.isHistoryOpen else { return } |
There was a problem hiding this comment.
If the user switches away and back while the Focus Write sheet is open, focusedCardID remains set but this guard allows the key-window notification to call beginEditing for the same selected text card. That mounts a second inline FreeWriteEditor behind the sheet even though toggleFocus explicitly ended inline editing to maintain a single editor; both instances then share the interaction and can overwrite its controller/coordinator, breaking focus and edit actions. Return early when focusedCardID is non-nil.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/ComposerApp/Panel/PanelController.swift">
<violation number="1" location="Sources/ComposerApp/Panel/PanelController.swift:143">
P2: `windowDidBecomeKey` posts `.composerEnterEditing` on every refocus of the already-visible board, not just quick captures. `enterEditingForEntry` guards only `isSettingsOpen`/`compiledDraft`/`isHistoryOpen` — not the Agent overlay (`showAgent`) or the command palette — so returning to the app while the Agent chat is open will `beginEditing` a board text card and re-pan the viewport, likely stealing first responder from the Agent's input. Consider gating this refocus path on the same conditions (e.g. also skip when the Agent overlay is active) or scoping it to the capture flow.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // Refocusing an already-visible board must restore the active card just like summoning a hidden | ||
| // one. Defer one turn so AppKit can finish handing the window its first responder first. | ||
| DispatchQueue.main.async { | ||
| NotificationCenter.default.post(name: .composerEnterEditing, object: nil) |
There was a problem hiding this comment.
P2: windowDidBecomeKey posts .composerEnterEditing on every refocus of the already-visible board, not just quick captures. enterEditingForEntry guards only isSettingsOpen/compiledDraft/isHistoryOpen — not the Agent overlay (showAgent) or the command palette — so returning to the app while the Agent chat is open will beginEditing a board text card and re-pan the viewport, likely stealing first responder from the Agent's input. Consider gating this refocus path on the same conditions (e.g. also skip when the Agent overlay is active) or scoping it to the capture flow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/ComposerApp/Panel/PanelController.swift, line 143:
<comment>`windowDidBecomeKey` posts `.composerEnterEditing` on every refocus of the already-visible board, not just quick captures. `enterEditingForEntry` guards only `isSettingsOpen`/`compiledDraft`/`isHistoryOpen` — not the Agent overlay (`showAgent`) or the command palette — so returning to the app while the Agent chat is open will `beginEditing` a board text card and re-pan the viewport, likely stealing first responder from the Agent's input. Consider gating this refocus path on the same conditions (e.g. also skip when the Agent overlay is active) or scoping it to the capture flow.</comment>
<file context>
@@ -134,7 +134,15 @@ final class PanelController: NSObject, NSWindowDelegate {
+ // Refocusing an already-visible board must restore the active card just like summoning a hidden
+ // one. Defer one turn so AppKit can finish handing the window its first responder first.
+ DispatchQueue.main.async {
+ NotificationCenter.default.post(name: .composerEnterEditing, object: nil)
+ }
+ }
</file context>
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ComposerApp/Views/ComposerCanvas.swift`:
- Line 345: Update the Toast call in ComposerCanvas to use the semantic
Theme.Palette.accent token for its tint instead of .accentColor, preserving the
existing message and symbol.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7dbc303d-0b46-4c46-af96-a946ff4c4dbf
📒 Files selected for processing (8)
CHANGELOG.mdSources/ComposerApp/App/AppDelegate.swiftSources/ComposerApp/Panel/CanvasWorkspaceSession.swiftSources/ComposerApp/Panel/PanelController.swiftSources/ComposerApp/Support/Notifications.swiftSources/ComposerApp/Views/BoardViewModel.swiftSources/ComposerApp/Views/ComposerCanvas.swiftTests/ComposerAppTests/QuickCaptureTests.swift
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/ComposerApp/Views/ComposerCanvas.swift (1)
340-342: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude the transient pan in the viewport anchor.
BoardCardLayerrenders withpan + panLive, butboardPoint(forViewport:)subtracts onlypan. If a capture arrives during a pan gesture, thearoundpoint is offset from the visible viewport center.Include
panLivein this conversion. Use a default.zeroparameter so existing call sites retain their current behavior.🤖 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/ComposerApp/Views/ComposerCanvas.swift` around lines 340 - 342, Update boardPoint(forViewport:) to accept a panLive parameter defaulting to .zero, and subtract both pan and panLive when converting the viewport anchor. Pass the current transient pan from the captureExternalText call in the surrounding capture flow, while preserving existing behavior for call sites that omit it.Sources/ComposerApp/Views/BoardViewModel.swift (1)
1581-1598: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFinalize the existing edit before switching to the captured card.
If
editingCardIDalready identifies another card, this path callsbeginEditing(id)for the new card without finalizing the previous card.beginEditingonly changes selection and editing state. The previous card'sliveTextFramesentry can remain stale, and rejected callbacks can no longer commit it. A blank previous text card also bypasses abandonment cleanup.Finalize the current edit before
insertTextandbeginEditing.Proposed fix
let activeID = editingCardID ?? primarySelectedCardID - let activeFrame = activeID.flatMap { id in + let preCommitFrame = activeID.flatMap { id in liveTextFrames[id] ?? cards.first(where: { $0.id == id })?.frame } + if editingCardID != nil { stopEditing() } + let activeFrame = activeID.flatMap { renderingFrame(for: $0) } ?? preCommitFrameAdd a regression test that captures while another card has a live editor frame.
🤖 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/ComposerApp/Views/BoardViewModel.swift` around lines 1581 - 1598, Finalize any existing edit identified by editingCardID before calculating the insertion point or calling insertText, so the prior card’s live editor frame is committed or abandoned through the normal cleanup path before beginEditing(id) switches editing state. Preserve the current insertion and selection behavior, and add a regression test covering capture while another card has a live editor frame.
🤖 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.
Outside diff comments:
In `@Sources/ComposerApp/Views/BoardViewModel.swift`:
- Around line 1581-1598: Finalize any existing edit identified by editingCardID
before calculating the insertion point or calling insertText, so the prior
card’s live editor frame is committed or abandoned through the normal cleanup
path before beginEditing(id) switches editing state. Preserve the current
insertion and selection behavior, and add a regression test covering capture
while another card has a live editor frame.
In `@Sources/ComposerApp/Views/ComposerCanvas.swift`:
- Around line 340-342: Update boardPoint(forViewport:) to accept a panLive
parameter defaulting to .zero, and subtract both pan and panLive when converting
the viewport anchor. Pass the current transient pan from the captureExternalText
call in the surrounding capture flow, while preserving existing behavior for
call sites that omit it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d7fb9dbf-a906-4831-81f0-78bd49357e51
📒 Files selected for processing (3)
Sources/ComposerApp/Views/BoardViewModel.swiftSources/ComposerApp/Views/ComposerCanvas.swiftTests/ComposerAppTests/QuickCaptureTests.swift
1 participant
Summary
Validation
swiftc -frontend -parsefor all changed Swift sources and testsgit diff --checkCloses #108
Note
Place quick-captured text near the active card or visible board area
autoPlacePoint(for:near:)in BoardViewModel.swift.composerTextCardLiveFrameChangednotification.composerEnterEditing) is now debounced and conditioned on window visibility, and is also restored when the app regains focus viaapplicationDidBecomeActivein AppDelegate.swift.captureExternalTextaccepts a viewport center point as a placement hint and returns the new card's ID so the canvas can track and reveal it.Macroscope summarized e609982.
Summary
git diff --check.