Skip to content

feat(hotkeys): add per-item hotkeys to open menu bar item menus#665

Merged
nightah merged 5 commits into
developmentfrom
feature/icon-hotkeys
Jun 4, 2026
Merged

feat(hotkeys): add per-item hotkeys to open menu bar item menus#665
nightah merged 5 commits into
developmentfrom
feature/icon-hotkeys

Conversation

@nightah

@nightah nightah commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Adds the ability to assign a global hotkey to any individual menu bar item so that pressing it opens that item's menu on demand, similar to Bartender and Barbee. Configuration lives under Settings > Hotkeys > Menu Bar Items as a collapsible table (icon, name, bundle, recorder), and the bindings are persisted and captured into profiles.

PR Type

  • Bugfix
  • CI/CD related changes
  • Code style update (formatting, renaming)
  • Documentation
  • Feature
  • Refactor
  • Performance improvement
  • Test addition or update
  • Other (please describe):

Does this PR introduce a breaking change?

  • Yes
  • No

If yes, please describe the impact and migration path:

N/A. Existing profiles remain compatible: the new MenuBarLayoutSnapshot.itemHotkeys field is optional and decoded with decodeIfPresent, so profiles saved before this change load unchanged.

What is the current behavior?

Hotkeys can only toggle whole menu bar sections (ToggleHiddenSection, ToggleAlwaysHiddenSection), open the search panel, enable the bar, or toggle application menus. There is no way to bind a hotkey that opens a specific menu bar item, so quick keyboard access to an individual item (for example Wi-Fi or a third-party item in the hidden section) is not possible.
Issue Number: #148

What is the new behavior?

Each menu bar item can be assigned its own hotkey from Settings > Hotkeys > Menu Bar Items, shown as a collapsible table with the item's captured icon, name, bundle identifier, and a HotkeyRecorder. Pressing the hotkey opens that item's menu: on-screen items are clicked in place, while hidden and always-hidden items are revealed, clicked, and rehidden, so all sections are supported. This reuses the search panel's activation path via a new shared MenuBarItemManager.activate(item:on:). Bindings are persisted in a dedicated MenuBarItemHotkeys defaults dictionary keyed by MenuBarItem.uniqueIdentifier and are captured into and restored from profiles alongside the existing layout data. Items whose owning app is not currently running still appear in the list (marked unavailable) so their binding can be reviewed or cleared, and their stored binding is retained rather than orphan-cleaned.

PR Checklist

  • I’ve built and run the app locally
  • I’ve checked for console errors or crashes
  • I've run relevant tests and they pass
  • I've added or updated tests (if applicable)
  • I've updated documentation as needed

Other information

Architecturally this mirrors the existing per-profile hotkey machinery: a new externally-handled openMenuBarItem action (excluded from settingsActions like profileApply) is dispatched by Hotkey.Listener through MenuBarManager.hotkeyItemMap to openItem(withIdentifier:), and MenuBarManager.rebuildItemHotkeys() reconciles the live Hotkey objects incrementally so a frequent item-cache tick never tears down an in-use registration. The image cache live-refresh loop was also extended to treat the Hotkeys pane as a capture consumer, so off-screen item icons render at their captured size instead of placeholders.

Notes for reviewers

  • The incremental (non-teardown) rebuild in rebuildItemHotkeys() is the main correctness point: please sanity-check that a routine itemCache tick does not drop an in-use binding, and that absent-app bindings persist and re-fire when the app relaunches.
  • The reused temporarilyShow activation path performs a physical reveal/rehide for hidden and always-hidden items; worth a look on both single-display and notched-display setups.
  • Tests added: HotkeyActionTests updated for the new openMenuBarItem case (case count, settingsActions exclusion, raw-value round-trip) and ProfileTests covering MenuBarLayoutSnapshot.itemHotkeys encode/decode plus backward compatibility when the field is absent.

Closes #148

Summary by CodeRabbit

  • New Features

    • Assign keyboard shortcuts to individual menu bar items and open items from anywhere.
    • Collapsible per-item hotkey list in Hotkeys settings showing available and configured-but-absent items.
  • Improvements

    • More reliable activation for menu bar items, including special tray apps and off-screen cases.
    • Item icons preloaded and live refresh while the per-item list is expanded.
    • Per-item hotkeys persisted and restored when applying saved profiles.
  • Documentation

    • Added UI strings for menu bar item hotkey configuration.
  • Tests

    • Updated tests to cover the new hotkey behavior.

