<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/100117/">http://git.reviewboard.kde.org/r/100117/</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;">Thanks for the applet! I'm testing it now... Inline comments below. One note: in the screenshot,the reload icon is at the top left. It would be better to put that in the top-right, so it's consistent with the other applets.</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://git.reviewboard.kde.org/r/100117/diff/1/?file=2787#file2787line1" style="color: black; font-weight: bold; text-decoration: underline;">src/context/applets/tabs/CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; "></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">1</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">#project(context-tabs)</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;">Don't think you need this :)</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://git.reviewboard.kde.org/r/100117/diff/1/?file=2789#file2789line98" style="color: black; font-weight: bold; text-decoration: underline;">src/context/applets/tabs/TabsApplet.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">class TabsApplet : public Context::Applet</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="cm"> * Updates the data from the Upcoming Events engine</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;">remnant of a copy-paste I think :)</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://git.reviewboard.kde.org/r/100117/diff/1/?file=2790#file2790line37" style="color: black; font-weight: bold; text-decoration: underline;">src/context/applets/tabs/TabsApplet.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; "></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">37</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">Q_DECLARE_METATYPE</span> <span class="p">(</span> <span class="n">TabsInfo</span> <span class="o">*</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;">Best to put the Q_DECLARE_METATYPE in the header file (outside namespace declarations, inside the include guard) so it can be shared if needed.</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://git.reviewboard.kde.org/r/100117/diff/1/?file=2790#file2790line137" style="color: black; font-weight: bold; text-decoration: underline;">src/context/applets/tabs/TabsApplet.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; "></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">137</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dataEngine</span><span class="p">(</span> <span class="s">"amarok-tabs"</span> <span class="p">)</span><span class="o">-></span><span class="n">query</span><span class="p">(</span> <span class="n">QString</span><span class="p">(</span> <span class="s">"tabs:fetchGuitar:"</span> <span class="p">)</span> <span class="o">+</span> <span class="n">QString</span><span class="p">().</span><span class="n">setNum</span><span class="p">(</span> <span class="n">m_fetchGuitar</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;">You could use QString::number() 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="http://git.reviewboard.kde.org/r/100117/diff/1/?file=2795#file2795line143" style="color: black; font-weight: bold; text-decoration: underline;">src/context/applets/tabs/TabsView.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">TabsView::showTab( TabsItem *tab )</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">143</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">richText</span> <span class="o">+=</span> <span class="s">"<p><span style=</span><span class="se">\"</span><span class="s"> font-family:'Sans Serif'; font-size:11pt; font-weight:600;</span><span class="se">\"</span><span class="s">>"</span> <span class="o">+</span> <span class="n">tab</span><span class="o">-></span><span class="n">getTabTitle</span><span class="p">()</span> <span class="o">+</span> <span class="s">" ("</span> <span class="o">+</span> <span class="n">i18n</span><span class="p">(</span> <span class="s">"source: "</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;">Not sure what translating "source:" is supposed to mean. you could make it clearer to the translators by doing something like i18np( "the source of the guitar tabs", "source:" ) (off the top of my head)</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://git.reviewboard.kde.org/r/100117/diff/1/?file=2795#file2795line145" style="color: black; font-weight: bold; text-decoration: underline;">src/context/applets/tabs/TabsView.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">TabsView::showTab( TabsItem *tab )</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">145</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">richText</span> <span class="o">+=</span> <span class="n">i18n</span><span class="p">(</span> <span class="s">")"</span><span class="p">)</span> <span class="o">+</span> <span class="s">"</span></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;">we can probably skip translating a ) here :)
also not sure about hardcoding colors in the html--have you tried with non-oxygen color schemes? you can use the utility functions in Applet (like hightlightColor) if you want a desaturated version of the current palette highlight color. I think it would be best to operate on a system color so it fits when the user changes color schemes.</pre>
</div>
<br />
<p>- Leo</p>
<br />
<p>On October 31st, 2010, 2:28 p.m., Rainer Sigle wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.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 Amarok.</div>
<div>By Rainer Sigle.</div>
<p style="color: grey;"><i>Updated 2010-10-31 14:28: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 adds a new applet to the Amarok context menu: "Tabs". It's designed for guitar and bass-players, as it downloads and displays tablature information for the current song. Directly playing along a song has therefore never been easier.
The tab data is currently fetched from two sites: www.ultimateguitar.com and www.fretplay.com. As these provide no direct API, the tab data is extracted using some simple HTML parsing. Multiple results, e.g. versions of the tab, are stored and can be choosen from a list. A settings dialog allows to define the type of tabs to be fetched. In this patch we've guitar and bass, but it could be extended to e.g. drums or piano in the future. Furthermore I've added a reload button which allows to easily refine the criteria for the tab-search. This can be used e.g. when no tab-results were found because the song name contains fragments like (live), remix, etc.
I'll send some new png-images to amarok devel mailinglist because I was not able to correctly include these binary files into the patch. Please be aware that I don't know if there are any copyright restrictions on these bitmaps! Maybe they can be replaced with some KDE/Plasma specific images. Unfortunately I didn't find anything appropriate.
Thanks for any feedback on this...
By the way: Also big Thanks! to all Amarok devs and supporters for their great work. I love this application.</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;">used and tested during the last months, works fine for me</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>src/context/applets/CMakeLists.txt <span style="color: grey">(98bdd2b)</span></li>
<li>src/context/applets/tabs/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/ReloadEditDialog.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/TabsApplet.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/TabsApplet.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/TabsItem.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/TabsItem.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/TabsSettings.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/TabsView.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/TabsView.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/amarok-context-applet-tabs.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/amarok-tabs-bass.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/amarok-tabs-drum.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/amarok-tabs-guitar.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/applets/tabs/amarok-tabs-piano.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/engines/CMakeLists.txt <span style="color: grey">(c1f029e)</span></li>
<li>src/context/engines/tabs/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/engines/tabs/TabsEngine.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/engines/tabs/TabsEngine.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/engines/tabs/TabsInfo.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/context/engines/tabs/amarok-data-engine-tabs.desktop <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100117/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://git.reviewboard.kde.org/r/100117/s/9/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2010/10/31/context_applet_tabs_400x100.png" style="border: 1px black solid;" alt="Tabs Applet" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>