Skip to content

feat: add "Icon color" submenu for Linux tray (Auto/Black/White)#491

Open
IliyaBrook wants to merge 6 commits intoaaddrick:mainfrom
IliyaBrook:features/change-icon-color
Open

feat: add "Icon color" submenu for Linux tray (Auto/Black/White)#491
IliyaBrook wants to merge 6 commits intoaaddrick:mainfrom
IliyaBrook:features/change-icon-color

Conversation

@IliyaBrook
Copy link
Copy Markdown
Contributor

Summary

Adds a user-selectable Icon color submenu (Auto / Black / White) to the Linux tray context menu, sitting between Show App and Quit. This gives users explicit control over the tray icon colour for the setups where automatic detection falls short.

As a bonus, the same patch fixes a pre-existing minor bug: changing the desktop theme while Claude is already running used to leave a duplicate tray icon behind.

Why a feature, not just a fix?

Electron's nativeTheme.shouldUseDarkColors reports the desktop colour scheme, not the actual panel colour — Chromium reads the xdg-desktop-portal org.freedesktop.appearance setting and nothing else. On a lot of real-world setups those two diverge, and the tray icon ends up invisible.

The one that prompted this PR is the out-of-the-box Fedora Plasma KDE 6 experience with the default Fedora theme: the panel is black by design, but the global colour scheme is light, so Electron picks the black tray icon and it disappears into the panel. Other affected scenarios include:

  • KDE users who intentionally override just the panel's colour (dark panel over a light desktop theme, or vice versa)
  • Plasma themes whose xdg-desktop-portal-kde mapping doesn't line up with the panel's rendered colour (the portal only translates the Breeze family — third-party schemes return "no preference")
  • Electron v39's Chromium 142 regression which inverts the value on KDE Plasma 6 + Wayland
dark_1 Before the fix on Fedora Plasma KDE 6 with the default theme: the tray icon is there, but it's black on a black panel.

Rather than try to paper over every edge case of the detection heuristic, this PR does what apps in similar spots have settled on: hand the choice back to the user while keeping auto-detection as the default.

How it works for the user

Right-click the tray icon → pick one of:

Option Behaviour
Auto (default) Electron's nativeTheme.shouldUseDarkColors picks the icon — identical to pre-PR behaviour
Black Force the black icon (TrayIconTemplate.png) for light panels
White Force the white icon (TrayIconTemplate-Dark.png) for dark panels

The radio labels describe the icon's own colour, so "White" means you'll see a white icon — no mental inversion required. The selection is persisted to ~/.config/Claude/linux-tray.json and survives restarts.

submenu The submenu with Auto / Black / White.

Bonus: duplicate tray icon on OS theme change

Unrelated to the feature, this PR also fixes a long-standing annoyance: if you switched the KDE colour scheme while Claude was open, you'd see two tray icons side by side — the old one plus the new one. The pre-existing tray rebuild logic destroys the current StatusNotifierItem and immediately registers a new one; on KDE Plasma the new SNI can appear before Plasma has processed the unregister signal for the old one, and both

IliyaBrook and others added 2 commits April 23, 2026 02:24
Adds a user-selectable "Icon color" submenu (Auto / Black / White)
between "Show App" and "Quit" on the Linux tray context menu. Fixes
the long-standing issue where the tray icon colour doesn't adapt to
the panel on setups where Electron's nativeTheme.shouldUseDarkColors
misdetects — most commonly KDE Plasma with a dark panel on a light
desktop theme. Label names describe the icon's colour (not the theme
it targets) to avoid the confusion the auto-detection naming caused.

- Auto (default) preserves pre-existing behaviour.
- Black / White are explicit overrides that win over the detector.

Supporting pieces:

- scripts/tray-icon-settings.js — persistence under
  ~/.config/Claude/linux-tray.json, with migration for users who
  picked an earlier build's "dark"/"light" values.

- scripts/frame-fix-wrapper.js — Proxy on nativeTheme so
  shouldUseDarkColors reflects the user's choice. Relaunch on
  change uses spawn($APPIMAGE) for AppImage builds (app.relaunch()
  crashes with SIGTRAP once the squashfs mount is torn down), and
  app.relaunch() otherwise.

