Add tray_icon_path to config to allow overriding tray icon

[3nprob]

Checklist

• Ensure your code works with manual testing
• Linter and other CI checks pass
• Sign-off given on the changes (see CONTRIBUTING.md)

Adds new config entry tray_icon_path, allowing the user to override the tray icon with their own.

This resolves #774

Use-cases:

• Custom theming (Easy branding of Riot interface - application name, icon, logo and colors element-web#4745)
• Allow separating icons for different profiles (Support for multiple matrix accounts element-web#2320 (comment))

Also minor fix in removing redundant global variable iconPath.

Type: enhancement

---

Here's what your changelog entry will look like:

✨ Features

• Add tray_icon_path to config to allow overriding tray icon (#396). Fixes Element desktop doesn't show which profile is being used in the title bar or tray icon bar #774. Contributed by @3nprob.
  Type: enhancement

[3nprob]

Signed-off-by: 3nprob <[email protected]>

[3nprob]

OT but since there is no issue tracker on this repo to file a meta issue: the CONTRIBUTING.md link in the PR template is 404ing.

[3nprob]

Tested that it works fine to use a path from the local filesystem by editing config.json in the profile directory.

[t3chguy]

The tray icon gets overridden by the app favicon when you get a notification, so this would cause your icon to flicker between custom and element, no?

[t3chguy]

@3nprob what OS did you test on? We only don't use the favicon override on macOS, where the OS has stable notification bubble count support so we can skip it

[3nprob]

@t3chguy Linux(Wayland). No flicker noticed yet.

[t3chguy]

https://github.com/vector-im/element-desktop/blob/61ed2a21c68f6d41fb57d103db6e025779eafcc7/src/tray.ts#L64-L93 is the bit responsible

[NathanC]

I actually looked up this repo specifically to see if there were any open issues about this, very happy to see it added. Would this support svgs?

[t3chguy]

Would this support svgs?

That probably depends on your OS, Electron docs don't specify other than a recommendation to use ico on Windows

[3nprob]

Hmm, with the new changes forcing filetype per platform no, svgs won't wok anymore even if supported. Is the platform-detection functionality for manual icons that vital @turt2live ?

[turt2live]

Well, it needs to work on all the platforms. Windows wants an ico, so we should be giving it an ico.

[turt2live]

Product team: Do we want to support this feature? (assuming it can be made to work when the user receives a notification)

[3nprob]

We also need to consider @t3chguy's comment about the icon changing when the user gets a notification. I don't really foresee that being an easy fix, however.

Could this be addressed in a separate issue perhaps? I can't seem to reproduce this yet (ie for me, the icon is always shown to the user-configured one, notifications or not, no flickering). Been running this in 3 concurrent instances for the past week fwiw.

[turt2live]

It would need solving for us to feel comfortable landing this PR, so I don't think it can be safely broken out to another issue, sorry. Some platforms might magically work but it would need extensive testing on all platforms we provide direct support for.

[3nprob]

@turt2live All right, let's hope someone else can help with this.

I'm surprised this isn't considered prioritized considered element-hq/element-web#2320 is still highly relevant (most commented and upvoted issue ever and it's not getting better over time).

The fix in this PR takes the one proposed workaround (profiles) from a painful hack (which it becomes when combined with the fact that frequent client freezes from other issues forces force-closes, which makes it a russian roulette on which tray icon to close each time) to actually usable.

This one issue is enough of a pain-point that it's hurting the organizational buy-in on the whole Matrix adaptation. In lack of another suitable client and support for this we'll probably have to start maintaining a fork, which I don't look forward to.

[turt2live]

Multiple accounts is indeed something we'd like to support one day. In the meantime, we require that features be complete in order to reduce maintenance burden of the app.

[3nprob]

So @t3chguy is saying "We only don't use the favicon override on macOS, where the OS has stable notification bubble count support so we can skip it".

Can we confirm which scenerios/platforms trigger the flickering and what changes are needed?

I'd at least like to figure out if this is a Windows-only issue or if there is any way for me to reproduce it

[3nprob]

There is now a check that disabled the webview-sourced icon override (that is, making non-mac platforms behave like mac) in case the tray icon has been explicitly configured. With the default, the previous behavior is retained.

[turt2live]

Please avoid force pushes after initial review has been given - it makes it much harder to review the code.

I would also suggest holding off on writing too much code before the Product team has a look. There's a chance that they will not approve the feature (which is why we're asking for their review), and we'd rather not have to throw away too much code.

[3nprob]

I would also suggest holding off on writing too much code before the Product team has a look. There's a chance that they will not approve the feature (which is why we're asking for their review), and we'd rather not have to throw away too much code.

Hm, I thought I had done my prework here but I guess I must have misinterpreted something.

#774 is Triaged (which I take to indicate that it's at least not been closed as "irrelevant"/"wontmerge"), recently enough that I think it's reasonable to assume it's not a remnant of past dreams.

element-hq/element-web#4745 is accepted and AFAICT all parts of it but this has already been implemented and exposed in user-accessible ways.

Where can one read up to better understand the decision-making process when deciding what features can be considered for merging into the develop branch? In a similar situation for the future, how could one preemptively get an indication before setting off on an implementation given the already existing issues?

[turt2live]

I'll re-review this after Product takes a look.

This PR already has more comments than it honestly needs to: let's take process discussion to #element-dev:matrix.org on Matrix please.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

3nprob seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[t3chguy]

Could this be addressed in a separate issue perhaps? I can't seem to reproduce this yet (ie for me, the icon is always shown to the user-configured one, notifications or not, no flickering).

So you see your custom icon with the numbered dot in the corner when a notification is shown? If not then likely your notification settings are such that this won't happen but with default settings it would at least for mentions/replies.

[t3chguy]

We had a similar change land grayscale icons on Linux and it suffered from the same issue. We are willing to accept this change without explicit Product sign-off if it was free of the same bug. Closing for now, but you can continue to iterate on that branch and let us know when the issue is fixed (it'll need a companion Element Web PR as that's where the notification dot overlay is generated)