<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 />
<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>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6153/diff/2/?file=42689#file42689line203" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/lib/global.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">const qreal MI2KM = 1.609344;</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">203</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c1">// Conversion Metric / Imperial System: meter vs. inch</span></pre></td>
</tr>
</tbody>
</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;">The comment is not quite correct</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6153/diff/2/?file=42692#file42692line98" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/plugins/render/mapscale/MapScaleFloatItem.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class MapScaleFloatItem : public AbstractFloatItem</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">98</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">bool</span> <span class="n">m_visible</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</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;">This is not required. It's being handled elsewhere already. Please remove it.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6153/diff/2/?file=42693#file42693line125" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/plugins/render/mapscale/MapScaleFloatItem.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void MapScaleFloatItem::changeViewport( ViewportParams *viewport )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">125</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">MapScaleFloatItem</span><span class="o">::</span><span class="n">paintBackground</span><span class="p">(</span> <span class="n">GeoPainter</span> <span class="o">*</span><span class="n">painter</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</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;">This is not required. It's being handled elsewhere already. Please remove it.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://svn.reviewboard.kde.org/r/6153/diff/2/?file=42693#file42693line141" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/plugins/render/mapscale/MapScaleFloatItem.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void MapScaleFloatItem::changeViewport( ViewportParams *viewport )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">141</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">m_visible</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</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;">This is not required. It's being handled elsewhere already. Please remove it.</pre>
</div>
<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>