<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/110226/">http://git.reviewboard.kde.org/r/110226/</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="http://git.reviewboard.kde.org/r/110226/diff/2/?file=141573#file141573line176" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/MapThemeDownloadDialog.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">176</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="nf">QSize</span><span class="p">(</span> <span class="n">iconSize</span><span class="p">.</span><span class="n">width</span><span class="p">()</span> <span class="o">+</span> <span class="n">doc</span><span class="p">.</span><span class="n">size</span><span class="p">().</span><span class="n">width</span><span class="p">(),</span> <span class="n">qMax</span><span class="p">(</span> <span class="n">iconSize</span><span class="p">.</span><span class="n">height</span><span class="p">(),</span> <span class="n">qRound</span><span class="p">(</span> <span class="n">doc</span><span class="p">.</span><span class="n">size</span><span class="p">().</span><span class="n">height</span><span class="p">()</span> <span class="p">)</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;">The doc.size() calculation doesn't terminate in my case for the osmarender map theme. The reason is that the umlaut in your surname is completely messed up. This makes the ghns registry file grow to 4MB!
Anyway, I think that all items in the list should have the same size and that all buttons should have fixed positions within a list element. This is currently not the case, as can be seen by jumping buttons when scrolling the list. So perhaps the size calculation could depend on the available space minus the size of the preview icon + buttons.</pre>
</div>
<br />
<p>- Bernhard</p>
<br />
<p>On April 30th, 2013, 6:05 p.m. UTC, Dennis Nienhüser 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 Marble.</div>
<div>By Dennis Nienhüser.</div>
<p style="color: grey;"><i>Updated April 30, 2013, 6:05 p.m.</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;">Uses our existing NewstuffModel to replace the "Download Maps" action in the Marble Qt action (which opens a browser) with an in-app map theme installation dialog. The patch is based on Bernhard's work in https://github.com/shentok/marble/tree/MapThemeDownloadDialog. I'll commit as a git patch series retaining ownership.
The dialog resembles the KDE GHNS "Download Maps" dialog (see screenshot). Differences between the two are mainly
- Qt version does not have the side bar to toggle list / detail mode, search, and sort by download count etc. The latter does not work anyway in the KDE version. Imho the sidebar in the KDE version is useless.
- Qt version displays map themes installed locally that were later removed on the server, so these don't become orphane and can still be uninstalled
- Detailed installation progress report next to each item in the Qt version. The KDE version only displays a global spinner anymore (see https://bugs.kde.org/show_bug.cgi?id=314210 )
- Details directly next to the item in the Qt version, no additional details dialog
It would be possible to extend this dialog to incorporate map themes uploaded to opendesktop.org by Marble users. Another possibility would be to open just installed themes automatically in Marble.
Currently the dialog shares the GHNS registry file (which saves which map themes are installed etc) with the KDE version, so (un)installing in one or the other application is reflected correctly in the other. This is great, but has the drawback of a somewhat hard-coded call
setRegistryFile( QDir::homePath() + "/.kde/share/apps/knewstuff3/marble.knsregistry", Marble::NewstuffModel::NameTag )
which will likely fail on some Linux distribution or Windows/Mac. Any thoughts how to do that cleanly? Sharing the registry between the KDE and the Qt version is important imho. One possibility would be to replace the KDE GHNS dialog with this one as well and move the registry file to the local Marble dir (could be an automated migration even). This would require proper testing of course to avoid regressions in the KDE version.
</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;">Linux only so far.
</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=315839">315839</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/QtMainWindow.h <span style="color: grey">(225d68d)</span></li>
<li>src/QtMainWindow.cpp <span style="color: grey">(fa45d75)</span></li>
<li>src/icons/16x16/dialog-ok.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/icons/16x16/edit-delete.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/icons/16x16/system-software-update.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/CMakeLists.txt <span style="color: grey">(534a7a9)</span></li>
<li>src/lib/MapThemeDownloadDialog.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/MapThemeDownloadDialog.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/MapThemeDownloadDialog.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/NewstuffModel.h <span style="color: grey">(cc2e369)</span></li>
<li>src/lib/NewstuffModel.cpp <span style="color: grey">(e8d4b2f)</span></li>
<li>src/lib/libmarble.qrc <span style="color: grey">(0aa5c6b)</span></li>
<li>src/marble_part.h <span style="color: grey">(0a56bad)</span></li>
<li>src/marble_part.cpp <span style="color: grey">(7302fc3)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110226/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/04/27/marble-qt-newstuff.png">Screenshot</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>