<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/6153/">http://svn.reviewboard.kde.org/r/6153/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 18th, 2010, 3:54 p.m., <b>Torsten Rahn</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;">Hi Nhan,
Almost correct - but you misunderstood the aim of the task a bit: We don't need another means of showing and hiding the whole scalebar. What this task is about is just showing and hiding the scale ratio below the scale ratio. However this should be quite easy to fix for you.
Your patch adds the functionality of displaying the scale ratio nicely! However there are some things that you need to adjust in order to mark the task as complete:
- There needs to be the margin honored below the scale ratio. Please ensure that the distance is kept between the scale ratio and the border of the floatitem.
- The scale ratio should be displayed centered (and not left aligned).
- The scale ration should be displayed in user readable form (so no "1:1.233456e6", but instead something like "1:1230000")
- You need to remove the whole "visible" stuff that you introduced. It's not needed for the plugin since this stuff already gets handled elsewhere.
- Instead you should adjust your patch so that the user can toggle the visibility of the scale ratio text (the text string e.g. "1:123000") that you introduced. When toggling the visibility of the scale ratio the visibility of the rest of the scale bar should stay unaffected.
Maybe instead of labeling the configuration item "Show scale" it should get labeled "Show scale ratio". This might make things more clear ;-)
Could you make those improvements and resubmit the patch?
Also make sure you don't actually commit the patch to svn trunk since we are on feature freeze :-) We just want to have the patch on review board so that it doesn't get lost :-)
</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;">Oh and it's good that you attached screenshots to your review request. :-) Could you create new screenshots once you fixed the issues above :-) ?</pre>
<br />
<p>- Torsten</p>
<br />
<p>On December 18th, 2010, 1:14 p.m., Khanh-Nhan Nguyen 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 KDE-Edu.</div>
<div>By Khanh-Nhan Nguyen.</div>
<p style="color: grey;"><i>Updated 2010-12-18 13:14:47</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;">- Config dialog for map scale plugin (with a single option visible/invisible).
- Scale ratio implemented and displayed below the scale bar (with an acceptable error).</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/kdeedu/marble/src/lib/global.h <span style="color: grey">(1207419)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/render/mapscale/CMakeLists.txt <span style="color: grey">(1207419)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/render/mapscale/MapScaleConfigWidget.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/render/mapscale/MapScaleFloatItem.h <span style="color: grey">(1207419)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/render/mapscale/MapScaleFloatItem.cpp <span style="color: grey">(1207419)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6153/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/6153/s/584/"><img src="http://svn.reviewboard.kde.org" style="border: 1px black solid;" alt="Config dialog" /></a>
<a href="http://svn.reviewboard.kde.org/r/6153/s/585/"><img src="http://svn.reviewboard.kde.org" style="border: 1px black solid;" alt="After uncheck the box in the config dialog - scale bar becomes invisible" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>