[SystemTray] Use unified data model everywhere
ClosedPublic

Authored by kmaterka on Jan 29 2020, 3:33 PM.

Details

Summary

Use the unified data model everywhere, not just in configuration. This simplifies UI code, separetes presentation from data.
This requires the use of ListView and GridView.
This change enables the implementation of more advanced sorting algorithms for systemtray items.

Test Plan

Affects almost all areas of system tray.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kmaterka requested review of this revision.Jan 29 2020, 3:33 PM
kmaterka created this revision.
kmaterka added a comment.EditedJan 29 2020, 3:36 PM

This change is very big. I can try to split in into two (maybe more) if that's required. If yes, should I connect revisions somehow? Create task, add parent/child revision?

kmaterka updated this revision to Diff 74805.Jan 31 2020, 3:39 PM

I will split it into smaller changes, this one is too big for a review. I'll keep it here as a backup.

The patch is big, but it's manageable.

Splitting is up to you, don't feel you have to.

First revision: D27085. In contains some random improvements

... and second one: D27088. It contains model refactoring and sorting.

After that two are accepped I will rebase this one.

kmaterka updated this revision to Diff 75134.Feb 6 2020, 7:44 PM

Rebase, only D27088 left

kmaterka updated this revision to Diff 76028.Feb 19 2020, 9:36 PM

Second (and last) change extracted from this one is in master. Rebased, ready for review.

UI looks good except for one thing: the changes to show a highlight effect on hover in the popup are unrelated and should be done in a separate patch (preferably in coordination with VDG folks since we're moving towards the idea of using this in other places too).

UI looks good except for one thing: the changes to show a highlight effect on hover in the popup are unrelated and should be done in a separate patch (preferably in coordination with VDG folks since we're moving towards the idea of using this in other places too).

This change should not introduce any visual change but one: add transition (main.qml, line 170).
I guess you are referring to HiddenItemsView.qml, are you? I had to change Column to ListView to support model so almost everything changed or moved. CurrentItemHighLight and PlasmaComponents.HighLight was there from the very beginning.

My bad, your patch actually fixes a pre-existing visual bug. :) That accounts for the change. +1 visually

ngraham accepted this revision.Wed, Mar 4, 5:32 PM

UI and interaction are good, and a basic code review reveals no issues to me. Still, you may want to wait for a more thorough code review from a Plasma person.

This revision is now accepted and ready to land.Wed, Mar 4, 5:32 PM
davidedmundson accepted this revision.Fri, Mar 6, 2:32 PM
davidedmundson added inline comments.
applets/systemtray/package/contents/ui/items/AbstractItem.qml
34–35

Lets look at fixing in a follow up patch

QML can do enums now

kmaterka marked an inline comment as done.Sat, Mar 7, 10:19 AM
kmaterka added inline comments.
applets/systemtray/package/contents/ui/items/AbstractItem.qml
34–35

OK, I will change that in a separate patch

This revision was automatically updated to reflect the committed changes.
kmaterka marked an inline comment as done.