<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/128466/">https://git.reviewboard.kde.org/r/128466/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 18th, 2016, 12:05 p.m. UTC, <b>Sebastian Kügler</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;">Please don't ship it, yet.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I find the UI illogical. There's a groupbox grouping the checksum buttons, but then you can input the checksum above, so essentially, the groupbox is unnecessary and confusing.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Perhaps the whole thing could be simplified by naming the tab "Checksums" and removing the groupbox altogether, as it's not providing any semantic value.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">A usability reviewer should have a look.</p></pre>
</blockquote>
<p>On July 18th, 2016, 12:08 p.m. UTC, <b>Kai Uwe Broulik</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;">This dialog has been created in Review 128283 and Usability has been involved from the beginning...</p></pre>
</blockquote>
<p>On July 18th, 2016, 12:13 p.m. UTC, <b>Sebastian Kügler</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;">It has changed in a significant way, though, making it illogical.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(Not that I understand the "Share" title in the original review, but that's another matter.)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This needs more work.</p></pre>
</blockquote>
<p>On July 18th, 2016, 12:28 p.m. UTC, <b>Elvis Angelaccio</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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Perhaps the whole thing could be simplified by naming the tab "Checksums" and removing the groupbox altogether, as it's not providing any semantic value.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Preview here: https://share.kde.org/index.php/s/RUs9gAhIQqpFIqF</p></pre>
</blockquote>
<p>On July 18th, 2016, 12:31 p.m. UTC, <b>Sebastian Kügler</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;">This looks logical to me, and it's simpler: Very good!</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(Take that as "sebas withdraws his objection" :))</p></pre>
</blockquote>
<p>On July 18th, 2016, 1:33 p.m. UTC, <b>Thomas Pfeiffer</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;">Clear -1 to removing the group box.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Here is tha rationale:
For most "regular" users, only the lineedit at the top is relevant. The calculate stuff is just distraction and - worse - potential confusion. If we remove any visual distinction between the two, we just make the situation worse for them.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I agree that calling the tab "Checksums" is still better, though, because "Integrity" is too vague.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For the "average user" I just re-tested this with, what would actually help her is creating a second box around the verification part, calling the top one "Verify checksum" and the bottom "Calculate checksums".
That way if she was told e.g. by a website to verify a checksum, she'd knew she could simply ignore the whole calculation part.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Overall simplicity should not be the top priority here: The priority should be to make it clear to users who just want to check whether a download went okay which part they should care about and which they can ignore, while still providing the calculation part for advanced users who want to do that.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Of course another option would be to split it in two tabs, but that might make the tab bar quite long especially in languages like German.</p></pre>
</blockquote>
<p>On July 18th, 2016, 1:43 p.m. UTC, <b>Sebastian Kügler</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;">The latter part seems redundant then. How can you verify a checksum without calculating it?</p></pre>
</blockquote>
<p>On July 18th, 2016, 2 p.m. UTC, <b>Thomas Pfeiffer</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;">See <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">that</em> is why the bottom box was originally called "Share". Of course the verification part also does calculation, but it does so without you telling it to. You paste in the checksum, and it tells you whether it matches or not.
The manual calculation at the bottom is for people who share a file with others and want to accompany it with a checksum for <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">them</em> to verify it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think we might get away with "Calculate" anyway because those who don#t know much about checksums don't need to know that the verify part calculates as well, and those who know it should still be able to use the thing correctly.</p></pre>
</blockquote>
<p>On July 18th, 2016, 2:08 p.m. UTC, <b>Sebastian Kügler</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;">That 'Share' title completely puzzled me, and I think I'm the kind of user this dialog should work for very well. (I need to verify checksums all the time.)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To be quite honest, from getting it explained, I get the strong impression that you're overthinking it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To me, the most logical would be:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Calculate checksums at the top</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Under that, the input field so I can c/p or type my checksum in there to have it compared automatically. </li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That's both, the order of the workflow as well as the logical order of operation. 'Calculate' underneath would raise exactly the same question as I put above: "...but but but ... it could not verify it without calculating it, yet I have to hit a button to calculate ... maybe I should try again and maybe I should just use the commandline to be sure". </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Point in case: for this kind of stuff, simplicity trumps since it makes it easier to TRUST the dialog. I can't trust anything I don't fully understand or have doubts about, and that's what the groupbox design and the share title cause: doubts.</p></pre>
</blockquote>
<p>On July 18th, 2016, 2:13 p.m. UTC, <b>Ivan Čukić</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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">See that is why the bottom box was originally called "Share".</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">When I saw the UI, it did not even occur to me that it behaves like this comment suggested. I'd say that is a wrong thing to happen with an UI.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To me, the most logical would be:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Calculate checksums at the top
Under that, the input field so I can c/p or type my checksum in there to have it compared automatically.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">+1</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Additionally, whe are there 3 buttons for calculating the check-sums. All those can be calculated in-parallel without any noticeable performance impact. All of them (IIRC) read the file in 512-bit chunks and perform some computations. Computations are insignificant compared to file-reads, performance-wise.</p></pre>
</blockquote>
<p>On July 18th, 2016, 3:23 p.m. UTC, <b>Thomas Pfeiffer</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;">Sebas, Ivan, please read the discussion here: https://git.reviewboard.kde.org/r/128283/ None of the things you're commenting on here fell from the sky, there were reasons for them.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As for what is "logical": Yes, this is the logical progression for <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">you</em>, who know how this whole checksum business works. You, however, can also just use [whatever]sum from the command line. This dialog was designed with users in mind who don't know how checksum verification works (the vast majority of "regular" computer users, I can assure you). They don't and need not know that a checksum has to be calculated first.
All they should need to know is what they should do, and the instruction at the very top of the dialog explains exactly that. And if they follow those instructions, they get a result.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In the first iteration, the dialog just threw three checksums at you, which leaves ordinary users utterly puzzled (I tried it with one). Plus, David Faure advised against calculating them automatically, and since he has a certain reputation of knowing what he's talking about, we followed his advice.
Of course we could just use one "Calculate" button.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Still, I don't see a point in making the vast majority of users who just want to verify a checksum they found a website calculate the checksum first.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm open for ideas on how to separate the two usecases (verifying the integrity of a file and getting checksums to do whatever with them) better, but I strongly oppose bothering users who know nothing about checksums with any information they don't need (and yes, that includes the calculated checksum, as that is 100% irrelevant to them).</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;">So you're saying that I'm too advanced a user for this dialog to make sense? I'm at a loss for words here. Maybe you can view Ivan and me as the kind of user this dialog should be used by and should work for well, and not try to conveniently place us outside of the target group by just telling us that the target user is dumber, but still not dumb enough to not need checksums. This way a bad design doesn't get fixed.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As to my actual issue, it's not addressed in the review you link, it rather leaves me more puzzled:
- the checksum must not be calculated automatically (I completely agree, I don't want my disk to be thrashed once I open this dialog or tab; it's unrelated to my comment, however)
- so I paste the checksum in there, and then hit calculate (which button, btw, how do I tell?)
- When I just need the checksum (for example when handing it out with a file, or to verify a file with a user), I hit calculate, and I can copy the resulting string?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For me, calculating the checksum and showing me is what this dialog really should do. Comparing it to something typed or copy/pasted is an extra, which can only happen once the checksum has been calculated. Calculating is clearly the first, necessary step. Why the paste box is above it ... I simply don't understand. Why a groupbox for the primary function that distinguishes it from the copy/paste part (which, without calculating is entirely useless) -- I don't know. It serves no purpose (and none is given in the link you post).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I rest my case, the design of this dialog doesn't make a lot of sense as it is, it can be done simpler, easier to understand and thus, better.</p></pre>
<br />
<p>- Sebastian</p>
<br />
<p>On July 16th, 2016, 12:35 p.m. UTC, Elvis Angelaccio 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, KDE Usability and Dominik Haumann.</div>
<div>By Elvis Angelaccio.</div>
<p style="color: grey;"><i>Updated July 16, 2016, 12:35 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</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;">Dominik suggested to rename the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Checksums</code> tab to <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Integrity</code>, so that we can "free" the Checksums string and use it as the title of the groupbox below (in place of the current <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Share</code> string, which can be confusing).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Preview in the attached screenshot.</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/widgets/checksumswidget.ui <span style="color: grey">(03c64db)</span></li>
<li>src/widgets/kpropertiesdialog.cpp <span style="color: grey">(808765c)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128466/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/07/16/6771ed06-c803-4d18-abe3-91e4f97c8c76__checksums-tab.png">Before</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/b2cd12c8-6bbf-4123-9e8e-59cb0c29cbdb__Spectacle.TJ7614.png">After</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>