- scripts/patches/tray.sh:
    * patch_tray_icon_submenu — injects the Menu.buildFromTemplate
      entry anchored on the Quit i18n ID (dKX0bpR+a2), which is
      more stable across releases than minified function names.
    * patch_tray_inplace_update — fast-path for the tray rebuild
      function that reuses the existing StatusNotifierItem on
      theme change (setImage + setContextMenu) instead of
      destroy+recreate. Fixes the pre-existing KDE duplicate-tray
      bug where the old SNI was still registered when the new one
      appeared.

- tests/tray-icon-settings.test.js — 19 unit tests (node --test).

- docs/learnings/tray-icon-theme.md — design notes, KDE Connect
  reference, and troubleshooting.

Co-Authored-By: Claude <[email protected]>
IliyaBrook and others added 2 commits April 23, 2026 04:05
Adds a user-selectable "Icon color" submenu (Auto / Black / White)
between "Show App" and "Quit" on the Linux tray context menu. Fixes
the long-standing issue where the tray icon colour doesn't adapt to
the panel on setups where Electron's nativeTheme.shouldUseDarkColors
misdetects — most commonly KDE Plasma with a dark panel on a light
desktop theme. Label names describe the icon's colour (not the theme
it targets) to avoid the confusion the auto-detection naming caused.

- Auto (default) preserves pre-existing behaviour.
- Black / White are explicit overrides that win over the detector.

Supporting pieces:

- scripts/tray-icon-settings.js — persistence under
  ~/.config/Claude/linux-tray.json, with migration for users who
  picked an earlier build's "dark"/"light" values.

- scripts/frame-fix-wrapper.js — Proxy on nativeTheme so
  shouldUseDarkColors reflects the user's choice. Relaunch on
  change uses spawn($APPIMAGE) for AppImage builds (app.relaunch()
  crashes with SIGTRAP once the squashfs mount is torn down), and
  app.relaunch() otherwise.

- scripts/patches/tray.sh:
    * patch_tray_icon_submenu — injects the Menu.buildFromTemplate
      entry anchored on the Quit i18n ID (dKX0bpR+a2), which is
      more stable across releases than minified function names.
    * patch_tray_inplace_update — fast-path for the tray rebuild
      function that reuses the existing StatusNotifierItem on
      theme change (setImage + setContextMenu) instead of
      destroy+recreate. Fixes the pre-existing KDE duplicate-tray
      bug where the old SNI was still registered when the new one
      appeared.

- tests/tray-icon-settings.test.js — 19 unit tests (node --test).

- docs/learnings/tray-icon-theme.md — design notes, KDE Connect
  reference, and troubleshooting.

Co-Authored-By: Claude <[email protected]>
… features/change-icon-color

# Conflicts:
#	docs/learnings/tray-icon-theme.md
Copy link
Copy Markdown
Owner

@aaddrick aaddrick left a comment

Choose a reason for hiding this comment

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

Hey @IliyaBrook — thanks for the detailed writeup, and the learnings doc in particular is great. Leaving this as a COMMENT because there's a blocker, a repro question, and one scope thing that isn't really yours to answer.

1. Hardcoded minified locals in patch_tray_inplace_update

T, E, and M are extracted dynamically, but t (the icon-path arg) and e (the menuBarEnabled local) are literal in the injected string. Per CLAUDE.md these names change between releases, and the idempotency grep keys on (t)) so a future drift wouldn't re-trigger the patch cleanly. Both are extractable with the same pattern the rest of tray.sh uses:

# the path arg is the first formal parameter of the tray function
path_arg=$(grep -oP \
  "async function ${tray_func}\(\K\w+(?=\))" "$index_js")

# the menuBarEnabled local — same shape as patch_menu_bar_default
menu_bar_local=$(grep -oP \
  'const \K\w+(?=\s*=\s*\w+\("menuBarEnabled"\))' "$index_js" | head -1)

Pass them in next to TRAY_VAR / EL_VAR / MENU_FUNC and interpolate.

2. Duplicate tray icon on OS theme change — got a repro?