Adds the ability to assign a global hotkey to any individual menu bar item so its menu opens on demand, similar to Bartender and Barbee. Previously hotkeys could only toggle whole sections (`ToggleHiddenSection`, `ToggleAlwaysHiddenSection`), open search, enable the bar, or toggle application menus; there was no way to trigger a specific item.

This mirrors the existing per-profile hotkey machinery. A new externally-handled `openMenuBarItem` action is added (excluded from `settingsActions` like `profileApply`), and `Hotkey.Listener` dispatches it through `MenuBarManager.hotkeyItemMap` to `openItem(withIdentifier:)`. Bindings are persisted in a dedicated `MenuBarItemHotkeys` defaults dictionary keyed by `MenuBarItem.uniqueIdentifier`, and `MenuBarManager.rebuildItemHotkeys()` reconciles the live `Hotkey` objects incrementally so a frequent item-cache tick never tears down an in-use registration; bindings for items whose owning app has quit are retained rather than orphan-cleaned.

Activation reuses the search panel's path via a new shared `MenuBarItemManager.activate(item:on:)` that clicks on-screen items in place and routes hidden and always-hidden items through `temporarilyShow` (reveal, click, rehide), so all sections are supported.

The UI lives under Settings > Hotkeys > Menu Bar Items as a collapsible table with columns for the item icon (rendered at its captured size), name, bundle, and a `HotkeyRecorder`. Items with a saved binding whose app is not currently running are still listed so the binding can be reviewed or cleared.

Per-item bindings are captured into profiles via a new optional `MenuBarLayoutSnapshot.itemHotkeys` field (decoded with `decodeIfPresent` for backward compatibility) and restored on apply, exactly like `customNames`. The image cache live-refresh loop is also extended to treat the Hotkeys pane as a capture consumer, so off-screen item icons render instead of placeholders.

Closes #148

Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d2ef00d1-4850-4814-ab23-51de07c7d6d5

📥 Commits

Reviewing files that changed from the base of the PR and between b7eff59 and bded5a5.

📒 Files selected for processing (1)
  • Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift

📝 Walkthrough

Walkthrough

Adds an externally-handled openMenuBarItem hotkey, manages live per-item Hotkey instances with Defaults persistence and reconciliation, centralizes activation (including AX handling for Electron items), surfaces a per-item Hotkeys list with icons, and captures/restores bindings in profiles.

Changes

Per-item menu bar hotkeys

Layer / File(s) Summary
HotkeyAction enum extension
Thaw/Hotkeys/HotkeyAction.swift, ThawTests/HotkeyActionTests.swift, Thaw/Utilities/Defaults.swift
Adds openMenuBarItem enum case, excludes it from settings UI, adds Defaults.Key.menuBarItemHotkeys, and updates tests for new case and settings filter.
MenuBarManager hotkey state & reconciliation
Thaw/MenuBar/MenuBarManager.swift
Introduces itemHotkeys, hotkeyItemMap, itemHotkeyCancellables, wires rebuild on setup and debounced itemCache changes, and implements rebuildItemHotkeys() to reconcile persisted bindings and live items.
Item activation entrypoint
Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift, Thaw/MenuBar/MenuBarManager.swift, Shared/Utilities/AXHelpers.swift
Adds MenuBarItemManager.activate(item:on:) to choose direct click vs temporarilyShow (with Electron AX-press path), imports AXSwift, adds AXHelpers.press, and exposes MenuBarManager.openItem(withIdentifier:).
Hotkey dispatch to manager
Thaw/Hotkeys/Hotkey.swift, Thaw/MenuBar/Search/MenuBarSearchPanel.swift
Hotkey listener maps openMenuBarItem Hotkey instances via hotkeyItemMap and invokes MenuBarManager.openItem(withIdentifier:). Search panel now calls itemManager.activate(item:on:).
Hotkeys settings UI & image cache gating
Thaw/Settings/SettingsPanes/HotkeysSettingsPane.swift, Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift, Thaw/Resources/Localizable.xcstrings
Adds MenuBarItemHotkeyList disclosure with rows for on-screen and configured-but-absent items, prewarms icons, toggles image cache expansion state, and updates live-refresh gating; adds two new localization strings.
Profile snapshot support
Thaw/Settings/Models/Profile.swift, Thaw/Settings/Models/ProfileManager.swift, ThawTests/ProfileTests.swift
Adds optional itemHotkeys to MenuBarLayoutSnapshot, persists/restores item bindings on profile apply/capture, and adds encode/decode tests for the new field.

