Rework system tray settings
Needs ReviewPublic

Authored by nicolasfella on Jun 30 2019, 6:04 PM.

Details

Reviewers
None
Group Reviewers
Plasma
VDG
Summary

The current system tray settings has two pages with two slightly different lists. The first page shows a list of all available plasmoids and allows to en/disable them.
The second list shows the in the previous page enabled plasmoids plus the available statusnotifieritems and offers configuration (visibility and shortcuts for plasmoids).

This patch combines both into a single list. Therefor it:

  • Has a new model that holds both plasmoids and SNIs
  • Replaces the QQC1 Tableview with a ListView + Rowlayout
  • Adds an entry 'Disabled' to the visibility combobox to disable plasmoids (functionality from the former first list)
  • Removes the old 'General' page

This removes the ability to (de-)select entire categories, but I don't consider this a particularly useful feature. The categories are intransparent to the user and the individual elements can still be en/disabled

Currently it has a slight visual bug. The scrollbar overlaps the shortcut clear buttons. Help to resolve this is welcome

BUG: 360307
BUG: 397950

Test Plan

Open system tray. Change the visibility of a plasmoid. Apply. Verify visibility in system tray. Add a shortcut to plasmoid. Verify shortcut

Diff Detail

Repository
R120 Plasma Workspace
Branch
systray
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13439
Build 13457: arc lint + arc unit
nicolasfella created this revision.Jun 30 2019, 6:04 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJun 30 2019, 6:04 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
nicolasfella requested review of this revision.Jun 30 2019, 6:04 PM
nicolasfella edited the summary of this revision. (Show Details)Jun 30 2019, 6:04 PM

Nice!!!

Let's give the scrollview that holds this list a visible frame, like we did in the Purpose job dialog. Also maybe rename the "General" tab to say "Entries" since that's all there is there.

Nice, looks good and is a lot cleaner.

I'm mostly only echoing @broulik here, but I think we should still have some sort of help with figuring out which entry belongs to which row of buttons.

This (mockup) is something that would work, but is pretty hacky and is undocumented style:

Nice, looks good and is a lot cleaner.

I'm mostly only echoing @broulik here, but I think we should still have some sort of help with figuring out which entry belongs to which row of buttons.

This (mockup) is something that would work, but is pretty hacky and is undocumented style:

The generally accepted way to do this is to use alternating background colors. The old table view got this automatically, however this patch ports away from it since it's QQC1 only. Unfortunately, there is no QQC2 TableView.

Honestly I would support staying with the QQC1 TableView for now. Trying to move to QQC2 without a replacement for it just means we're rolling our own table view, which seems even hackier.

The generally accepted way to do this is to use alternating background colors. The old table view got this automatically, however this patch ports away from it since it's QQC1 only. Unfortunately, there is no QQC2 TableView.

Honestly I would support staying with the QQC1 TableView for now. Trying to move to QQC2 without a replacement for it just means we're rolling our own table view, which seems even hackier.

@nicolasfella does the above sound reasonable?

Honestly I would support staying with the QQC1 TableView for now. Trying to move to QQC2 without a replacement for it just means we're rolling our own table view, which seems even hackier.

+1, we've already stuck with QQC1 TableView elsewhere.

@nicolasfella would you be able to move forward with this? I have some additions in mind for the config UI but I don't think it makes to start work on them while this very desirable cleanup is still in flight.

kmaterka added a subscriber: kmaterka.EditedAug 24 2019, 11:00 PM

@nicolasfella Please correct me if I'm wrong. EntryModel is created on the first use and it is populated in the constructor. If user runs application that uses StatusNotifier (for example Skype, Dropbox, Electron apps, any Windows app via Wine, etc) later, it won't be added to configuration list.

I was working on a very similar feature:
https://phabricator.kde.org/D23413
It is used for rendering so it has to have dynamic update implemented. Right now it is not suitable for configuration because it contains only items to be rendered (active). It should be very easy to add disabled items. Then one model can be used for both configuration and rendering, which would be great!

@nicolasfella I updated D23413, now it can be used in configuration entries. Can you check if you can use this?

I like the idea of removing "Categories", these are confusing and probably no-one uses them.
Important thing, if we remove categories, then we need to remove:

  • check in AbstractItem.qml
  • "shownCategories" in main.qml
  • probably small cleanup in few other places.

If not, user won't see items, as they won't be able to re-enable categories again.

Honestly I would support staying with the QQC1 TableView for now. Trying to move to QQC2 without a replacement for it just means we're rolling our own table view, which seems even hackier.

In Qt 5.12 TableView is added to QtQuick module, in the similar way as ListView. There is no plan to add it to QQC2. I don't know how difficult migration is or how complete the implementation in Qt 5.12 is (missing header support?).

@nicolasfella I updated D23413, now it can be used in configuration entries. Can you check if you can use this?

That makes sense, thanks

I like the idea of removing "Categories", these are confusing and probably no-one uses them.
Important thing, if we remove categories, then we need to remove:

  • check in AbstractItem.qml
  • "shownCategories" in main.qml
  • probably small cleanup in few other places. If not, user won't see items, as they won't be able to re-enable categories again.

ack

Honestly I would support staying with the QQC1 TableView for now. Trying to move to QQC2 without a replacement for it just means we're rolling our own table view, which seems even hackier.

In Qt 5.12 TableView is added to QtQuick module, in the similar way as ListView. There is no plan to add it to QQC2. I don't know how difficult migration is or how complete the implementation in Qt 5.12 is (missing header support?).

I think QQC1 TableView and the new TableView are quite different things. I had some issues with the QQC1 TableView and my new Model, so I decided to use something else, but I can revisit this

I think QQC1 TableView and the new TableView are quite different things. I had some issues with the QQC1 TableView and my new Model, so I decided to use something else, but I can revisit this

I'm not insisting on TableView, I'm not even a KDE developer so don't listen to me :) I also had problems with QQC1 TableView. modelData property is not available if you use data model directly. It is undocumented property (in fact it is documented, but not directly), available only if you use List/array of objects. That's why in D23413 I'm rewriting model to an array of objects. Ugly, but works with minimal effort.

New TableView is very different but at least it correctly manages the model. I didn't try it yet, but most probably is is not ready for use yet. AFAIK it lacks header support, no sorting, ordering etc, no automatic banded rows etc. In other works it is not a component, but a base for it. It would be great to have reusable TableView component (based on new TableView) in KDE, if someone is willing to implement one :)