Show accurate checked state for system monitor configuration pages
ClosedPublic

Authored by pavelmos on Dec 25 2018, 12:43 PM.

Details

Summary

Fixed general settings of the widgets "Memory Status", "Hard Disk Space Usage", "Network Monitor", "Hard Disk Monitor".

  • correctly set checked/unchecked state on load.

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.
pavelmos created this revision.Dec 25 2018, 12:43 PM
Restricted Application edited projects, added Plasma; removed Plasma (Plasma 5.14), Plasma: Workspaces. · View Herald TranscriptDec 25 2018, 12:43 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
pavelmos requested review of this revision.Dec 25 2018, 12:43 PM
pavelmos added a reviewer: jriddell.
ngraham requested changes to this revision.Dec 26 2018, 6:05 PM
ngraham added a subscriber: ngraham.

As far as I can tell, this doesn't work: the listed widgets still have all their configuration pages' checkboxes unchecked after checking some of them, closing the window, and re-opening it. Am I missing something?

Also please change the title to something more descriptive, such as "Show accurate checked state for system monitor configuration pages"

This revision now requires changes to proceed.Dec 26 2018, 6:05 PM
pavelmos updated this revision to Diff 48278.Dec 28 2018, 9:42 AM
ngraham accepted this revision.Dec 28 2018, 3:04 PM

There we go, it works now! Code change looks sane to me.

Please change the title to something more descriptive, such as "Show accurate checked state for system monitor configuration pages". And then we'll wait for a review from a Plasma developer.

This revision is now accepted and ready to land.Dec 28 2018, 3:04 PM

The call on line 141 looks superfluous - if there's any change to checked we modify cfg_sources which should trigger the reload.

Can you confirm?

pavelmos retitled this revision from Plasma Workspace. Fixed general settings of the widgets. to Show accurate checked state for system monitor configuration pages.Dec 29 2018, 8:58 AM

There we go, it works now! Code change looks sane to me.

Please change the title to something more descriptive, such as "Show accurate checked state for system monitor configuration pages". And then we'll wait for a review from a Plasma developer.

Okay. Thank you!

I have no commit access.

broulik added a subscriber: broulik.Feb 8 2019, 9:04 AM

The call on line 141 looks superfluous - if there's any change to checked we modify cfg_sources which should trigger the reload.

QML cannot detect changes within a JS structure, if you push() into an Array or modify members of an Object it doesn't see the change and the automatism for onCfg_...Changed doesn't trigger

It seems to me that everything is fine.
Are there any other comments or questions?

We need a Plasma review first. Calling all Plasma devs! :)

pavelmos set the repository for this revision to R120 Plasma Workspace.
davidedmundson accepted this revision.Tue, Apr 9, 1:45 PM
This revision was automatically updated to reflect the committed changes.