<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/129607/">https://git.reviewboard.kde.org/r/129607/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 3rd, 2016, 1:28 p.m. CET, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As the one who did the mentioned change: -1 to this</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Please note that this change was done in coordination with the usability group. So please add our usability experts to this review request.</p></pre>
</blockquote>
<p>On December 3rd, 2016, 1:36 p.m. CET, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think I should explain a little bit more why I give a -1 to this.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">First of all there was a reason to move the version information into a dedicated tab. If we now see that this was a bad move, then we need to go a step back and re-evaluate why the step was done and what would be a better solution.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Then what I absolutely dislike in the screenshot is that it lists "Version" twice. From a UI perspective I think that is really bad and confusing. The minimum needed change would be a rename of the Version tab and also remove the Version from the Version tab.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Furthermore I want to mention that an idea back then was to add API which allows to fine tune the Version tab for the application, to allow adding further information.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also to consider here is whether the version information is really that important for all our applications to be that prominent. I see that for your case that's true, but honestly I doubt it. The version information is not that important in general and one click more is not that bad. Others might think that frameworks version is so important that it needs to be prominent and some might say that the Qt version is absolutely important. We cannot add everything there. So is it really that important?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We have many applications where the version information is hardly of interest. Example would be all applications from Plasma on e.g. KDE Neon. The version number is the same as the operating system. It's not important at all. That's clearly something to consider here.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">and please also add sebas to the review. He mostly drove the design of this change by sitting next to me while I implemented it and gave valuable feedback.</p></pre>
<br />
<p>- Martin</p>
<br />
<p>On December 3rd, 2016, 1:29 p.m. CET, Jean-Baptiste Mardelle wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks and KDE Usability.</div>
<div>By Jean-Baptiste Mardelle.</div>
<p style="color: grey;"><i>Updated Dec. 3, 2016, 1:29 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kxmlgui
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Last year, a change in kaboutapplicationdialog (review 124133) moved the application version in a separate tab. However for applications, it is really annoying (several Kdenlive users complained and I fully agree) to not have this information clearly displayed as soon as the About dialog is displayed. I understand the interest of having a separate version tab displaying the various underlying libraries used, but I propose to display again the application version in the About dialog header (see screenshot).</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Reverts a recent change, tested and works.</p></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/kaboutapplicationdialog.cpp <span style="color: grey">(156410e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129607/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/12/03/c02f4767-2a74-4c25-8599-de410678867e__about.png">About dialog with version</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>