<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="https://git.reviewboard.kde.org/r/116736/">https://git.reviewboard.kde.org/r/116736/</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;">Hey,
Thanks for your patch, unfortunately, the changes are wrong, I've explained why inline.
For reference, also see the following two review requests:
https://git.reviewboard.kde.org/r/116460/
https://git.reviewboard.kde.org/r/115870/
I wonder if you're just woefully unlucky, or if this bug is listed somewhere, in the latter case, it should probably link to this review request, or one of the above. It's getting a bit tiring to explain the exact same mistake to someone else every week. :)</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="https://git.reviewboard.kde.org/r/116736/diff/1/?file=253360#file253360line68" style="color: black; font-weight: bold; text-decoration: underline;">mediaelements/mediaplayer/MusicStats.qml</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">68</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nx">StatsLabel</span> <span class="p">{</span> <span class="k">text:</span> <span class="nx">metaData</span><span class="p">.</span><span class="nx">albumArtist</span> <span class="o">?</span> <span class="k">metaData.albumArtist :</span> <span class="s2">""</span> <span class="p">}</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">68</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nx">StatsLabel</span> <span class="p">{</span> <span class="k">text:</span> <span class="nx">metaData</span><span class="p">.</span><span class="nx">albumArtist</span> <span class="o">?</span> <span class="s2"><span class="hl">"<font color='orange'>Artist : </font>"</span></span><span class="o"><span class="hl">+</span></span><span class="k">metaData.albumArtist :</span> <span class="s2">""</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;">Hardcoding colors is a complete no-go. It will clash with themes, and makes the UI unthemable. Hardcoding the color like this is also very ugly, use two labels each, if you and set the according font.color property.
Also, this change seems unrelated, and should not be part of this review.</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="https://git.reviewboard.kde.org/r/116736/diff/1/?file=253361#file253361line37" style="color: black; font-weight: bold; text-decoration: underline;">mediaelements/mediawelcome/HomeScreenFooter.qml</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">37</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span> <span class="kd">var</span> <span class="nx">timeOfDay</span> <span class="o">=</span> <span class="nx">hours</span> <span class="o">></span> <span class="mi">12</span> <span class="o">?</span> <span class="s1">'PM'</span> <span class="o">:</span> <span class="s1">'AM'</span><span class="p">;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">37</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span> <span class="kd">var</span> <span class="nx">timeOfDay</span> <span class="o">=</span> <span class="nx">hours</span> <span class="o">><span class="hl">=</span></span> <span class="mi">12</span> <span class="o">?</span> <span class="s1">'PM'</span> <span class="o">:</span> <span class="s1">'AM'</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;">Congrats, you're the third person to come up with this solution.
Unfortunately, it's wrong: The time will be displayed in the wrong format for most locales.
You should use the time dataengine, and Qt.formatTime (for example) to format the time string.</pre>
</div>
<br />
<p>- Sebastian Kügler</p>
<br />
<p>On March 11th, 2014, 7:14 p.m. UTC, Atul Dubey wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 Plasma, Akshay Ratan, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan.</div>
<div>By Atul Dubey.</div>
<p style="color: grey;"><i>Updated March 11, 2014, 7:14 p.m.</i></p>
<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=330115">330115</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-mediacenter
</div>
<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;">Fixed the bug. Applied a new condition. Tested on local system.</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>mediaelements/mediaplayer/MusicStats.qml <span style="color: grey">(178a37d)</span></li>
<li>mediaelements/mediawelcome/HomeScreenFooter.qml <span style="color: grey">(d2c0eb7)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/116736/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>