<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/112533/">http://git.reviewboard.kde.org/r/112533/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin: 1em 0 0 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 5th, 2013, 10:47 a.m. UTC, <b>Aaron J. Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding: 0 0 0 1em;">
<br style="display: none;" />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/09/05/ratings-main.png" style="color: black; font-weight: bold; font-size: 9pt;">ratings-main.png</a></td>
</tr>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I wonder if we need an explicit "Ratings" header here. Replacing the raw numbers with stars (or some other icon, but stars are well recognized) should make it obvious that these are ratings. So it probably doesn't need to say that explicitly.
A header here adds more visual noise and takes up screen space.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">the Ratings header is gone</pre>
<br />
<blockquote style="margin: 1em 0 0 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 5th, 2013, 10:47 a.m. UTC, <b>Aaron J. Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding: 0 0 0 1em;">
<br style="display: none;" />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/09/05/ratings-main.png" style="color: black; font-weight: bold; font-size: 9pt;">ratings-main.png</a></td>
</tr>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">instead of numbers, using stars would be better.
the labels should also be center aligned:
Usability: **
Funny: ***
</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I couldn't find a star icon. Can you suggest one?
Also my labels aren't aligned in the center.
I have attached a new Image. What am I doing wrong?</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 5th, 2013, 10:47 a.m. UTC, <b>Aaron J. Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/112533/diff/1/?file=187171#file187171line51" style="color: black; font-weight: bold; text-decoration: underline;">activeclient/package/contents/ui/storebrowser/AssetColumn.qml</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">51</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nx">itemBrowser</span><span class="p">.</span><span class="nx">pop</span><span class="p">(</span><span class="nx">root</span><span class="p">)</span></pre></td>
</tr>
<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">52</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kd">var</span> <span class="nx">page</span> <span class="o">=</span> <span class="nx">itemBrowser</span><span class="p">.</span><span class="nx">replace</span><span class="p">(</span><span class="nx">Qt</span><span class="p">.</span><span class="nx">resolvedUrl</span><span class="p">(</span><span class="s2">"AssetColumn.qml"</span><span class="p">))</span></pre></td>
</tr>
<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">53</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nx">page</span><span class="p">.</span><span class="nx">assetId</span> <span class="o">=</span> <span class="nx">assetId</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this seems very brute force; just re-setting the assetId was not enough?
i'm also a little concerned about popping the column from *inside* the column itself. i'm sure QML does the right thing behind the scenes w/regards to memory allocation, etc but it would be far nicer if simply re-setting the assetId would do the trick, or even if page offered a reload function that re-sent the asset info request.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have moved the functionality of onAssetIdChanged to reloadPage and it works.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 5th, 2013, 10:47 a.m. UTC, <b>Aaron J. Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/112533/diff/1/?file=187171#file187171line299" style="color: black; font-weight: bold; text-decoration: underline;">activeclient/package/contents/ui/storebrowser/AssetColumn.qml</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">299</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nx">PlasmaComponents</span><span class="p">.</span><span class="nx">Button</span> <span class="p">{</span></pre></td>
</tr>
<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">300</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kd">id: ratingsDeleteButton</span></pre></td>
</tr>
<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">301</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">visible:</span> <span class="nx">assetOperations</span><span class="p">.</span><span class="nx">ratingsModel</span><span class="p">.</span><span class="nx">ratingsCount</span> <span class="o">></span> <span class="mi">0</span></pre></td>
</tr>
<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">302</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">anchors.horizontalCenter:</span> <span class="nx">parent</span><span class="p">.</span><span class="nx">horizontalCenter</span></pre></td>
</tr>
<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">303</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">text:</span> <span class="nx">i18n</span><span class="p">(</span><span class="s2">"Delete ratings"</span><span class="p">)</span></pre></td>
</tr>
<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">304</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">onClicked:</span> <span class="nx">ratingsDeleteConfirmation</span><span class="p">.</span><span class="nx">open</span><span class="p">()</span></pre></td>
</tr>
<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">305</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">there was something about this button that just never felt right. it was Yet Another Button in an already pretty busy area of the UI.
when looking at the Ratings page in the settings, it came to me: the Delete button should be there. each rating in the Participant Ratings list could have a delete button. that way a function which will be rarely used is not in the main application UI and is instead in the configuration / settings area where it fits in quite nicely.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Each rating is a new row in our model. So if we have the asset 3 with two ratings
we will have two rows, so when I delete one of the ratings both of them will get deleted because
that's how our API works. It deletes all the ratings of an asset. I have also attached an image which
shows the new UI. Is that ok? Should we modify something else from the ui?
</pre>
<br />
<p>- Giorgos</p>
<br />
<p>On September 5th, 2013, 10:25 a.m. UTC, Giorgos Tsiapaliokas 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 Bodega.</div>
<div>By Giorgos Tsiapaliokas.</div>
<p style="color: grey;"><i>Updated Sept. 5, 2013, 10:25 a.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;">This patch implements the ratings feature</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;">check the attached images</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>activeclient/package/contents/ui/SettingsPage.qml <span style="color: grey">(fb46eca)</span></li>
<li>activeclient/package/contents/ui/settings/ParticipantRatings.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeclient/package/contents/ui/storebrowser/AssetColumn.qml <span style="color: grey">(a678f16)</span></li>
<li>activeclient/package/contents/ui/storebrowser/Ratings.js <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeclient/package/contents/ui/storebrowser/RatingsColumn.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>activeclient/src/bodegastore.h <span style="color: grey">(1e5aac5)</span></li>
<li>activeclient/src/bodegastore.cpp <span style="color: grey">(ba9dc27)</span></li>
<li>lib/bodega/CMakeLists.txt <span style="color: grey">(8d382a7)</span></li>
<li>lib/bodega/assetjob.h <span style="color: grey">(5aab88c)</span></li>
<li>lib/bodega/assetjob.cpp <span style="color: grey">(5f539cb)</span></li>
<li>lib/bodega/assetoperations.h <span style="color: grey">(7ce7900)</span></li>
<li>lib/bodega/assetoperations.cpp <span style="color: grey">(9f9c2d5)</span></li>
<li>lib/bodega/assetratingsjob.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/assetratingsjob.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/assetratingsjobmodel.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/assetratingsjobmodel.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/globals.h <span style="color: grey">(5ab45da)</span></li>
<li>lib/bodega/participantratingsjob.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/participantratingsjob.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/participantratingsjobmodel.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/participantratingsjobmodel.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/ratingattributesjob.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/ratingattributesjob.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/ratingsmodel_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/ratingsmodel_p.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/bodega/session.h <span style="color: grey">(d27d284)</span></li>
<li>lib/bodega/session.cpp <span style="color: grey">(a7c7e94)</span></li>
<li>lib/bodega/session_p.h <span style="color: grey">(ebefb4f)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112533/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/09/05/ratings-main.png">ratings-main.png</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/09/05/participantratings.png">participantratings.png</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>