Sequence Diagram(s):

sequenceDiagram
  participant HotkeyListener as Hotkey.Listener
  participant MenuBarMgr as MenuBarManager
  participant ItemMgr as MenuBarItemManager
  participant AXHelper as AXHelpers
  HotkeyListener->>MenuBarMgr: lookup hotkeyItemMap[ObjectIdentifier(hotkey)]
  MenuBarMgr->>MenuBarMgr: openItem(withIdentifier:)
  MenuBarMgr->>ItemMgr: activate(item:on: displayID)
  ItemMgr->>AXHelper: press via AX (Electron) or synthetic click
  AXHelper-->>ItemMgr: press result
  ItemMgr-->>MenuBarMgr: activation result
  MenuBarMgr-->>HotkeyListener: done
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • stonerl/Thaw#452: Related menu-bar item activation/reveal reliability changes touching MenuBarItemManager.
  • stonerl/Thaw#331: Prior hotkey dispatch changes around externally-handled .profileApply that this PR extends with .openMenuBarItem.

Suggested labels

feature, test

Suggested reviewers

  • diazdesandi

"I'm a rabbit in the codewood, hopping through the keys,
I bind a hotkey to a menu friend with nimble, gentle ease.
Press once — the bar unfolds a tale, no longer out of sight,
A tiny click, a little cheer, your workflow's feeling right!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 64.10% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main feature: adding per-item hotkeys to open menu bar item menus.
Description check ✅ Passed The description comprehensively covers all template sections: what it does, PR type (Feature + Test update), breaking changes (No), current and new behavior with issue reference, and a detailed checklist with notes for reviewers.
Linked Issues check ✅ Passed All requirements from issue #148 are met: the PR implements per-item hotkeys to open menu bar item menus, stores configuration in Settings > Hotkeys > Menu Bar Items, supports items in any section (including hidden/always-hidden), and enables quick keyboard access similar to Bartender/Barbee.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing per-item menu bar hotkeys. The accessibility improvements for item activation, image cache adjustments, and profile integration are all supporting features necessary for the core objective.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/icon-hotkeys

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Repository owner deleted a comment from github-actions Bot Jun 2, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift`:
- Around line 444-446: The Hotkeys settings branch is opting the whole Hotkeys
pane into live capture even when the MenuBarItemHotkeyList disclosure is
collapsed; update the predicate that currently uses nav.isAppFrontmost &&
nav.isSettingsPresented && (nav.settingsNavigationIdentifier == .hotkeys) to
also require the hotkey-list disclosure be expanded (e.g., check the
disclosure's isExpanded state or an explicit "item hotkey list visible" flag)
before including MenuBarSection.Name.allCases; do the same change for the
corresponding duplicate logic used around MenuBarSection.Name.allCases (the
other occurrence you noted) so capture only activates when the list is actually
visible.

In `@Thaw/Settings/Models/ProfileManager.swift`:
- Around line 454-460: The code currently coerces
profile.menuBarLayout.itemHotkeys to an empty dictionary and always writes to
Defaults, which wipes existing per-item hotkeys for older profiles; change this
to only write and rebuild when the profile actually provides itemHotkeys: check
whether profile.menuBarLayout.itemHotkeys is non-nil, and if so call
Defaults.set(hotkeys, forKey: .menuBarItemHotkeys) and then
appState.menuBarManager.rebuildItemHotkeys(); do not write or call rebuild when
itemHotkeys is nil so existing user bindings remain untouched.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: ee340c2f-fcaa-402f-8288-c0ee5ac4941f

📥 Commits

Reviewing files that changed from the base of the PR and between 7b1001b and 7a402a3.

📒 Files selected for processing (13)
  • Thaw/Hotkeys/Hotkey.swift
  • Thaw/Hotkeys/HotkeyAction.swift
  • Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift
  • Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift
  • Thaw/MenuBar/MenuBarManager.swift
  • Thaw/MenuBar/Search/MenuBarSearchPanel.swift
  • Thaw/Resources/Localizable.xcstrings
  • Thaw/Settings/Models/Profile.swift
  • Thaw/Settings/Models/ProfileManager.swift
  • Thaw/Settings/SettingsPanes/HotkeysSettingsPane.swift
  • Thaw/Utilities/Defaults.swift
  • ThawTests/HotkeyActionTests.swift
  • ThawTests/ProfileTests.swift

Comment thread Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift Outdated
Comment thread Thaw/Settings/Models/ProfileManager.swift

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift (1)

463-480: ⚡ Quick win

Read isItemHotkeyListExpanded through the snapshot instead of directly.

hasVisibleCaptureConsumer(nav:) is nonisolated and is invoked off the main actor (e.g. from refreshVisibleConsumersOrPrewarmLayoutCache), but the .hotkeys branch now reads self.isItemHotkeyListExpanded live while that @Published flag is mutated on the main thread via setItemHotkeyListExpanded. NavigationStateSnapshot exists precisely to batch such state into a single MainActor hop and avoid cross-actor reads; the new flag bypasses that contract. Fold it into the snapshot for consistency and to keep the read race-free.

♻️ Proposed change: carry the flag in the snapshot
 struct NavigationStateSnapshot {
     let isIceBarPresented: Bool
     let isSearchPresented: Bool
     let isAppFrontmost: Bool
     let isSettingsPresented: Bool
     let settingsNavigationIdentifier: SettingsNavigationIdentifier?
+    let isItemHotkeyListExpanded: Bool
 }

Populate it in makeNavigationStateSnapshot() / hasVisibleCaptureConsumer() (and the early-return default), then:

         case .hotkeys:
             // Only the expanded per-item hotkey list consumes item icons.
-            return isItemHotkeyListExpanded
+            return nav.isItemHotkeyListExpanded
🤖 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 `@Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift` around lines 463 -
480, hasVisibleCaptureConsumer(nav:) currently reads
self.isItemHotkeyListExpanded directly (a `@Published` main-actor property)
causing cross-actor access; instead add a boolean isItemHotkeyListExpanded to
NavigationStateSnapshot, populate it in makeNavigationStateSnapshot(), and
change hasVisibleCaptureConsumer(nav:) to read nav.isItemHotkeyListExpanded (and
use that early-return default) so callers like
refreshVisibleConsumersOrPrewarmLayoutCache remain race-free; keep
setItemHotkeyListExpanded as the main-actor mutator.
🤖 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.

Nitpick comments:
In `@Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift`:
- Around line 463-480: hasVisibleCaptureConsumer(nav:) currently reads
self.isItemHotkeyListExpanded directly (a `@Published` main-actor property)
causing cross-actor access; instead add a boolean isItemHotkeyListExpanded to
NavigationStateSnapshot, populate it in makeNavigationStateSnapshot(), and
change hasVisibleCaptureConsumer(nav:) to read nav.isItemHotkeyListExpanded (and
use that early-return default) so callers like
refreshVisibleConsumersOrPrewarmLayoutCache remain race-free; keep
setItemHotkeyListExpanded as the main-actor mutator.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: edb6a73c-94f2-4cc7-9035-9ccb08e427ee

📥 Commits

Reviewing files that changed from the base of the PR and between 7a402a3 and c9f214b.

📒 Files selected for processing (2)
  • Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift
  • Thaw/Settings/SettingsPanes/HotkeysSettingsPane.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • Thaw/Settings/SettingsPanes/HotkeysSettingsPane.swift

@nightah nightah force-pushed the feature/icon-hotkeys branch from c9f214b to b299afd Compare June 2, 2026 23:17

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Thaw/Settings/SettingsPanes/HotkeysSettingsPane.swift (1)

209-219: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Sort the configured-but-absent rows for stable ordering.

menuBarManager.itemHotkeys is a Dictionary, so its iteration order is not deterministic and changes whenever the dictionary mutates (e.g. a binding is added/removed). The absent rows will reshuffle relative to the deterministically-ordered present rows, producing a jumpy list. Sort by a stable key (e.g. name or id) before appending.

🔀 Proposed fix for deterministic ordering
-        for (id, hotkey) in menuBarManager.itemHotkeys
-            where hotkey.keyCombination != nil && !seen.contains(id)
-        {
+        for (id, hotkey) in menuBarManager.itemHotkeys
+            .filter({ $0.value.keyCombination != nil && !seen.contains($0.key) })
+            .sorted(by: { $0.key < $1.key })
+        {
             rows.append(Row(
                 id: id,
                 name: lastKnownName(for: id),
                 bundle: bundle(forIdentifier: id),
                 item: nil,
                 hotkey: hotkey
             ))
         }
🤖 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 `@Thaw/Settings/SettingsPanes/HotkeysSettingsPane.swift` around lines 209 -
219, menuBarManager.itemHotkeys is iterated directly (in
HotkeysSettingsPane.swift) which is a Dictionary and yields non-deterministic
order; to fix, create a stable sorted sequence first (e.g. sort
menuBarManager.itemHotkeys.map { ($0.key, $0.value) } by lastKnownName(for: id)
or by id) and then iterate that sorted array instead of the raw dictionary so
the absent Row entries appended to rows are in a deterministic order; update the
loop that builds Row(id:..., hotkey:...) to use the sorted collection and keep
the existing seen check.
🤖 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 `@Thaw/Settings/SettingsPanes/HotkeysSettingsPane.swift`:
- Around line 209-219: menuBarManager.itemHotkeys is iterated directly (in
HotkeysSettingsPane.swift) which is a Dictionary and yields non-deterministic
order; to fix, create a stable sorted sequence first (e.g. sort
menuBarManager.itemHotkeys.map { ($0.key, $0.value) } by lastKnownName(for: id)
or by id) and then iterate that sorted array instead of the raw dictionary so
the absent Row entries appended to rows are in a deterministic order; update the
loop that builds Row(id:..., hotkey:...) to use the sorted collection and keep
the existing seen check.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 04174356-a5cf-4828-a031-35e2efe4a323

📥 Commits

Reviewing files that changed from the base of the PR and between c9f214b and b299afd.

📒 Files selected for processing (2)
  • Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift
  • Thaw/Settings/SettingsPanes/HotkeysSettingsPane.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift

Gate the Hotkeys pane's live image capture on the per-item hotkey list actually being visible. Previously navigating to the Hotkeys pane opted the whole pane into the live capture loop, which captures hidden and always-hidden item icons via the costly off-screen SkyLight path, even while the `MenuBarItemHotkeyList` disclosure was collapsed and showing nothing.

`MenuBarItemImageCache` now tracks an `isItemHotkeyListExpanded` flag (set by the disclosure via `setItemHotkeyListExpanded(_:)`), and both the `hasVisibleCaptureConsumer` predicate and the duplicate section-selection logic inside `runLiveRefreshLoop` require it for the `.hotkeys` case; the `.menuBarLayout` pane still qualifies unconditionally. An observer on the flag starts and stops the loop through the existing `startLiveRefreshIfNeeded()`, and the list's prewarm task is keyed on its expanded state so it no-ops while collapsed.

Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
@nightah nightah force-pushed the feature/icon-hotkeys branch from b299afd to fc61f04 Compare June 2, 2026 23:33
@nightah

nightah commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator Author

.build

Here are some pictures of the feature in action:

image image

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

Build triggered for branch feature/icon-hotkeys. Check the Actions tab for progress.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift (1)

3535-3553: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Propagate the Electron activation path through the temporarily-show branch.

The off-screen branch at Line 3552 still delegates to temporarilyShow(...), and that path clicks synthetically at Lines 4167-4195. So hidden/always-hidden Electron tray items will still fail to open from a hotkey even though the on-screen branch now handles them via AX press.

Please route the post-reveal activation through the same strategy selection instead of hard-coding synthetic clicks inside the temporary-show flow.

🤖 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 `@Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift` around lines 3535 - 3553,
The off-screen path should use the same Electron-via-AX vs synthetic click
decision as the on-screen branch: modify temporarilyShow(item:clickingWith:on:)
(or the code that runs after it returns) so that once the item is revealed you
call isElectronItem(item) and, if true, attempt pressItemViaAccessibility(item)
and log/return on success (MenuBarItemManager.diagLog.info("Activated
\(item.logString) via AX press")), otherwise fall back to try await click(item:
item, with: .left) and catch/log errors
(MenuBarItemManager.diagLog.error("Failed to activate \(item.logString):
\(error)")). Ensure the same helper functions (isElectronItem,
pressItemViaAccessibility, click) are used and no synthetic click is hard-coded
inside the temporarily-show flow.
🤖 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 `@Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift`:
- Around line 3559-3612: Update the checks that require item.sourcePID in both
isElectronItem(_:) and pressItemViaAccessibility(_:) to fall back to
item.ownerPID when sourcePID is nil: where you currently guard/unwrap let
sourcePID = item.sourcePID and use it for
NSRunningApplication(processIdentifier:) and subsequent
AXHelpers.application(for:)/extrasMenuBar resolution, change to use let pid =
item.sourcePID ?? item.ownerPID (or equivalent) so the Electron-detection path
(isElectronItem) and the AX press path (pressItemViaAccessibility) work during
startup before sourcePID is resolved.

---

Outside diff comments:
In `@Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift`:
- Around line 3535-3553: The off-screen path should use the same Electron-via-AX
vs synthetic click decision as the on-screen branch: modify
temporarilyShow(item:clickingWith:on:) (or the code that runs after it returns)
so that once the item is revealed you call isElectronItem(item) and, if true,
attempt pressItemViaAccessibility(item) and log/return on success
(MenuBarItemManager.diagLog.info("Activated \(item.logString) via AX press")),
otherwise fall back to try await click(item: item, with: .left) and catch/log
errors (MenuBarItemManager.diagLog.error("Failed to activate \(item.logString):
\(error)")). Ensure the same helper functions (isElectronItem,
pressItemViaAccessibility, click) are used and no synthetic click is hard-coded
inside the temporarily-show flow.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: f5e43d25-0c10-48e6-ab01-bec4225dc1c5

📥 Commits

Reviewing files that changed from the base of the PR and between fc61f04 and 662793e.

📒 Files selected for processing (3)
  • Shared/Utilities/AXHelpers.swift
  • Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift
  • Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift

Comment thread Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift
Electron and Chromium menu bar items (for example Claude) ignore the synthetic mouse click Thaw posts to open a menu, so a per-item hotkey or the search panel could not open them. Detect those items by the presence of their bundled Electron framework and open them with an Accessibility press instead. Every other app keeps the normal synthetic click, which preserves its native open and close toggle and works with the popover-style menus (such as Cap and Droppy) that a stray Accessibility interaction would otherwise disturb.

Also remove backticks from two doc comments so they follow the project convention of not wrapping identifiers in backticks inside Swift comments.

Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
@nightah nightah force-pushed the feature/icon-hotkeys branch from 662793e to 282b77c Compare June 3, 2026 04:15
When an item from the hidden or always-hidden section is opened (via a per-item hotkey or search), it is temporarily moved to the visible area and rehidden once its menu closes. The rehide waits on isShowingInterface, which tracks the popup window opened by the click; for several apps that detection failed and the item rehid out from under its still-open menu.

Three cases are addressed. Menu-bar agent apps (LSUIElement / .accessory, e.g. BetterDisplay) can never report isActive, so their captured popover was treated as hidden the instant it appeared; the captured popup's on-screen state is now trusted for them. Some apps (e.g. Claude) draw their menu at a non-standard window level (Electron uses level 1000), and because Thaw, not the user, opened it the app is not frontmost, so a menu-sized captured window is now trusted regardless of isActive. Finally, some apps (e.g. DisplayLink) draw their menu as a status-level or main-menu-level window owned by the app rather than at pop-up level, so the appHasVisiblePopup fallback now matches those levels too, gated on a window taller than a menu bar item so the status item itself is not mistaken for a menu.

Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
@nightah

nightah commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

.build

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

Build triggered for branch feature/icon-hotkeys. Check the Actions tab for progress.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift`:
- Around line 3598-3607: openItem(withIdentifier:) is using the cached
item.bounds.center which can be stale; replace that with the live AX frame
before the 10pt distance check: call AXHelpers.frame(for: item) to obtain a
liveFrame and compute itemCenter = liveFrame.center, falling back to
item.bounds.center only if AXHelpers.frame(for:) returns nil, then run the
existing children.min(...) and bestFrame.distance check against that
live-derived center so Electron items don’t fall back to the synthetic click
path. Ensure the new lookup is used where itemCenter is defined and keep the
rest of the nearest-child logic unchanged.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09b883d7-7f71-405e-806a-d92e5e5141c9

📥 Commits

Reviewing files that changed from the base of the PR and between fc61f04 and b7eff59.

📒 Files selected for processing (3)
  • Shared/Utilities/AXHelpers.swift
  • Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift
  • Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift
🚧 Files skipped from review as they are similar to previous changes (2)
  • Shared/Utilities/AXHelpers.swift
  • Thaw/MenuBar/MenuBarItems/MenuBarItemImageCache.swift

Comment thread Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift Outdated
Match a multi-status-item app's Accessibility child against the item's live window bounds (Bridging.getWindowBounds) instead of its cached bounds in pressItemViaAccessibility, falling back to the cached bounds only when the live lookup returns nil. A stale cached position could mis-match the nearest child or exceed the proximity check and make an Electron item fall back to the synthetic click it ignores.

Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2026

Copy link
Copy Markdown

@nightah nightah merged commit 8b0e7bd into development Jun 4, 2026
8 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.

[Feature Request]: hotkey to open app in menubar

2 participants