<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/107296/">http://git.reviewboard.kde.org/r/107296/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 15th, 2012, 5:30 p.m., <b>Aurélien Gâteau</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;">Looks nicer and nicer, but I have a few suggestions:
# Fonts
I am not fond of the mix of italics and bold though: it makes it harder to focus on the content I think. Have you tried getting rid of them?
# Expand / Collapse links
It may also look a bit more tidy with [-] and [+] symbols on the left of the text instead of [Expand], [Collapse] links. You can draw those with something like this (from QTreeView code):
QStyleOptionViewItem opt;
opt.rect = ...
opt.state = QStyle::State_Children;
if (expanded) {
opt.state |= QStyle::State_Open;
}
style()->drawPrimitive(QStyle::PE_IndicatorBranch, &opt, painter, this);
# Wording
The "%d uses" string should be harmonized I think:
Right now text of top-level lines is:
%d uses: /foo/bar/baz.cpp
Whereas text of context lines is:
In context Class::method(...): %d uses
I would suggest using "%d uses in %s" for both top-level and context lines.
Assuming "context" is not ambiguous, "In context Class::method(...)" can be simplified to "In Class::method(...)"
# Alignments
I think it would look nicer if the left of the purple bullets were aligned with the left of the "In context" string.
</pre>
</blockquote>
<p>On November 15th, 2012, 7:42 p.m., <b>Sven Brauch</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;">Hello!
Thanks for your comments.
About the fonts, I removed the italic for Declaration / Definition and the "n uses". I would also remove the italic of the project name, but it's provided by kdevplatform and I'm not sure whether I should touch it (it's probably being used in other places too).
The uses count and file name in bold are good I think, those are important, and I think it's okay that they're bold.
About the expand/collapse links, I think such a tiny grey icon back there would look weird. You'd have to put it at the beginning, then, and I don't really like it there either. Plus, that painter method isn't really that easy to implement, it requires some extra efforts which don't really seem to be worth it (you don't have the painter in the widget, and the rect you don't have either etc.).
We could consider just changing it to [+] and [-], that'd look like this: http://i.imgur.com/iZLty.png
Not sure, what do you think?
About the wording: I removed the "Context", and I also just removed the "%d uses" (see screenshot). Aleix was unhappy about it too, and it's not really necesary, I agree.
For the alignments, I aligned it like this on purpose in order to make it very clearly visible where the list of uses in a specific context ends. I think it's less visible if you remove the indent.
Greetings,
Sven</pre>
</blockquote>
<p>On November 15th, 2012, 10:37 p.m., <b>Aurélien Gâteau</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;"># Fonts
The problem right now is that for many projects (like the screenshot you posted), you have *bold*/italic/*bold*. Nothing stands out if everything is emphasised.
# Expand/collapse links
I just realized it is an HTML view, so I understand using QStyle is not practical indeed. Still, I think the current links add a bit too much noise. A good enough alternative could be using the triangle unicode characters (U+25B8: ? and U+25BE: ?).
Thinking more about it, maybe the order of the elements in the header items could be improved. Something like this would look more natural to me:
? @ radio_logtool/mainwindow.py: 11 uses
? @ radio_logtool/main.py: 2 uses
...
...
(where @ is the icon)
This is a very common layout: expand/collapse | icon | label | details. KMail uses a similar layout for its folder hierarchy. Dolphin also uses something similar in "detailed tree mode" (except details are in separate columns). That helps the user to parse the UI because it matches with known UI layouts.
Furthermore placing filenames before use count keep file names vertically aligned, which helps when scanning the list.
# Alignments
There are bulletpoints in front of the items, that should be enough to mark the beginning of the list IMHO.
Here is a mockup of what I have in mind:
http://simplest-image-hosting.net/png-0-kdevelop-use-widget</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;">Damn, Reviewboard ate my unicode characters :'(. Look at the mockup to see them.</pre>
<br />
<p>- Aurélien</p>
<br />
<p>On November 15th, 2012, 9:43 p.m., Sven Brauch wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDevelop, KDE Usability and Milian Wolff.</div>
<div>By Sven Brauch.</div>
<p style="color: grey;"><i>Updated Nov. 15, 2012, 9:43 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;">TL;DR: Look at the screenshots below. :)
Make the uses widget nicer.
Main changes include:
* Fix contexts not being clickable (they were clickable but didn't work)
* Remove the blue arrows; all widgets are expanded by default and can
be collapsed with a small link in the text instead. This operation is
not common enough to justify such a huge ugly button.
(The main problem with the button was that it looked ugly, tough)
* Contexts without actual uses are always sorted last.
* Add some nice icons to the list.
* Remove all the ugly frames.
* Make text displaying code use a fixed-width font.
* Make the tooltip use a fixed-width font too, make it smaller (just 2
lines of context instead of 3), and highlight the line with the use
by making it bold-face.
* Re-format and rename all the widget headers to be more readable
and look nicer.
I will add a few comments to the patch to explain what I was doing.
Oh and also, I had that impression for quite a while now, but right now I'm pretty sure the find uses feature is broken in some way. Just look at the screenshots: it found way more occurences in the first screenshot than in the second (which happened after a reparse of some projects). And this has nothing to do with my changes. :)</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;">Manual testing; no functional changes were done except for the context link fix.</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>language/duchain/navigation/useswidget.h <span style="color: grey">(ec0440e)</span></li>
<li>language/duchain/navigation/useswidget.cpp <span style="color: grey">(8c01e7c)</span></li>
<li>shell/projectcontroller.cpp <span style="color: grey">(6168066)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107296/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/107296/s/821/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/11/12/useswidgetcomp_400x100.png" style="border: 1px black solid;" alt="Comparsion between the old and the new uses widget." /></a>
<a href="http://git.reviewboard.kde.org/r/107296/s/822/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/11/12/useswidget_new4_400x100.png" style="border: 1px black solid;" alt="The new useswidget after the latest updates" /></a>
<a href="http://git.reviewboard.kde.org/r/107296/s/832/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/11/14/useswidget_400x100.png" style="border: 1px black solid;" alt="Third version; the header line updates while uses are being searched" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>