I've been running KDE Plasma 6 daily and haven't hit the duplicate SNI you're describing. patch_tray_menu_handler already sleeps 250 ms after tray.destroy() for this race (see scripts/patches/tray.sh:56-59), and your learnings doc mentions that delay, so I want to rule out that the existing mitigation is already covering it on most setups before we add a second path.

Happy to believe there's a config where 250 ms isn't enough — Plasma DBus timing drifts a lot between Wayland / X11 and compositor versions — but I haven't run the build with these changes and triggered the race myself, so right now this is static analysis. If you can share KDE / Plasma version, trigger steps, and a screenshot of the two icons, I can confirm on my end. Either way the in-place update is cleaner than destroy + delay + recreate, so I'd just rather land it as "strictly cleaner than the existing mitigation" than "fixes a bug" unless we've got the repro — keeps the commit message and learnings doc honest for whoever hunts for it later.

3. Scope

Separate from anything above: I'm not sold on this repo taking on net-new features versus sticking to Linux compat patches and packaging. Not a code comment — the engineering is solid — it's a precedent question for me and the other maintainers. I've opened #492 for that conversation. Nothing for you to do here; just flagging why I'm not hitting approve.

Non-blocking

  • scripts/tray-icon-settings.js_save() is a single writeFileSync, so a kill mid-write can truncate. _load() falls back to DEFAULT_MODE gracefully so it's soft-fail, but a .tmp + renameSync closes the window if you're already in there.
  • The submenu-strip regex works today because no radio label contains ]. Worth a comment if someone adds a label with nested brackets.
  • .gitignore picked up .idea — benign but unrelated.

Thanks again for the writeup.


Written by Claude Opus 4.7 via Claude Code

IliyaBrook and others added 2 commits April 23, 2026 16:24
…extraction

Expands `patch_tray_inplace_update` in `tray.sh` to dynamically extract key variables (icon path, menuBarEnabled flag) and improve idempotency checks. Refines fast-path logic to update the tray icon in place, avoiding duplicate registrations on KDE Plasma.
- scripts/tray-icon-settings.js: _save() now writes to <path>.tmp
  and renameSync's on top of the real file, so a kill mid-write
  can't truncate the settings file. The failure path also unlinks
  the stray .tmp if the write itself threw.
- tests/tray-icon-settings.test.js: add coverage asserting the
  parent directory has only the final file after a save cycle (no
  leftover .tmp).
- scripts/patches/tray.sh: document the [^\]]*? assumption in the
  submenu-strip regex — relies on no radio label containing a
  literal ']'.

All addressed from @aaddrick's PR aaddrick#491 review.

Co-Authored-By: Claude <[email protected]>
@IliyaBrook
Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — addressing each point below.

1. Blocker: hardcoded t and e — fixed

Addressed in ea7eaaa. patch_tray_inplace_update now extracts two additional names dynamically, using the same pattern as the rest of tray.sh:

  • path_var — the icon-path local used inside the original new Tray(nativeImage.createFromPath(X)) call. Extracted via:
    path_var=$(grep -oP \
      "${tray_var_re}=new ${electron_var_re_local}\.Tray\(${electron_var_re_local}\.nativeImage\.createFromPath\(\K\w+(?=\))" \
      "$index_js" | head -1)
  Anchored on the existing `new Tray(...)` call rather than the function signature — the tray function is parameterless (`async function wle()`) so the "first formal parameter" pattern you suggested wouldn't match here, but the `createFromPath()` form is reliable.

