On Thu, Aug 1, 2019 at 10:30 AM Konrad Materka <mate...@gmail.com> wrote: > > Hi, > > recently I was fixing two bugs related to SystemTray (in review: > https://phabricator.kde.org/D22804, > https://phabricator.kde.org/D22767). I noticed that code is highly > coupled and "fragile" with some unclear references like: > item.parent.parent.attrbute etc. Items are imperatively assigned to > different parents depending on the status.
Firstly, thanks ever so much for looking into the xembedsniproxy code! It's good to have someone active on it. (note that I have a super minor review comment on one, then let's ship it ASAP) > > I would like to add one meta-model with all items (both > SystemNotifications and plasmoids) plus Delegate Model(s) with groups: > * active > * passive/hidden > * disabled etc > and additional sorting: > * notification always as first > * other rules possible Seems sane. If you use actual models, see KExtraRowsProxyModel for merging. > > That model will allow: > * better separation between data and view, it is additional abstract > layer between backend data (SystemNotifications model + plasmoids > list) and current view. > * ability to filter/sort/group easily - no need for > plasmoid.nativeInterface.reorderItemAfter > * clear information about the model used in active/hidden containers > * easier implementation of new features like ordering, even with drag > and drop (https://bugs.kde.org/show_bug.cgi?id=384782) > * decoupled code (with further refactoring, preferably not in one step) > no direct parent assignment. > > The biggest disadvantage (?) I see is that it adds some complexity, > change is big and will require quite long review process. > I predict it will take around 4 weeks + 4 weeks (or more) for reviews/fixes. > > What do you think? Should I start work or someone else is working on SystemTray? Go for it. You might find it useful to review Marco's new containment management plugin for the desktop. It has a controls like approach for creating loaders. There should be some crossover which might be relevant. > -- > Regards, > Konrad Materka
