<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://reviewboard.kde.org/r/4819/">http://reviewboard.kde.org/r/4819/</a>
</td>
</tr>
</table>
<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="/r/4819/diff/3/?file=32465#file32465line611" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void MainWindow::controlTime()</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">611</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_timeControlDialog</span><span class="o">-></span><span class="n">show</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;">I think a comment here would be nice explaining that this dialog is not supposed to be modal and why it's not supposed to be modal.</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="/r/4819/diff/3/?file=32465#file32465line665" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void MainWindow::showDateTime()</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">665</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_clock</span> <span class="o">=</span> <span class="n">m_controlView</span><span class="o">-></span><span class="n">marbleWidget</span><span class="p">()</span><span class="o">-></span><span class="n">model</span><span class="p">()</span><span class="o">-></span><span class="n">dateTime</span><span class="p">().</span><span class="n">addSecs</span><span class="p">(</span> <span class="n">m_controlView</span><span class="o">-></span><span class="n">marbleWidget</span><span class="p">()</span><span class="o">-></span><span class="n">model</span><span class="p">()</span><span class="o">-></span><span class="n">timezone</span><span class="p">()</span> <span class="p">).</span><span class="n">toString</span><span class="p">(</span> <span class="s">"MMM d,yyyy hh:mm AP"</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;">Hm, why not adhere to the timeformat provided via QLocale? In Germany and other countries putting the month in front of the day and the year is not used at all.</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="/r/4819/diff/3/?file=32465#file32465line728" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 MainWindow::setupStatusBar()</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">728</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span><span class="p">(</span> <span class="s">"%1 0000-00-00T00:00:00"</span> <span class="p">).</span><span class="n">arg</span><span class="p">(</span> <span class="n">DATETIME_STRING</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;">See comment about QLocale use above.</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="/r/4819/diff/3/?file=32465#file32465line738" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 MainWindow::setupStatusBar()</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">738</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">connect</span><span class="p">(</span> <span class="n">m_controlView</span><span class="o">-></span><span class="n">marbleWidget</span><span class="p">()</span><span class="o">-></span><span class="n">model</span><span class="p">()</span><span class="o">-></span><span class="n">clock</span><span class="p">(),</span> <span class="n">SIGNAL</span><span class="p">(</span> <span class="n">timeChanged</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;">I think that this way of using signals and slots is atypical. Usually the changed datetime would get passed as a parameter.
Wouldn't this also get around having the clock pointer as a member (which to me looks a bit messy) ? Why do we want to have the clock as a member in here? Just makes the code more complex imho.
</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="/r/4819/diff/3/?file=32476#file32476line1089" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int MarbleModel::speed() const</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">1088</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">int</span> <span class="n">MarbleModel</span><span class="o">::</span><span class="n">speed</span><span class="p">()</span> <span class="k">const</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;">Hm, this is a bad name for the method imho. What is the "speed" of a model? That's something nobody will understand. Instead it should be named something like
clockSpeed()
Also: Do we really need a convenience method for this instead of just exposing the clock object?</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="/r/4819/diff/3/?file=32476#file32476line1094" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int MarbleModel::speed() const</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">1093</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">MarbleModel</span><span class="o">::</span><span class="n">setSpeed</span><span class="p">(</span> <span class="kt">int</span> <span class="n">speed</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;">see above. Bad naming imho.
setClockSpeed.
Do we really need this convenience method? Or does it just clutter up the model class?</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="/r/4819/diff/3/?file=32476#file32476line1099" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int MarbleModel::speed() const</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">1098</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">MarbleModel</span><span class="o">::</span><span class="n">setTimezone</span><span class="p">(</span> <span class="kt">int</span> <span class="n">timeInSec</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;">Do we really need this convenience method or wouldn't exposing the clock be good enough? Or do we just not want to expose the clock as a public class? I'm not sure.</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="/r/4819/diff/3/?file=32478#file32478line71" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/lib/QtMarbleConfigDialog.h</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 MARBLE_EXPORT QtMarbleConfigDialog : public QDialog</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">71</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">systemTimezone</span><span class="p">()</span> <span class="k">const</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;">systemTimezone vs. customTimezone should be an enum from what I understand.</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="/r/4819/diff/3/?file=32478#file32478line72" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/lib/QtMarbleConfigDialog.h</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 MARBLE_EXPORT QtMarbleConfigDialog : public QDialog</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">72</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">UTC</span><span class="p">()</span> <span class="k">const</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;">what does "UTC()" mean? I don't get what this method does. Please use better naming.</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="/r/4819/diff/3/?file=32492#file32492line131" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/marble_part.h</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 MarblePart: public KParts::ReadOnlyPart</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">131</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">showZenith</span><span class="p">(</span> <span class="n">bool</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;">"Show Zenith" is not a proper name imho.
methods should always be concise and easily tell what the method is about.
So
void showSunInZenith(bool) might be a better method name.</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="/r/4819/diff/3/?file=32492#file32492line221" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/marble_part.h</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 MarblePart: public KParts::ReadOnlyPart</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">221</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KToggleAction</span> <span class="o">*</span><span class="n">m_showNightMap</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 doesn't look correct to me.
The choice is really between 3 different states:
1) Shadow off
2) Normal Shadow on
3) Nightmap Shadow on
Two toggle actions buttons don't sufficiently represent this in terms of UI since the nightmap action depends on the shadow action. We need a better solution here.</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="/r/4819/diff/3/?file=32493#file32493line829" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/marble_part.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 MarblePart::setupActions()</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">828</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">//Toggle Action: Show night map</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;">See my comment above about the problematic mapping of the three states to two toggle buttons.</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="/r/4819/diff/3/?file=32497#file32497line227" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/plugins/render/stars/StarsPlugin.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">bool StarsPlugin::render( GeoPainter *painter, 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">227</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="n">renderPos</span> <span class="o">==</span> <span class="s">"ALWAYS_ON_TOP"</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;">YAY, you got it implemented correctly. Very cool :-)</pre>
</div>
<br />
<p>- Torsten</p>
<br />
<p>On August 4th, 2010, 10:13 p.m., hjain wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 marble and Bastian Holst.</div>
<div>By hjain.</div>
<p style="color: grey;"><i>Updated 2010-08-04 22:13:25</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;">This patch has the following features:-
1) The older 'Sun Control' dialog box is broken into new 'Sun Control' dialog box and 'Time Control' dialog box.
2) The 'Configure Marble Desktop Globe' has 'Date and Time' tab which has configuration features for time.
3) The current time is shown in status bar.
4) The icon of the Sun is shown on the Earth for zenith feature in 'Sun Control' dialog box.
5) Toolbar action button are added for shadow, night map and zenith features(although shadow and night map toolbar buttons are disabled in this patch because of lack of appropriate icons for them).
6) The name of ExtDateTime class is changed to MarbleClock. The license of David Roberts(author of ExtDateTime class) has also been updated for this class with this permissions.</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/QtMainWindow.h <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/bindings/python/sip/MarbleModel.sip <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/bindings/python/sip/SunLocator.sip <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/ExtDateTime.h <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/ExtDateTime.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/LayerManager.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleClock.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleClock.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleDataFacade.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleModel.h <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleTimeSettingsWidget.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/QtMarbleConfigDialog.h <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/QtMarbleConfigDialog.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/SunControlWidget.h <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/SunControlWidget.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/SunControlWidget.ui <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/SunLocator.h <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/SunLocator.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/TimeControlWidget.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/TimeControlWidget.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/TimeControlWidget.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/blendings/SunLightBlending.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/AdjustNavigation.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingManager.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/marble.kcfg <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/marble_part.h <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/marble_part.cpp <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/marble_part.rc <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/marbleui.rc <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/render/stars/StarsPlugin.h <span style="color: grey">(1157379)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/render/stars/StarsPlugin.cpp <span style="color: grey">(1157379)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4819/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>