Refine onboarding follow-up flow#6041
Conversation
Greptile SummaryThis PR refines the second half of the desktop onboarding flow: it splits the combined voice-shortcut+demo step into two focused steps, adds a global key monitor to the floating-bar shortcut step so the hotkey is caught from any focused app, starts screen monitoring as soon as screen-recording permission is granted, and harmonises all accent colours from purple to white/neutral. Three issues are worth addressing before merging:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant OV as OnboardingView
participant FS as FileScanStep
participant OPC as OnboardingPagedIntroCoordinator
participant BIS as BackgroundInsights(Gmail/Cal)
participant VSS as VoiceShortcutStep
participant VD as VoiceDemoView
participant FBS as FloatingBarShortcutStep
participant FBD as FloatingBarDemoView
OV->>FS: show (step ~8)
User->>FS: grants file access
FS->>OPC: startScan()
OPC->>BIS: Task { startBackgroundInsightsIfNeeded() }
Note over OPC,BIS: NEW: insights start in parallel with scan
OPC->>OPC: executeTool("scan_files")
BIS-->>OPC: buildSuggestedGoals(scanSnapshot?) [may be nil]
OPC-->>FS: scanState = .complete
OV->>FBS: show (floating bar shortcut step)
FBS->>FBS: installLocalKeyMonitor + installGlobalKeyMonitor
User->>FBS: presses shortcut (from any app)
FBS-->>FBS: shortcutDetected = true → show Continue
OV->>FBD: show (floating bar demo)
User->>FBD: opens bar, types question, sees response
OV->>VSS: show (PTT shortcut step)
VSS->>VSS: hide floating bar, cleanup PTT, installLocalKeyMonitor
User->>VSS: presses modifier key (must be focused)
VSS-->>VSS: shortcutDetected = true → show Continue
OV->>VD: show (voice demo step)
VD->>VD: resetBarState, set .live PTT, warmupBridge()
User->>VD: holds PTT key, speaks
VD->>VD: poll 0.25s×80 for AI response
VD-->>OV: showContinue → onComplete
OV->>OV: screen recording granted → startMonitoringIfNeeded()
Note over OV: NEW: monitoring starts earlier, not just at onboarding end
Reviews (1): Last reviewed commit: "Refine onboarding follow-up flow" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if !chatProvider.isSending, | ||
| observedShortcutPress, | ||
| (chatProvider.errorMessage != nil || barState.currentAIMessage != nil) { | ||
| showContinueNow() | ||
| return | ||
| } |
There was a problem hiding this comment.
chatProvider.errorMessage may prematurely unlock Continue
resetFloatingBarConversation() resets barState properties but does not clear chatProvider.errorMessage. If the user arrives at this step with a non-nil errorMessage left over from a previous interaction, the second branch of the poll can fire during the very first iteration (right after the PTT key is released and isSending returns to false), showing the Continue button before Omi has had a chance to respond.
The safest fix is to reset the error when the view appears:
.onAppear {
FloatingControlBarManager.shared.setup(appState: appState, chatProvider: chatProvider)
resetFloatingBarConversation()
chatProvider.errorMessage = nil // ← clear any stale error
...
}Alternatively, tighten the poll condition so it only fires as a fallback when barState.currentAIMessage transitions from nil to non-nil after the send has started, rather than on any pre-existing non-nil error.
| private func installKeyMonitor() { | ||
| keyMonitor = NSEvent.addLocalMonitorForEvents(matching: .flagsChanged) { event in | ||
| guard !shortcutDetected else { return event } | ||
| guard matchesCurrentPTTShortcut(event) else { return event } | ||
|
|
||
| shortcutDetected = true | ||
| withAnimation(.easeInOut(duration: 0.3)) { | ||
| showContinue = true | ||
| } | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
installKeyMonitor registers only a local flagsChanged monitor, which only fires when the onboarding window is the key event target. OnboardingFloatingBarShortcutStepView was explicitly updated in this same PR to add both a local and a global monitor precisely to handle the case where focus moves to another app before the shortcut is pressed. The same scenario applies here: if the user clicks into another window (common when they're trying the shortcut while a different app is visible) the key press will not be detected and the step will stay stuck until they click back into the onboarding window.
Consider adding a paired global monitor following the same pattern used in OnboardingFloatingBarShortcutStepView:
@State private var globalKeyMonitor: Any?
private func installKeyMonitor() {
localKeyMonitor = NSEvent.addLocalMonitorForEvents(matching: .flagsChanged) { event in
if handleShortcutEvent(event) { return nil }
return event
}
globalKeyMonitor = NSEvent.addGlobalMonitorForEvents(matching: .flagsChanged) { event in
_ = handleShortcutEvent(event)
}
}And remove both on onDisappear.
| scanState = .scanning | ||
| scanStatusText = "Scanning your projects and apps..." | ||
|
|
||
| Task { await startBackgroundInsightsIfNeeded() } |
There was a problem hiding this comment.
buildSuggestedGoals may be called with a nil scanSnapshot
startBackgroundInsightsIfNeeded() is now fired as a detached Task before executeTool("scan_files") runs. Inside that function the Gmail and Calendar tasks both call buildSuggestedGoals(from: self.scanSnapshot, ...) when they complete. If either task finishes before the file scan populates scanSnapshot, the goals are built without any file-context data, producing less relevant suggestions.
Previously the call was placed after refreshSnapshotIfAvailable(), guaranteeing scanSnapshot was populated first. If the intent is specifically to parallelise the network requests, consider buffering the goal build: hold off calling buildSuggestedGoals until both the insight data and scanSnapshot are available, or at minimum call it again inside refreshSnapshotIfAvailable() once the snapshot is ready.
| private func resetFloatingBarConversation() { | ||
| guard let barState = FloatingControlBarManager.shared.barState else { return } | ||
| barState.showingAIConversation = false | ||
| barState.showingAIResponse = false | ||
| barState.aiInputText = "" | ||
| barState.currentAIMessage = nil | ||
| barState.chatHistory = [] | ||
| barState.isVoiceFollowUp = false | ||
| barState.voiceFollowUpTranscript = "" | ||
| } |
There was a problem hiding this comment.
resetFloatingBarConversation() duplicated across two views
This method is defined identically in both OnboardingVoiceDemoView (lines 164-173) and OnboardingVoiceShortcutStepView (lines 119-128). Future changes to the reset logic need to be applied in two places, which is easy to miss. Consider extracting it to a shared extension on FloatingControlBarManager or a free function in an OnboardingHelpers.swift file.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Verification
Notes