<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://svn.reviewboard.kde.org/r/5747/">http://svn.reviewboard.kde.org/r/5747/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">so once again we descend into a situation where we have a configuration dialog with a billion twiddly options in it. i'm really not very impressed by this, to be honest.
the layout could perhaps be a bit cleaner, e.g.:
Custom font color: [ x ] ( Color button )
Show shadow: [ x ]
Custom shadow color: [ x ] ( Color button )
that's probably clearer and is both one less row and one less widget with the same features.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="/r/5747/s/546/" style="color: black; font-weight: bold; font-size: 9pt;">clock-config.png</a></td>
</tr>
<tr>
<td><a href="http://svn.reviewboard.kde.org/r/5747/s/546/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2010/11/04/clock-config_286_233_198_34.png" style="border: 1px black solid;" alt="The changed configuration"></a></td>
</tr>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">when this is unchecked, both the label and the colour combo should be disabled. it's not really clear to me that the checkbox turns off the shadow versus just turns off a custom color for the shadow.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="/r/5747/s/546/" style="color: black; font-weight: bold; font-size: 9pt;">clock-config.png</a></td>
</tr>
<tr>
<td><a href="http://svn.reviewboard.kde.org/r/5747/s/546/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2010/11/04/clock-config_266_183_141_24.png" style="border: 1px black solid;" alt="The changed configuration"></a></td>
</tr>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">shouldn't have a colon at the end of it, and since there are two colors, it should probably be plural ("colors"), ditto for "Use theme color" above.</pre>
<br />
<p>- Aaron</p>
<br />
<p>On November 4th, 2010, 2:07 a.m., Alex Merry wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated 2010-11-04 02:07:24</i></p>
<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;">Since the shadow was introduced for the digital clock a few weeks ago, the custom colour setting has been ignored. This re-enables it, and also allows the user to choose a shadow colour.
This changes the configuration dialog and introduces a new option, which is why I'm submitting it for review before committing.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Changing, saving and loading the settings worked in plasmoid-viewer.</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>/trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.h <span style="color: grey">(1191270)</span></li>
<li>/trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clock.cpp <span style="color: grey">(1191270)</span></li>
<li>/trunk/KDE/kdebase/workspace/plasma/generic/applets/digital-clock/clockConfig.ui <span style="color: grey">(1191270)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5747/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://svn.reviewboard.kde.org/r/5747/s/546/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2010/11/04/clock-config_400x100.png" style="border: 1px black solid;" alt="The changed configuration" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>