- **`enabled_var`** — same pattern you suggested (identical to `patch_menu_bar_default`):
  ```bash
  enabled_var=$(grep -oP \
    'const \K\w+(?=\s*=\s*\w+\("menuBarEnabled"\))' "$index_js" | head -1)

Both are then substituted into the injected template. The idempotency guard also moved from the literal (t)) off-cursor match to:

fast_path_marker="${tray_var}.setImage(${electron_var}.nativeImage.createFromPath(${path_var}))"
grep -qF "$fast_path_marker" "$index_js"

so it re-keys cleanly if the minifier renames the locals between releases.

Re-verified on the current upstream asar: extraction returns tray_func=wle, tray_var=Nh, electron_var=pA, menu_func=wAt, path_var=t, enabled_var=e — the fast-path is injected correctly, and a second run is a no-op via the guard.

2. Duplicate tray icon on OS theme change — repro

Reproduces for me on:

Distro Fedora Linux 43 (KDE Plasma Desktop Edition)
Plasma 6.6.4
xdg-desktop-portal-kde 6.6.4
Session Wayland (XDG_SESSION_TYPE=wayland)
Kernel 6.19.12-200.fc43.x86_64

Steps (on pristine main, no PR changes)

git clone https://github.com/aaddrick/claude-desktop-debian.git
cd claude-desktop-debian
./build.sh --build appimage --clean no
./claude-desktop-*-amd64.AppImage

Then in System Settings → Colors → Plasma Style, switch the Plasma style to a different variant (e.g. Breeze Light ↔ Breeze Dark) while the app is running.

Expected: the tray icon updates in place with the new colour.
Actual on main: the old StatusNotifierItem stays registered and the new one appears next to it — two Claude icons side by side. Neither survives logout on its own (both are gone on next login, but they persist for the session).

Video — reproduced on main clean cloned into new dir

duplicate_tray_icon_bug_change_theme.mp4

Video — same steps after this PR

duplicate_tray_icon_after_fix.webm

Happy to believe the 250 ms delay in patch_tray_menu_handler is sufficient on some setups — it genuinely might be timing-dependent on compositor version / portal implementation / hardware speed — but on Plasma 6.6.4 + Wayland + Fedora 43 it isn't, and the in-place update sidesteps the race regardless of where the timing line falls.

I'm fine either way on the wording in the commit message / learnings doc. If you'd still prefer "strictly cleaner than destroy + delay + recreate" over "fixes a bug" so the phrasing holds for future readers on whatever setup, I'll happily reword — the videos establish the behaviour on record either way.

3. Scope

Understood — parked, awaiting the outcome of #492. No scope expansion from my side beyond this PR.

4. Non-blocking nits — addressed

All three in 6458f81:

  • Atomic write in _save() — now writes to <path>.tmp + renameSync so a kill mid-write can't truncate the file. Failure path unlinks the stray .tmp. Added a test asserting no leftover .tmp after a save cycle.
  • Submenu-strip regex — added an NB: [^\]]*? assumes no radio label contains ']' comment next to the regex with a pointer to what would need to change if a future label contains one (switch to a balanced-brace match).
  • .gitignore picking up .idea — kept on this branch as 8fd1945. Happy to drop it and ship separately``` if you'd rather have the feature PR contain only feature commits — one word and I'll force-push without it.

Thanks again for the detailed feedback and for keeping the bar high on minifier-resilience.

@IliyaBrook IliyaBrook requested a review from aaddrick April 23, 2026 14:42
@IliyaBrook
Copy link
Copy Markdown
Contributor Author

One more note, in case the scope discussion in #492 tilts against user-facing settings:

The "Icon color" submenu is the only fix I was able to find for the invisible-tray-icon problem on KDE. The learnings doc walks through the three auto-detection approaches I tried — xdg-desktop-portal (org.freedesktop.appearance.color-scheme), kdeglobals [Colors:Window], and the Plasma theme's colors file — and each misses real-world setups (most notably Fedora KDE's default theme with a black panel over a light global colour scheme). Handing the choice to the user was what I landed on after those didn't hold up. If you or the other maintainers see a cleaner automatic path I missed, I'm happy to pivot the implementation; right now this is the only approach I have that actually works on the setups I could test on.

If #492 concludes "no net-new features in this repo," I'd be glad to split patch_tray_inplace_update (the duplicate-tray-icon-on-theme-change fix) into its own PR. That one is unambiguously a bug fix and stands on its own — it's milder than the invisible-icon case (users can work around it by restarting the app) but it's still a bug worth landing independent of the scope decision.

Either way, happy to follow whichever shape makes this easier to land for you.

@IliyaBrook
Copy link
Copy Markdown
Contributor Author

By the way! Just recently I discovered that this icon duplication bug also occurs when, in Claude Desktop itself, we open the dropdown next to the username, click the Appearance button, and change the Theme value in that section 🤔 here's the video:

duplicate_icon_change_theme_in_app.webm

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.

2 participants