Dynamically show and hide based on whether or not any vaults are configured
ClosedPublic

Authored by ngraham on Jan 5 2020, 11:35 PM.

Details

Summary

Currently, the Vaults icon is always visible in the System Tray. However most tray icons
dynamically show and hide themselves based on whether or not they're relevant and in use,
which results in a nice clean clutter-free System Tray. So it would be nice for Vaults
to do the same.

This patch implements that behavior by making rowCount a property that gets updated
based on vault creation and deletion, and making the icon only appear in the tray when at
last one vault has been created. People who prefer to have it always visible can of course
override this in the System Tray settings, but the auto-hide behavior in this patch is probably
a better default for most.

Test Plan
  1. Have no vaults -> icon is only in the popup
  2. Create a vault -> icon appears in the tray
  3. Delete the vault -> icon disappears from the tray and lives in the popup again

Diff Detail

Repository
R845 Plasma Vault
Branch
dynamically-show-and-hide (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20670
Build 20688: arc lint + arc unit
ngraham created this revision.Jan 5 2020, 11:35 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 5 2020, 11:35 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Jan 5 2020, 11:35 PM
broulik added inline comments.Jan 5 2020, 11:41 PM
plasma/vaultsmodel.cpp
104

You probably want to remember the old count and emit when it's different. The model could have been full and become empty after all.

plasma/vaultsmodel.h
45

If you give parent a default argument you can just use that in your property rather than adding a dedicated count method

ngraham updated this revision to Diff 72840.Jan 5 2020, 11:53 PM
ngraham marked 2 inline comments as done.

Address review comments

ngraham edited the summary of this revision. (Show Details)Jan 5 2020, 11:54 PM

+1

plasma/vaultsmodel.h
36

The property name should stay count to avoid conflict with rowCount which us invokable

45

Coding style

96

Imho this argument isn't necessary but it's probably consistent with the rest

ngraham updated this revision to Diff 72841.Jan 6 2020, 12:00 AM
ngraham marked 3 inline comments as done.

Address more review comments (eventually I'll get this right...)

ivan added a comment.Jan 6 2020, 9:59 AM

For some reason, hiding this icon when there are no vaults was deemed undesired before. I don't recall why as I think it is a good idea. :)

Maybe it was always shown for discoverability purposes... don't know.

Is there a reason why this is a count instead of (or in addition to) just being isEmpty or something similar since the actual count is not used?

In D26447#588486, @ivan wrote:

For some reason, hiding this icon when there are no vaults was deemed undesired before. I don't recall why as I think it is a good idea. :)

Maybe it was always shown for discoverability purposes... don't know.

Probably that, though VDG discussion on the matter was strongly negative on the idea of putting more things in the system tray's visible section for discoverability purposes.

Is there a reason why this is a count instead of (or in addition to) just being isEmpty or something similar since the actual count is not used?

It was a smaller code change than adding a new isEmpty property, and I suppose exposing count as a property could be useful in other contexts too.

why this is a count

It's only consistent with how we usually do it elsewhere, and I think for a list model having a count in general makes more sense/is more generic/more useful than an isEmpty property

ivan accepted this revision.Jan 7 2020, 2:35 PM

That is why I put the "or in addition to" part.

Anyhow, I don't mind this to go in as is.

This revision is now accepted and ready to land.Jan 7 2020, 2:35 PM
This revision was automatically updated to reflect the committed changes.