[System Tray] Unified data model for System Tray items
ClosedPublic

Authored by kmaterka on Nov 27 2019, 9:10 PM.

Details

Summary

Currently there are two different sources of SystemTray items:

  • Plasmoids
  • Status Notifier

Thi change adds new model that holds both Plasmoids and SNIs.

Test Plan

Add/disable applets. Start/stop SNI app.

Diff Detail

Repository
R120 Plasma Workspace
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19670
Build 19688: arc lint + arc unit
kmaterka requested review of this revision.Nov 27 2019, 9:10 PM
kmaterka created this revision.

This is a simplified version of D23413 needed for the D22176 to move forward.

Looks very good in general. A few style nitpicks

applets/systemtray/package/contents/ui/ConfigEntries.qml
68–72

You can try doing

for (item of systemTrayModel) {
]

But I don't know if that will work

applets/systemtray/systemtraymodel.cpp
61

roles.insert(ItemType, ...) should be enough

75

This could be a candidate for std::find_if

154

if (contains) {

return stuff

}

otherStuff()

applets/systemtray/systemtraymodel.h
78

What's the purpose of the *Changed roles?

kmaterka marked 5 inline comments as done.Dec 10 2019, 9:11 PM
kmaterka added inline comments.
applets/systemtray/package/contents/ui/ConfigEntries.qml
68–72

Unfortunately model is not iterable. This code will be removed in D22176 anyway.

applets/systemtray/systemtraymodel.cpp
61

I'm using enum class instead of simple enum, so explicit cast is required.

75

It would be nice, but does QStandardItemModel have an iterator?

154

OK, changed.

applets/systemtray/systemtraymodel.h
78

All of these roles are copied from StatusNotifierItemSource. It might be an overkill to support all.
I don't know exactly what was the intention to use "*Changed" roles, there is only one comment: // record what has changed. I found usages in old KDE4 code:

./kde4/kde-workspace/plasma-workspace/applets/systemtray/plugin/protocols/dbussystemtray/dbussystemtraytask.cpp

void DBusSystemTrayTask::dataUpdated(const QString &taskName, const Plasma::DataEngine::Data &properties)
{
    // ....
    if (properties["TitleChanged"].toBool() || becomeValid) {
        QString title = properties["Title"].toString();
        // ...
    }
    // ...
kmaterka updated this revision to Diff 71231.Dec 10 2019, 9:11 PM
kmaterka marked 5 inline comments as done.

Review fix

kmaterka updated this revision to Diff 71232.Dec 10 2019, 9:18 PM

Review fix

This revision is now accepted and ready to land.Dec 15 2019, 9:29 PM
This revision was automatically updated to reflect the committed changes.

Please make sure it would not bring this bug back: https://phabricator.kde.org/R120:6fcf9a5e03ba573fd0bfe30125f4c739b196a989

I double checked that. If I understand correctly the root cause of BUG 393630 was the use of plasmoid.rootItem - it is not introduced again.