Skip to content

Update floating bar onboarding Mac lineup#5610

Merged
kodjima33 merged 1 commit intomainfrom
feat/floating-bar-onboarding-mac-lineup
Mar 13, 2026
Merged

Update floating bar onboarding Mac lineup#5610
kodjima33 merged 1 commit intomainfrom
feat/floating-bar-onboarding-mac-lineup

Conversation

@kodjima33
Copy link
Collaborator

Summary

  • replace the floating bar onboarding illustration with the provided Mac lineup screenshot
  • update the prompt copy to ask which computer suits the user best
  • use the normal global shortcut flow during onboarding and add ACP bridge path fallbacks for dev packaging
  • document that desktop build verification should include launching and checking the installed dev app

Verification

  • swift build
  • rebuilt, reinstalled, and relaunched /Applications/Omi Dev.app during testing

@kodjima33 kodjima33 merged commit dfd10c7 into main Mar 13, 2026
2 checks passed
@kodjima33 kodjima33 deleted the feat/floating-bar-onboarding-mac-lineup branch March 13, 2026 23:24
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 13, 2026

Greptile Summary

This PR refreshes the floating bar onboarding step by replacing the animated icon illustration with a MacLineupPreview image, updating the copy to invite the user to ask which Mac suits them, and switching from a local NSEvent key monitor to the production global shortcut path. It also expands the ACPBridge script-discovery fallbacks for dev environments, and adds documentation reminding agents to launch the installed app after a successful compile.

Key changes:

  • OnboardingFloatingBarDemoView: animated icon + local key monitor removed; replaced with MacLineupPreview and a 0.25 s timer that polls showingAIConversation to detect bar activation.
  • MacLineupPreview: static NSImage loaded synchronously from the resource bundle — see inline comment about blocking the main thread on first render.
  • GlobalShortcutManager.registerShortcuts() is now called in onAppear (previously shortcuts were unregistered during this step); the shortcut handler calls toggleAIInput(), which can close the bar if it is already visible, potentially locking the user in a 60-second wait for Continue (see inline comment).
  • ACPBridge.findBridgeScript(): adds two extra CWD-relative candidates (desktop/acp-bridge/… and ../desktop/acp-bridge/…) for repo-root and parent-directory dev layouts; all paths fail gracefully.
  • AGENTS.md / CLAUDE.md: doc-only: agents must launch and interact with the dev app, not just compile.

Confidence Score: 3/5

  • Safe to merge after addressing the toggleAIInput regression; the remaining issues are performance/style.
  • The visual and copy changes are straightforward. The ACPBridge fallback expansion is defensive and correct. However, there is one concrete behavior regression: the global shortcut now calls toggleAIInput() instead of the old openAIInput(), meaning a second ⌘+Enter press (or any press while the AI panel is already open) closes the panel and causes a 60-second wait before Continue appears. There is also a main-thread I/O stall risk from the synchronous NSImage(contentsOf:) in the static initializer, and a minor resource waste from the timer not being cancelled after activation.
  • desktop/Desktop/Sources/OnboardingFloatingBarDemoView.swift — contains the toggle regression and the synchronous image load.

Important Files Changed

Filename Overview
desktop/Desktop/Sources/OnboardingFloatingBarDemoView.swift Replaces animated icon + local key monitor with a Mac lineup image + global shortcut + timer polling; introduces a toggle-vs-open regression and a synchronous image load on the main thread.
desktop/Desktop/Sources/Chat/ACPBridge.swift Expands CWD-relative bridge script search paths from one to three candidates to cover repo-root and parent-of-repo layouts; all paths degrade gracefully by returning nil if not found.
desktop/Desktop/Sources/Resources/onboarding_mac_lineup.png New binary asset added to Sources/Resources/, which is already covered by the .process("Resources") rule in Package.swift — no build config change needed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[View onAppear] --> B[FloatingControlBarManager.setup]
    B --> C[GlobalShortcutManager.registerShortcuts]
    C --> D[0.25s Timer starts polling showingAIConversation]

    D -->|showingAIConversation == true| E[barActivated = true]
    D -->|showingAIConversation == false| D

    E --> F[waitForResponse task starts]
    F -->|polls every 0.5s up to 60s| G{showingAIResponse && !isStreaming?}
    G -->|yes| H[showContinue = true → Continue button shown]
    G -->|no, timeout| H

    subgraph GlobalShortcut [Global Shortcut: ⌘+Enter]
        I[toggleAIInput] -->|showingAIConversation == false| J[openAIInput → showingAIConversation = true]
        I -->|showingAIConversation == true| K[closeAIConversation ⚠️ regression]
    end

    UserPress[User presses ⌘+Enter] --> I
    J --> D
    K -->|barActivated already true| F
