<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/113659/">http://git.reviewboard.kde.org/r/113659/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 5th, 2013, 7:39 p.m. UTC, <b>Eike Hein</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is a promising first start. Indeed, the System Monitor applet didn't implement configChanged(), causing scripting its configuration not to work, as we talked about on IRC.
However, the implementation in the proposed patch is not yet complete, either:
- What if one of the child applets was *removed* from the config since the last configChanged() event?
- What if one of the child config groups changed, not the applets list?
Conceptually, configChanged() is supposed to re-read and apply the config, which means it has to adapt whatever the *current* state is to whatever the config prescribes.
In the case of the System Monitor applet, which maintains a set of child applets, that means applets have to be instanciated (as seen in your patch) or *destroyed* as necessary.
Further, for whatever child applets remain, it should forward configChanged() to them, so they, too, are poked to reload their configs.</pre>
</blockquote>
<p>On November 5th, 2013, 7:52 p.m. UTC, <b>Sergei Lopatin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">For now my code removes all child applets and re-adds them according to config. I think that it is the best way to save order of applets and re-read config of child applets. But I agree that if changes not belong to applets list, such behaviour is not necessary. </pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ah right, I overread that toggled() calls removeApplet() indiscriminately, sorry.
Still means it's a bit heavy-handed :). But an improvement over not supporting configChanged() I guess, yeah.</pre>
<br />
<p>- Eike</p>
<br />
<p>On November 5th, 2013, 7:44 p.m. UTC, Sergei Lopatin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma and Eike Hein.</div>
<div>By Sergei Lopatin.</div>
<p style="color: grey;"><i>Updated Nov. 5, 2013, 7:44 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-workspace
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Added ability to system-monitor plasmoid to set configuration from js code. Something like this:
a = activityForScreen(0)
if (a) {
var applet = a.addWidget("system-monitor_applet")
applet.writeConfig("applets", "sm_ram,sm_cpu")
}</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>plasma/generic/applets/system-monitor/system-monitor.h <span style="color: grey">(f8a2975)</span></li>
<li>plasma/generic/applets/system-monitor/system-monitor.cpp <span style="color: grey">(1705a99)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113659/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>