element-desktop
element-desktop copied to clipboard
element-hq
Make the tray icon blink on unread messages
This pull request (together with appropriate PRs in two other repos) adds a new option and functionality, allowing the user to enable tray icon blinking when there are unread messages pending. Blinking stops as soon as there are no unread messages or if the option is explicitly turned off in the settings. Our users sometimes miss the messages because the red circle with a number doesn't attract their attention. We used Gajim (an XMPP client) before and its icon blinks on new messages so they always notice it. This option is off by default so it's completely opt-in.
What it looks like: https://www.youtube.com/watch?v=1n2LEZqwZRw
Signed-off-by: Sergey Shpikin [email protected]
Fixes https://github.com/vector-im/element-web/issues/15017 Depends on https://github.com/vector-im/element-web/pull/15015 Depends on https://github.com/matrix-org/matrix-react-sdk/pull/5135
For changes like this is it often useful to have an issue open to collect user opinions and feedback, including from the Element Product team which decide if this is something that they are wanting in the product.
@vector-im/product is this something we want? Flashing icon opt-in Won't work on Mac, won't work on certain Windows configurations: e.g https://github.com/vector-im/element-desktop/issues/762
Needs DCO sign-off as per the CONTRIBUTING doc: https://github.com/matrix-org/matrix-js-sdk/blob/master/CONTRIBUTING.rst#sign-off
This is a big issue for us, without this flashing icon it's hard to justify the switch to Element from XMPP (as ridiculous as it may sound). If people miss the messages and don't read them it's a problem. But I understand if it's not something you as a company want or need in your product, in that case we'll maintain our own fork.
Regarding vector-im/element-desktop#762, I don't think it's related. What I see in the screenshots is the taskbar button (not tray icon) not being in sync with the window icon. On Win10 there's a known problem with the tray icon disappearing after every update (vector-im/element-desktop#786) but that can be worked around by turning off tray collapsing. And even then people don't always notice there are new events they missed.
What I see in the screenshots is the taskbar button (not tray icon) not being in sync with the window icon.
Ah yes this does not affect the Window icon, that might feel inconsistent. Also in that case the setting should surely be disabled if the tray icon is disabled;
I created an issue for this feature (referenced above).
I'm not sure if we should instantly turn this setting off if the user turns the icon itself off. The setting becomes disabled (which is logical) but I think it's okay to keep its state if the user decides to enable the icon later. I saw this behavior in many other programs when dependent setting becomes inaccessible but it doesn't change its state just because of that. I sure wouldn't want my other settings to change if I change some parent setting that makes them non-functional (it might be a temporary switch after all) but it's debatable.
@rkfg could you expand on this PR with a screen cap of the behaviour please to aid review?
Also following some internal discussion, we're not sure about the wider utility of this for our entire user base so wouldn't like to merge it in as the default behaviour. Please can you update it to be behind a labs flag?
@nadonomy added a video to the first post. I'll take a look at the labs flags shortly.
@nadonomy added a video to the first post. I'll take a look at the labs flags shortly.
Thanks, the video really helps! I'm concerned the blinking is too aggressive and that users would find it fatiguing when multi tasking. What could we do to ease that? Aside from increasing the duration, could we leave a longer duration between blinks?
The duration is the same between appearing and disappearing, I chose 500 ms to match the Gajim behavior but it can be longer of course.
I haven't yet looked into the labs flags, was busy with my other project. I believe it's easy enough to implement so let's put it aside until there are no other issues with the feature.
@rkfg I added myself as a reviewer on this, if you could explicitly mention me in a comment when there's some iteration based on this comment then I can take a look at it then!
@nadonomy ah okay, I thought you have some thoughts about the exact durations. I set them to 2000ms, is it better now? (the second blink is shorter because of incorrect looping of the GIF, the first one is what it really looks like)
@nadonomy anything else I should change/fix? I think we'll begin testing this patch on our users pretty soon!
@rkfg apologies missed this, how's testing going? Happy to merge this once behind a labs flag & the other comments are resolved.
@nadonomy we haven't tested it widely yet, there's another blocking issue with the tray icon becoming hidden after every update (vector-im/element-desktop#786, Squirrel/Squirrel.Windows#863). I worked around it with a hacky utility I wrote that patches the registry and restarts explorer so the icon becomes visible again but it requires deployment on every PC and we're a bit busy already due to the pandemic.
The only unresolved comment left is this one and I haven't got a reply. I know it's not ideal but it works and so far I'm not aware of a better solution.
If the only thing that's left is the labs flag, good! I'll try to implement it in the near future.
@nadonomy The new setting is now at the labs page, see matrix-org/matrix-react-sdk#5135
Known bug: setting the labs setting to true or "enabled" doesn't actually enable blinking (only the setting switch itself). So far only manual toggling is supported. The reason is that this seems to be the only labs setting that needs its value to be stored in the desktop layer. Because of that there's a setting duplication: the labs switch gets and sets its value in the local storage under the key mx_labs_feature_feature_trayicon_blink but when the switch is changed the accompanying controller issues an ipc call to the platform layer to change the setting there as well. Settings loading doesn't trigger SettingsStore.setValue (which calls SettingController.onChange) and thus the ipc call setTrayIconBlinkEnabled never happens.
I couldn't find any similar desktop-only features to get some ideas about solving this puzzle so I left it as is. Unfortunately, it makes the feature slightly less useful because it can't be automatically enabled for all clients via the config file so if anyone knows how to pass a setting from the config to the desktop layer I'll gladly implement it.
For the moment, I'm going to temporarily clear code review requests on this series, as I believe we need to first make past the Product review here, and then we can dive into the code after that.
Closing as rejected by product, see https://github.com/matrix-org/matrix-react-sdk/pull/5135#issuecomment-1513430453