Differential D28185

[applet/systemtray] Use model binding instead of copy
ClosedPublic
Actions

Authored by kmaterka on Mar 21 2020, 3:53 PM.

Details

Reviewers
ngraham
broulik
davidedmundson
apol
Group Reviewers
Plasma: Workspaces
Plasma
Commits
R120:17153c4ae204: [applet/systemtray] Use model binding instead of copy
Summary

The effectiveStatus parameter was passed as a copy, use proper binding.

Test Plan

No visible impact

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.Mar 21 2020, 3:53 PM
kmaterka created this revision.
apol added a subscriber: apol.Mar 21 2020, 4:15 PM

I'm not convinced this is all that better.

kmaterka updated this revision to Diff 78178.Mar 21 2020, 4:51 PM

Bind model as well

kmaterka added a comment.Mar 21 2020, 4:51 PM
In D28185#631972, @apol wrote:

I'm not convinced this is all that better.

Without this effectiveStatus never changes. Right now it works by happy coincidence - Item is simple destroyed and created in a different, as a side effect new value of effectiveStatus is used.
This change tries to fix my mistake introduced in D26992. I used QML Loader incorrectly... :(
Anyway, I changed it a little bin, please check now.

kmaterka retitled this revision from [applet/systemtray] Us model binding instead of copy to [applet/systemtray] Use model binding instead of copy.Mar 21 2020, 4:59 PM
kmaterka added a reviewer: apol.
broulik added inline comments.Mar 21 2020, 5:20 PM
applets/systemtray/package/contents/ui/items/AbstractItem.qml
36–37

You can simplify this to

model.status || PlasmaCore.Types.UnknownStatus
apol added a comment.Mar 21 2020, 5:22 PM

I like it better now, +1. Please address Kai's comment.

kmaterka updated this revision to Diff 78181.Mar 21 2020, 5:44 PM

Simplify JS

kmaterka marked an inline comment as done.Mar 21 2020, 5:45 PM
kmaterka added inline comments.
applets/systemtray/package/contents/ui/items/AbstractItem.qml
36–37

Done

ngraham accepted this revision.Mar 21 2020, 6:00 PM
This revision is now accepted and ready to land.Mar 21 2020, 6:00 PM
Closed by commit R120:17153c4ae204: [applet/systemtray] Use model binding instead of copy (authored by kmaterka). · Explain WhyMar 21 2020, 6:12 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 78183

View Options

applets/systemtray/package/contents/ui/items/AbstractItem.qml

Loading...
View Options

applets/systemtray/package/contents/ui/items/ItemLoader.qml

Loading...
View Options

applets/systemtray/package/contents/ui/items/PlasmoidItem.qml

Loading...
View Options

applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml

Loading...