feat(menubar): survive a hidden icon and let the user hide it#40
Merged
Conversation
A SwiftUI `MenuBarExtra`-only app self-terminates the instant its status item is hidden — Apple documents this. The user can hide the icon via Control Center (macOS 26+) or by ⌘-dragging it off the bar, and because that hidden state is *persisted*, an unguarded app re-terminates on every later launch before any UI appears: it looks like a crash and can never be reopened. Veto that one unsolicited `terminate:` in `applicationShouldTerminate` when the icon is hidden, so the lock engine keeps running. Recovery comes from two paths: relaunching the running app re-presents Settings (`applicationShouldHandleReopen`), and a cold launch while the icon is hidden also opens Settings — guarded by `launchAtLoginActive` and the default-launch key so it never pops a window at login. Every *wanted* exit still goes through. Route the explicit Quit, the `lockime://quit` command, and the self-test teardown through a new `AppState.quit()` that sets `terminationRequested`; Sparkle's install-and-relaunch is recognized via `UpdateController.isInstallingUpdate`; and a logout/restart/shutdown is recognized by its `kAEQuitApplication` Apple Event. Settings is opened through the captured `\.openSettings` action (`SettingsActionBridge`) because `showSettingsWindow:` silently no-ops for this accessory app. Signed-off-by: Kevin Cui <bh@bugs.cc>
The previous commit taught the app to survive an *unwanted* hide of its status item; this adds an explicit General ▸ Menu Bar toggle so the user can hide it on purpose. The lock engine keeps running with the icon gone, and relaunching re-presents Settings. Store it as a per-device preference (`menuBarIconHidden`) in its own `UserDefaults` key, deliberately outside `LockConfiguration` so it never travels through config export/import — the same treatment as `apiEnabled`. `setMenuBarIconHidden` is the single write path. Drive `MenuBarExtra(isInserted:)` from an `@AppStorage` mirror of that key rather than the `@Observable` flag directly: an `@Observable` read inside a `Binding`'s `get` closure doesn't register with the `App`'s scene graph, so the scene would never re-evaluate when the flag flips. `@AppStorage` is the `DynamicProperty` that does, and routing its `set` back through `AppState` keeps one source of truth. Extend the terminate veto and the cold-launch Settings reveal to honor either signal — the system-persisted hide (now `statusItemPersistedHidden`) or the in-app toggle (`appState.menuBarIconHidden`) — because a programmatic hide via `isInserted` doesn't write the system visibility default the old guard watched. Every user-facing string is translated for all supported languages. Signed-off-by: Kevin Cui <bh@bugs.cc>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughThe PR adds a menu-bar icon hide preference. Sequence DiagramssequenceDiagram
participant AppDelegate
participant AppState
participant LockIMEApp
participant SettingsActionBridge
participant SwiftUIOpenSettings
LockIMEApp->>SettingsActionBridge: attach background view
SettingsActionBridge->>AppState: set openSettingsAction
AppDelegate->>AppState: openSettingsAction?()
AppState->>SwiftUIOpenSettings: invoke openSettings
sequenceDiagram
participant MenuBarView
participant URLCommandHandler
participant AppState
participant AppDelegate
MenuBarView->>AppState: quit()
URLCommandHandler->>AppState: quit()
AppState->>AppDelegate: applicationShouldTerminate
AppDelegate-->>AppState: allow termination
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/LockIME/AppDelegate.swift`:
- Around line 45-47: The launch gating in
AppDelegate.applicationDidFinishLaunching is using the persisted Launch at Login
preference as if it were a per-launch signal, which can incorrectly suppress the
recovery window on a manual default launch. Update the isDefaultLaunch/guard
logic to rely only on a verified current-login auto-start indicator for this
launch, and keep the Settings-opening path in the DispatchQueue.main.asyncAfter
block available when the app is manually opened while hidden.
In `@Sources/LockIME/Updates/UpdateController.swift`:
- Around line 45-48: The isInstallingUpdate flag in UpdateController is too
broad because it treats .readyToInstall as an active install phase, which lets
applicationShouldTerminate bypass the hidden-icon guard too early. Tighten the
logic in isInstallingUpdate so it only returns true for the actual
install/relaunch phase (for example, .installing) unless you can guarantee
.readyToInstall only occurs after user-confirmed relaunch. Keep the change
localized to UpdateController and the phase check that
applicationShouldTerminate relies on.
🪄 Autofix (Beta)
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: CHILL
Plan: Pro
Run ID: 247f1f96-255b-4d06-9f72-8afb5818d5c6
📒 Files selected for processing (8)
Sources/LockIME/API/URLCommandHandler.swiftSources/LockIME/AppDelegate.swiftSources/LockIME/AppState.swiftSources/LockIME/Localizable.xcstringsSources/LockIME/LockIMEApp.swiftSources/LockIME/UI/MenuBarView.swiftSources/LockIME/UI/Settings/GeneralSettingsPane.swiftSources/LockIME/Updates/UpdateController.swift
`isInstallingUpdate` also returned true for `.readyToInstall`, the "install and relaunch?" prompt that waits on the user and can stay up indefinitely. While it showed, a status-item hide would hit the `isInstallingUpdate` short-circuit in `applicationShouldTerminate` and terminate the app instead of being vetoed — losing the survival behavior during that window. Limit it to `.installing`, the phase entered only once the user has committed to the relaunch (`LockIMEUserDriver.showInstallingUpdate`), so the ready-to-install prompt now stays protected by the hidden-icon veto. Signed-off-by: Kevin Cui <bh@bugs.cc>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A SwiftUI
MenuBarExtra-only app self-terminates the instant its status item is hidden — Apple documents this — and because that hidden state is persisted, an unguarded app re-terminates on every later launch before any UI appears: it looks like a crash and can never be reopened. This branch makes a hidden icon a survivable, and now deliberate, state.First, it vetoes that one unsolicited
terminate:inapplicationShouldTerminateso the lock engine keeps running, and gives the user a way back: relaunching the app re-presents Settings (applicationShouldHandleReopen), and a non-login cold launch while hidden reveals Settings too. Every wanted exit still goes through — an explicit Quit /lockime://quit(routed through a newAppState.quit()that flags the termination), a Sparkle install-and-relaunch, and a logout/restart/shutdown (itskAEQuitApplicationApple Event). BecauseshowSettingsWindow:silently no-ops for this accessory app, Settings is opened through the captured\.openSettingsaction (SettingsActionBridge).On top of that, it adds an explicit General ▸ Menu Bar toggle so the user can hide the icon on purpose, with the same keep-running-and-recover behavior. A few load-bearing details:
menuBarIconHidden) lives in its ownUserDefaultskey, outsideLockConfiguration, so it never travels through config export/import — the same treatment asapiEnabled.MenuBarExtra(isInserted:)is driven from an@AppStoragemirror of that key, not the@Observableflag directly: an@Observableread inside aBinding'sgetclosure doesn't register with theApp's scene graph, so the scene would never re-evaluate when the flag flips. Writes still route throughAppState, keeping one source of truth.statusItemPersistedHidden) or the in-app toggle — because a programmatic hide viaisInserteddoesn't write the system visibility default the original guard watched.Every user-facing string is translated for all supported languages. I verified the framework behavior directly, since it's subtle:
isInserted: falseflips the underlyingNSStatusItem.isVisibleto false (the icon really disappears), the app survives the hide, and the\.openSettingsbridge is still captured even on a cold launch that starts hidden — so recovery works in every case.fixed: #38