<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/108569/">http://git.reviewboard.kde.org/r/108569/</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;">Quite nice, a good improvement on the existing UI. I have a bunch of comments on some of the details though. </pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/SnapSettings.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Snap Settings Docker</a></td>
</tr>
</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;">What's this? I'm guessing "enable snapping" but a random checkbox really does not work for that. Though I think this is a larger issue for this docker, tiny buttons with no description and non-obvious icons makes for a hard to learn UI.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/PathDrawingTool.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Freehand path drawing tool config widgets</a></td>
</tr>
</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;">This...</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/PathDrawingTool.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Freehand path drawing tool config widgets</a></td>
</tr>
</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;">... is less wide than this, which makes it look unbalanced. Giving them both a sizeHint should solve that.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/PathDrawingTool.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Freehand path drawing tool config widgets</a></td>
</tr>
</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;">This is using an awful lot of space while the two buttons next to it feel cramped. Try and play a bit with the spacing, you do not necessarily need to align the second row exactly to the first row. It would also help with equalising the width of the two arrow sections.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/PathDrawingTool.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Freehand path drawing tool config widgets</a></td>
</tr>
</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;">These have inequal heights which looks really awkward. Try and give them the same sizeHint for height.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/DefaultTools.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Default Tools config widgets</a></td>
</tr>
</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;">Why "Line/Border" when you use "Stroke" in the freehand tool? Why not stick with "Stroke" everywhere, considering that was also the term used in the old docker.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/DefaultTools.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Default Tools config widgets</a></td>
</tr>
</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;">Please see the Freehand tool for comments about this area.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/DefaultTools.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Default Tools config widgets</a></td>
</tr>
</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;">"No Fill" is not really distinguishable from "Solid Fill". The "Delete" icon used in the old docker works better, though you might want to use a grayscale variant of that.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/DefaultTools.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Default Tools config widgets</a></td>
</tr>
</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;">The preview is quite small here. What happens with the dropdown, does it have the same size previews?
Also, how does this look with Gradient fill? Does it use the same small preview?</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/DefaultTools.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Default Tools config widgets</a></td>
</tr>
</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;">This has the same issue as the Snap docker. Single checkboxes without apparent context are really confusing.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/DefaultTools.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Default Tools config widgets</a></td>
</tr>
</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;">Try and fix the alignment here. The labels and widgets for Blur and distance really should be aligned with eachother, now it just looks messy.</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/SnapSettings.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Snap Settings Docker</a></td>
</tr>
</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;">See also the checkbox: 10px. 10px of what?</pre>
<br />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/SnapSettings.jpg" style="color: black; font-weight: bold; font-size: 9pt;">Snap Settings Docker</a></td>
</tr>
</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;">I understand this is the current UI. I would really like to see a cleanup of this as a follow up though, since you are already busy with other parts of this UI. Please consider the comments for that follow-up.</pre>
<br />
<p>- Arjen</p>
<br />
<p>On January 24th, 2013, 2:12 p.m. UTC, Jean-Nicolas Artaud 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 Calligra.</div>
<div>By Jean-Nicolas Artaud.</div>
<p style="color: grey;"><i>Updated Jan. 24, 2013, 2:12 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 totally changes the design of the tools config widget use. This new way of doing had been discussed with several people (boemann and a UI designer worked 8 hours at the aKademy, and this part is one of the main thing to do. The manipulation of shapes on pages and slides is way easier now. The good widgets appear to you when you select a tool, and you know directly have what you need and want to manipulate the shapes!
- If you select a tool, it will open the config widgets associated.
- As you can see on the screenshots, all the tools needs their own widgets.
- Stroke, Fill and Shadow config widgets are the main one.
- A same config widget can be used with several tools.
- Snap Settings had been removed from config widgets and added as a Docker -- the opposite way that the StrokeDocker for instance.</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;">Tested in the flake-configwidgets-boemann branch, on words and stage, with several documents, and different kind of shapes.</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>karbon/plugins/tools/CalligraphyTool/KarbonCalligraphyTool.h <span style="color: grey">(7de7a14)</span></li>
<li>karbon/plugins/tools/CalligraphyTool/KarbonCalligraphyTool.cpp <span style="color: grey">(4833d86)</span></li>
<li>krita/CMakeLists.txt <span style="color: grey">(835f90d)</span></li>
<li>libs/basicflakes/tools/KoCreatePathTool.cpp <span style="color: grey">(043ec2e)</span></li>
<li>libs/widgets/KoFillConfigWidget.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/widgets/KoResourcePopupAction.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/widgets/KoShadowConfigWidget.cpp <span style="color: grey">(e9e9502)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108569/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/01/23/DefaultTools.jpg">Default Tools config widgets</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/NewDockersList.jpg">New docker list</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/PathDrawingTool.jpg">Freehand path drawing tool config widgets</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/23/SnapSettings.jpg">Snap Settings Docker</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>