[Applet] Show in system tray iff at least one vault is open
AbandonedPublic

Authored by ksmanis on Apr 23 2020, 1:24 PM.

Details

Reviewers
ivan
Group Reviewers
Plasma
VDG
Summary

Currently, the "Shown when relevant" visibility option in the system
tray shows an icon iff at least one vault exists, regardless of open
status (i.e., if it is open or closed). Change this behavior to show an
icon iff at least one *open* vault exists, allowing the user to be more
mindful of potentially sensitive information being accessible.

The previous behavior can be emulated by selecting the "Always shown"
visibility option, assuming that the user does not create and delete
Plasma Vaults for recreation.

Test Plan

Compiles and runs as expected both on master (commit 4d11dcf) and
Plasma/5.18 (commit 1f546c1).

Diff Detail

Repository
R845 Plasma Vault
Branch
applet-systray-relevance (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25760
Build 25778: arc lint + arc unit
ksmanis created this revision.Apr 23 2020, 1:24 PM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 23 2020, 1:24 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ksmanis requested review of this revision.Apr 23 2020, 1:24 PM
ngraham added a subscriber: ngraham.

With this change, a person who creates a Vault they will be regularly using and wants the system tray icon to be always visible now has to go out of their way to use the "always visible" setting in the system tray settings; the "Shown when relevant" setting will make the tray icon hidden when they want to go and access the vault.

I guess it depends on whether your definition of "relevant" means "the user has created at least one vault" or "the user is actively using a Vault right now".

The latter definition is more consistent with the visibility status of the Printers applet, which shows itself only when a printer is in use, not when a printer is simply connected. However it would be inconsistent with the Bluetooth applet, which currently behaves in the same way as the Vaults applet does (the tray icon is visible when Bluetooth is on, not only when a device is connected). And I worry that it will be a bit annoying for people who regularly use Vaults, as it means they will have to access the item in the System Tray's expanded pop-up or change the default settings. I use Vaults quite frequently; if this change lands, I would change the default visibility setting to "Always visible"

Thoughts from VDG and Goal: Consistency people?

Accurate analysis. If I might add, as an infrequent user of Plasma Vaults, I am stuck between an (almost) always visible applet (whether I select "Always shown" or "Shown when relevant") and an always hidden applet. A "currently active" relevance would cater for both frequent and infrequent users, with the drawback that frequent users would have to select "Always shown". I'm not particularly ecstatic about the Bluetooth applet taking up unnecessary space 95% of the time, either.

Imho, in KDE's configurable spirit, I would prefer it if systray relevance was configurable, but to my knowledge no plasmoid provides such an option; the creator is always in charge of making this decision with potential inconsistent results, as mentioned.

Yeah. Thinking about it more, your proposal makes some sense because people like you can currently not get what you want at all, while if we integrate this change, people like me can just change the visibility setting to "Always visible". So I'm leaning in the direction of approving.

However if we really want to overthink things, we could go deeper and add more visibility states to satisfy everyone. We could have the following five states for visibility:

  1. Disabled
  2. Always hidden in pop-up
  3. Shown when there are any devices/services (as appropriate for the applet) configured
  4. Shown when in use
  5. Always shown

#3 would only be visible for applets which can have devices/services/etc added and configured: Bluetooth, KDE, Printers, Vaults Connect

Thoughts?

I gave it some thought and I would be reluctant to introduce one more fuzzy (as in ambiguous) state.

More specifically, right now, we have four states: "Shown when relevant", "Always shown", "Always hidden", and "Disabled". Without having gone through the source code, I can guess what the latter three mean, but "Shown when relevant" is context-specific (and the reason this patch came to be). If we introduce another state, let's call it "Show when configured", we will just exacerbate the problem, because it is not immediately clear what it means. For instance, for the Bluetooth applet, does "configured" mean that I powered my adapter or maybe that I configured at least one device? As a user, both of them make sense to me, but again they are context-specific. Therefore, if we want to move in this direction of pleasing everyone, I suggest we delegate this responsibility to the applet itself where the context is clear. In this way, the Bluetooth applet could define relevance (in "Shown when relevant") as: "Show when adapter is powered", "Show when devices are configured", or "Show when devices are connected". This could be done in the applet's config dialog without breaking the current "interface" (i.e., the four current states).

If we want to keep things simple, I suggest we make the status quo consistent by defining relevance as a dynamic event that requires the user's attention. In the context of the Bluetooth applet, that would mean a device was connected. In the context of vaults, that would mean I opened one of them. Having configured a Bluetooth device or having created a vault is pretty much a steady state for most users (as in you only do it once); I don't think anyone goes around setting up things over and over again. In that case, we are pretty much in the "Always shown" domain.

tl;dr:

  • Do you want to keep things simple? Maintain the status quo but iron out any inconsistencies.
  • Do you want to accommodate all needs? Delegate to the applets where the context is clear.
broulik added inline comments.
plasma/vaultsmodel.h
36

I think this should be openCount to match count.

IMO it is the best not to add new status in System Tray - it is already quite confusing with "Disabled" state, the 3 other (Always hidden, Shown when relevant and Always shown) are OK and self-explanatory.
"Shown when relevant" might be tricky and we will never please everyone. As stated in previous comment, this is tricky.

Currently the decision is in the developer's hand only. There can be a configuration option, but too many options is also confusing. For configuration option, there additional questions: how to describe it with simple label? will users understand the purpose? what if applet is standalone (not in systemtray)? will user ever use it? is it easily discoverable? etc. If someone doesn't like default behavior it is always possible to force Shown/Hidden state. After all, it is not that users delete all vaults and creates new one on everyday basis.

Personally I like the idea to hide Vaults icon when no vault is opened. I have one configured, but I use it very rarely, so for me it is better :) From the other side, Bluetooth and Network icons are visible not only when something is connected but also when connection is possible (Bluetooth/networking is enabled). So maybe it would be more consistent if we leave it as it is now, wouldn't it? For someone who is using vaults often hiding Vaults icon might be really annoying. I don't know what's the best...

BTW. For reference, MS Windows tries to hide as much as possible. But they are trying to kill system tray, so sometimes it is taken to the extreme and annoying.

ivan added a comment.EditedApr 23 2020, 9:03 PM

I'm not fond of this change, but I agree with Nate's comment about the possibility of setting 'always visible' for users that wish to keep the current behaviour, so I'm not going to give this a -1.

I'd still like the current behaviour kept, maybe with a different icon to show when there is a vault that is currently open to remedy one of the reasons behind this patch.

But, again, this is a personal preference and I'm not going to block the patch if everybody else decides it is a good idea.

p.s. Having applets define different levels for showing for themselves might be a good idea.

(If this patch lands, I'll set it to "Always visible" because I use my vault a lot)

ivan added a comment.Apr 23 2020, 9:26 PM

And I'l just be using my keyboard shortcut ;)

It seems to me that this revision does not have the required momentum to move forward. I'll keep it open for another week in case someone else wants to chime in, but unless something changes I'll abandon it and maintain it off tree for personal use.

ksmanis abandoned this revision.May 8 2020, 4:54 PM