D17796: Plasma Workspace. Fixed general settings of the widgets.

[Pavel Mos]

pavelmos created this revision.
pavelmos added projects: Plasma: Workspaces, Plasma (Plasma 5.14).
Herald edited projects, added Plasma; removed Plasma (Plasma 5.14), Plasma: 
Workspaces.
Herald added a subscriber: plasma-devel.
pavelmos requested review of this revision.

REVISION 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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D17796

AFFECTED FILES
  applets/systemmonitor/common/contents/ui/ConfigGeneral.qml

To: pavelmos
Cc: plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17796: Plasma Workspace. Fixed general settings of the widgets.

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  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"

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D17796

To: pavelmos, jriddell, #plasma, ngraham
Cc: ngraham, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17796: Plasma Workspace. Fixed general settings of the widgets.

[Pavel Mos]

pavelmos updated this revision to Diff 48278.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17796?vs=48186&id=48278

REVISION DETAIL
  https://phabricator.kde.org/D17796

AFFECTED FILES
  applets/systemmonitor/common/contents/ui/ConfigGeneral.qml

To: pavelmos, jriddell, #plasma, ngraham
Cc: ngraham, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17796: Plasma Workspace. Fixed general settings of the widgets.

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  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.

REVISION DETAIL
  https://phabricator.kde.org/D17796

To: pavelmos, jriddell, #plasma, ngraham
Cc: ngraham, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17796: Plasma Workspace. Fixed general settings of the widgets.

[David Edmundson]

davidedmundson added a comment.


  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?

REVISION DETAIL
  https://phabricator.kde.org/D17796

To: pavelmos, jriddell, #plasma, ngraham
Cc: davidedmundson, ngraham, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart