<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/110276/">http://git.reviewboard.kde.org/r/110276/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 2nd, 2013, 10:20 p.m. UTC, <b>Heiko Tietze</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;">Please make sure that left and right spacing fits design specification (if we have any). And the edit fields looks a little bit narrow at first glance.
>From a usability point of view I would always place edits below their caption. That makes translation easier and leads to better alignment. But in this case it would be weird to break lines.
I would sort the panels differently, ie. "Now playing" to the bottom (or into another tab).
"Auto offline": Isn't it part of the "Auto away" area? ("Sorry, it isn't.") To prevent this mistake you could rename it to "Contact list behaviour". But make this feature really sense?
Take care about short-cuts: It's not easy to access the away message, for instance. Better you provide Alt+M/E/R per "With &message" and "With m&essage" as well as "Afte&r" (second item), or the like.
"Now playing" looks quite difficult to use. If you decide to move it into another tab you could add assistance features or at least some help.
After all, settings don't have a consistent layout, so it's hard to give a good advice. Ship it, if you don't worry about a particular problem.
>Additionally, I'd like to add a dialog->resize(700, 640) to ContactListWidget::showSettingsKCM()
Is it a predefined window size? Sounds evil.</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;">Hi Heiko,
I'm sorry to respond with many questions, but I can't help it ;)
> Please make sure that left and right spacing fits design specification
It uses the default spacings, so shouldn't this be decided by the theme? i.e. can anything be wrong if I never alter the pre-set spacings?
> And the edit fields looks a little bit narrow at first glance.
Sorry, which ones do you mean? All edit fields are much wider than before, aren't they?
> I would sort the panels differently, ie. "Now playing" to the bottom
Good idea, I already reordered it a bit, but I guess you're right that the current sorting isn't optimal yet.
> To prevent this mistake you could rename it to "Contact list behaviour".
I will do that too.
> But make this feature really sense?
Yes, I think, since if you don't have any presence widgets enabled, if you close the contact list you'll still be online with no obvious way to go offline (or even notice you're still online). So in that case it would make sense. Given the fact that there is not really any usable cross-desktop presence widgets, I think this feature should be kept.
> Take care about short-cuts:
Oh, another thing I totally overlooked ;) I'll fix them.
> If you decide to move it into another tab you could add assistance features or at least some help.
I'm unsure how you'd call the two tabs so it makes sense... I can't really come up with anything useful. "General" and "Status" sound stupid and it's not very obvious what they contain :(
> Is it a predefined window size? Sounds evil.
Okay. But, how does one do that, setting a recommended initial size for the dialog? The current size is too large.
Thanks for reviewing!
Cheers
</pre>
<br />
<p>- Sven</p>
<br />
<p>On May 2nd, 2013, 9:07 p.m. UTC, Sven Brauch 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 Telepathy, KDE Usability, David Edmundson, and Martin Klapetek.</div>
<div>By Sven Brauch.</div>
<p style="color: grey;"><i>Updated May 2, 2013, 9:07 p.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 revamps the settings UI of the kded module. Especially, it removes the "flat" flag from the group boxes, and also removes lots of spacers. But talk doesn't really help with UI changes, so you'd better look at the screenshot below ;)
Additionally, I'd like to add a dialog->resize(700, 640) to ContactListWidget::showSettingsKCM() in contact-list-widget.cpp in ktp-contactlist (the function which opens the dialog), but I don't want to open a seperate review for that.</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;">The settings dialog still seems to work fine.</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>config/telepathy-kded-config.cpp <span style="color: grey">(50c176d)</span></li>
<li>config/telepathy-kded-config.ui <span style="color: grey">(54ebc54)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110276/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/05/02/ktp-settings.png">Old (left) and new (right) settings dialog, with their respective default sizes</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>