Loading

Last reviewed commit: a774dbc

Comment on lines +179 to +182
private static let lineupImage: NSImage? = {
guard let url = Bundle.resourceBundle.url(forResource: "onboarding_mac_lineup", withExtension: "png") else { return nil }
return NSImage(contentsOf: url)
}()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synchronous image loading blocks the main thread

NSImage(contentsOf:) performs synchronous file I/O. Because this is stored in a static let, the initializer runs lazily — but it runs on the main thread the very first time MacLineupPreview appears (during a SwiftUI body evaluation). A Mac lineup screenshot can easily be several MB, so the first render of this onboarding step can freeze the UI noticeably.

Consider loading the image off-thread and publishing it via a @State or @StateObject:

private struct MacLineupPreview: View {
    @State private var nsImage: NSImage?

    var body: some View {
        Group {
            if let nsImage {
                Image(nsImage: nsImage)
                    .resizable()
                    .interpolation(.high)
                    .scaledToFit()
                    .clipShape(RoundedRectangle(cornerRadius: 24, style: .continuous))
            } else {
                RoundedRectangle(cornerRadius: 24)
                    .fill(Color.white.opacity(0.06))
                    .frame(height: 280)
                    .overlay(
                        Text("Mac lineup image unavailable")
                            .font(.system(size: 14, weight: .medium))
                            .foregroundColor(OmiColors.textTertiary)
                    )
            }
        }
        .task {
            guard let url = Bundle.resourceBundle.url(
                forResource: "onboarding_mac_lineup", withExtension: "png") else { return }
            nsImage = await Task.detached(priority: .userInitiated) {
                NSImage(contentsOf: url)
            }.value
        }
    }
}

Comment on lines +125 to +131
.onReceive(Timer.publish(every: 0.25, on: .main, in: .common).autoconnect()) { _ in
guard !barActivated,
FloatingControlBarManager.shared.barState?.showingAIConversation == true else { return }
withAnimation(.spring(response: 0.4, dampingFraction: 0.8)) {
barActivated = true
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timer fires indefinitely after barActivated becomes true

Once barActivated is set to true, the guard exits early on every subsequent tick, but the timer is never cancelled — it keeps firing at 4 Hz for the entire lifetime of the view (which includes the 60-second waitForResponse loop). Storing the timer's Cancellable and cancelling it when activation is detected would avoid this waste:

// @State private var timerCancellable: AnyCancellable?   (declared at top of struct)

.onReceive(Timer.publish(every: 0.25, on: .main, in: .common).autoconnect()) { _ in
    guard !barActivated,
          FloatingControlBarManager.shared.barState?.showingAIConversation == true else { return }
    withAnimation(.spring(response: 0.4, dampingFraction: 0.8)) {
        barActivated = true
    }
    timerCancellable?.cancel()
}

Alternatively, using a dedicated Cancellable stored in a @State var gives explicit control over when polling stops.

Comment on lines +125 to +131
.onReceive(Timer.publish(every: 0.25, on: .main, in: .common).autoconnect()) { _ in
guard !barActivated,
FloatingControlBarManager.shared.barState?.showingAIConversation == true else { return }
withAnimation(.spring(response: 0.4, dampingFraction: 0.8)) {
barActivated = true
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toggleAIInput can close the bar mid-interaction, causing a 60 s stall

The global shortcut now calls toggleAIInput() (via GlobalShortcutManager), which closes the AI panel if it is already visible. In the old code the key monitor always called openAIInput() (open-only). This creates a regression:

  1. User presses ⌘+Enter → showingAIConversation = true → timer sets barActivated = truewaitForResponse() starts.
  2. User types a question and the AI begins streaming.
  3. User accidentally presses ⌘+Enter again → toggleAIInput() calls closeAIConversation(), showingAIConversation drops to false.
  4. waitForResponse() polls barState.showingAIResponse — which is now false — for the full 60-second timeout before showing Continue.

The onboarding intent is "open the bar," not "toggle the bar." Consider using FloatingControlBarManager.shared.openAIInput() (which is idempotent when already open) instead of routing through the global shortcut machinery, or subscribe to a notification that the shortcut fired and always call openAIInput().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant