Skip to content

Let users toggle popup-card suggestion display from Settings and the menu#351

Merged
FuJacob merged 1 commit into
mainfrom
feat/mirror-overlay-user-toggle
May 28, 2026
Merged

Let users toggle popup-card suggestion display from Settings and the menu#351
FuJacob merged 1 commit into
mainfrom
feat/mirror-overlay-user-toggle

Conversation

@FuJacob
Copy link
Copy Markdown
Owner

@FuJacob FuJacob commented May 28, 2026

Summary

Builds on #347. That PR triggered the popup-card render mode automatically when caret geometry came back as .estimated. This PR adds a persistent global preference (Auto / Inline / Popup) so users can pin one style for every app, and surfaces the picker both in the Settings General section and the menu bar pop-up so it's reachable without opening Settings.

Validation

xcodegen generate                                  # no drift
swiftlint lint --quiet                             # exit 0
xcodebuild -project Cotabby.xcodeproj -scheme Cotabby \
  -destination 'platform=macOS' build              # ** BUILD SUCCEEDED **  (no warnings)
xcodebuild -project Cotabby.xcodeproj -scheme Cotabby \
  -destination 'platform=macOS' build-for-testing  # ** TEST BUILD SUCCEEDED **

Two new tests cover persistence (test_mirrorPreference_defaultsToAutoAndPersists) and the snapshot publisher (test_snapshotPublisher_emitsWhenMirrorPreferenceChanges). All existing snapshot/fixture call sites pick up the new field through a defaulted argument.

Local xcodebuild test execution hits the known Team ID code-signing mismatch on this machine (called out in CLAUDE.md). Tests compile correctly and need CI signing or Xcode UI to execute.

Risk / rollout notes

  • Default is .auto, so existing users see identical behavior to Add mirror overlay fallback for unreliable caret geometry #347. Popup card still only appears for .estimated caret geometry unless the user pins .alwaysMirror.
  • New mirrorPreference field on SuggestionSettingsSnapshot. Construction is centralized in two places (the model's snapshot getter and snapshotPublisher chain) and the shared test fixture; all sites updated.
  • snapshotPublisher's contextToggles group went from CombineLatest to CombineLatest3 so the inner combine stack stays under Combine's 4-upstream cap; renamed to presentationToggles to reflect the wider scope.
  • OverlayController now reads the preference live per showSuggestion call via a computed currentRenderModePolicy. Tests can still inject a fixed policy via the renamed renderModePolicyOverride: init parameter.
  • Per-app overrides are intentionally still out of scope; the setCurrentBundleIdentifier plumbing stays dormant for a follow-up phase.

Linked issues

Refs #347.

Greptile Summary

Adds a persistent MirrorPreference setting (Auto / Inline / Popup) that lets users pin a suggestion display style globally, surfaced in both Settings and the menu bar. The default is .auto, preserving existing behavior for all current users.

  • SuggestionSettingsModel stores and persists the preference via UserDefaults, threads it through snapshot and snapshotPublisher, and upgrades the inner CombineLatest to CombineLatest3 to stay within Combine's 4-upstream cap.
  • OverlayController switches from a cached policy to a computed currentRenderModePolicy that reads the live setting on each showSuggestion call, so changes take effect immediately without any subscription bookkeeping.
  • Two new tests cover default-value persistence and snapshot-publisher emission; the shared test fixture gains a defaulted mirrorPreference parameter so all existing call sites compile unchanged.

Confidence Score: 4/5

Safe to merge; the default is .auto so existing users are unaffected, and the new setting path is well-covered by tests.

The change is additive and backwards-compatible. The one notable issue is that helpDescription was added to MirrorPreference with the apparent intent of using it as per-item tooltips, but the UI never reads it — both pickers use hardcoded .help(…) strings instead. This leaves the property as dead code, but it has no runtime impact.

CompletionRenderModePolicy.swift — the unused helpDescription property warrants a quick follow-up (either wire it into the per-item .help() call or remove it).

Important Files Changed

Filename Overview
Cotabby/Support/CompletionRenderModePolicy.swift Adds Identifiable conformance and per-case displayLabel/helpDescription to MirrorPreference; helpDescription is dead code — never used by the UI.
Cotabby/Models/SuggestionEngineModels.swift Adds mirrorPreference: MirrorPreference field to SuggestionSettingsSnapshot; straightforward additive struct change.
Cotabby/Models/SuggestionSettingsModel.swift Adds mirrorPreference @Published property, persistence via UserDefaults raw-value string, setMirrorPreference mutator, and threads it through both the snapshot getter and snapshotPublisher (upgrading the inner CombineLatest to CombineLatest3). Logic is consistent and well-guarded.
Cotabby/Services/UI/OverlayController.swift Replaces the stored renderModePolicy with a computed currentRenderModePolicy that reads the live mirrorPreference on each showSuggestion call; keeps the optional override seam for tests. Clean and correct.
Cotabby/UI/MenuBarView.swift Adds a "Display" picker row wired through mirrorPreferenceBinding; mirrors the SettingsView implementation correctly.
Cotabby/UI/SettingsView.swift Adds "Suggestion Display" picker in the General section with a Binding that calls setMirrorPreference; well-placed and consistent with other settings rows.
CotabbyTests/CotabbyTestFixtures.swift Adds mirrorPreference: MirrorPreference = .auto to the shared snapshot fixture factory; all existing call sites pick it up transparently.
CotabbyTests/SuggestionAvailabilityEvaluatorTests.swift Adds two new tests covering default persistence and snapshot-publisher emission on preference change; both use the established runOnMainActor + XCTestExpectation pattern correctly.

Sequence Diagram

sequenceDiagram
    actor User
    participant Settings/MenuBar as Settings / MenuBar
    participant SettingsModel as SuggestionSettingsModel
    participant UserDefaults
    participant OverlayController
    participant Policy as CompletionRenderModePolicy

    User->>Settings/MenuBar: Select display preference (Auto/Inline/Popup)
    Settings/MenuBar->>SettingsModel: setMirrorPreference(_:)
    SettingsModel->>SettingsModel: "mirrorPreference = preference"
    SettingsModel->>UserDefaults: persist rawValue string
    SettingsModel-->>Settings/MenuBar: "@Published emits (UI updates)"

    Note over SettingsModel: snapshotPublisher picks up change via CombineLatest3

    User->>OverlayController: (types, triggers suggestion)
    OverlayController->>OverlayController: currentRenderModePolicy (computed)
    OverlayController->>SettingsModel: read mirrorPreference live
    OverlayController->>Policy: mode(for: geometry, bundleIdentifier:)
    Policy-->>OverlayController: .inline or .mirror(reason:)
    OverlayController-->>User: Show ghost text or popup card
Loading

Comments Outside Diff (1)

  1. Cotabby/Support/CompletionRenderModePolicy.swift, line 33-45 (link)

    P2 helpDescription is defined but never read

    The helpDescription property was added on MirrorPreference apparently for per-item tooltips, but neither SettingsView nor MenuBarView reference it — both views attach a single hardcoded .help(…) string to the whole Picker. The property is unreachable dead code as shipped.

    Fix in Codex Fix in Claude Code

Fix All in Codex Fix All in Claude Code

Reviews (1): Last reviewed commit: "Let users toggle popup-card suggestion d..." | Re-trigger Greptile

…menu

Phase 1 only triggered the new render mode when caret geometry was
estimated. This adds a persistent global preference (Auto / Inline /
Popup) so users can pin one style for every app, and surfaces it both
in the Settings General section and in the menu bar pop-up so it's
reachable without opening Settings.

The setting flows through `SuggestionSettingsSnapshot.mirrorPreference`
and the overlay controller reads it live each presentation, so the
toggle takes effect on the very next completion without a restart.
Defaults remain `.auto`, so existing users see identical behavior.
@FuJacob FuJacob merged commit 5277c21 into main May 28, 2026
4 checks passed
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