<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/102871/">http://git.reviewboard.kde.org/r/102871/</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;">In general I like this change. It lessens the problem with dockers being too big or many to fit in the dockwidgets. It improves the overview a lot. But I have a big problem with the part that removes certain tools; see the extended comment below.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/102871/diff/1/?file=38913#file38913line32" style="color: black; font-weight: bold; text-decoration: underline;">karbon/plugins/tools/KarbonGradientToolFactory.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">32</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setToolType</span><span class="p">(</span><span class="n"><span class="hl">mainToolType</span></span><span class="p"><span class="hl">()</span>);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">32</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setToolType</span><span class="p">(</span><span class="s"><span class="hl">"karbon"</span></span><span class="p">);</span></pre></td>
</tr>
</tbody>
</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'm very uncomfortable with this change and the next for the following reasons:
* Graphics contents, including gradients and patterns, is very common in text documents. In my opinion these tools should be made part of the standard Calligra set of tools for productivity applications instead of being disabled. But I guess that's a topic for the mailing list rather than the review board. Or perhaps even the sprint.
* I think we should have a similar set of tools, especially for graphics, in the 3 productivity apps Words, Stage and Tables. Only disabling them in Words leads to surprises for users.
* I don't think these changes are so tightly connected to the new mode box that they need to be in the same patch even if they were voted through.
So I suggest removing them for this patch and start a thread on them on the ML. I'm more hesitant about the change for the calligraphy tool and the filter effect tool, so I'll just keep quiet on them.</pre>
</div>
<br />
<p>- Inge</p>
<br />
<p>On October 15th, 2011, 4:49 p.m., C. Boemann 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 Calligra.</div>
<div>By C. Boemann.</div>
<p style="color: grey;"><i>Updated Oct. 15, 2011, 4:49 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 UI was conceived during the KDE User Experience sprint in Berlin 2011
It basically provides a drop-in-replacement for the toolbox + tooloptionsdocker in the form of a single docker containing QToolBox widget.
The new KoModeBox is used in Words only. All the other applications should be left unchanged.
The tools have a new extended usage of the Type attribute. See the KoToolFactoryBase.h changes for description. It should be compatible with the krita usage.
The only visible changes outside Words would probably be that the gradient, pattern,calligraphy tools end up in a separate section of the toolbox.</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/KarbonCalligraphyToolFactory.cpp <span style="color: grey">(cb1c092)</span></li>
<li>karbon/plugins/tools/KarbonGradientToolFactory.cpp <span style="color: grey">(786d98a)</span></li>
<li>karbon/plugins/tools/KarbonPatternToolFactory.cpp <span style="color: grey">(1ec250fa)</span></li>
<li>karbon/plugins/tools/KarbonPencilToolFactory.cpp <span style="color: grey">(4fd1105)</span></li>
<li>karbon/plugins/tools/filterEffectTool/KarbonFilterEffectsToolFactory.cpp <span style="color: grey">(c30f1be)</span></li>
<li>libs/flake/KoToolFactoryBase.h <span style="color: grey">(73f2534)</span></li>
<li>libs/flake/KoToolManager.cpp <span style="color: grey">(3bcae08)</span></li>
<li>libs/main/CMakeLists.txt <span style="color: grey">(9539a5a)</span></li>
<li>libs/main/KoDockerManager.h <span style="color: grey">(7e44455)</span></li>
<li>libs/main/KoDockerManager.cpp <span style="color: grey">(7f5b17d)</span></li>
<li>libs/main/KoModeBox.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/main/KoModeBoxFactory.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/main/KoModeBoxFactory.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/main/KoModeBox_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/main/KoToolBox.cpp <span style="color: grey">(ede46d4)</span></li>
<li>plugins/textshape/ReferencesToolFactory.cpp <span style="color: grey">(096e602)</span></li>
<li>plugins/textshape/ReviewToolFactory.cpp <span style="color: grey">(bbd6452)</span></li>
<li>plugins/textshape/TextToolFactory.cpp <span style="color: grey">(438846c)</span></li>
<li>words/part/KWGui.cpp <span style="color: grey">(69d309b)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102871/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/102871/s/306/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/10/15/wordsui_400x100.png" style="border: 1px black solid;" alt="screentshot of the new ui" /></a>
<a href="http://git.reviewboard.kde.org/r/102871/s/307/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/10/15/inkarbon_400x100.png" style="border: 1px black solid;" alt="toolbox in karbon" /></a>
<a href="http://git.reviewboard.kde.org/r/102871/s/308/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/10/15/inkrita_400x100.png" style="border: 1px black solid;" alt="toolbox in krita" /></a>
<a href="http://git.reviewboard.kde.org/r/102871/s/309/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/10/15/instage_400x100.png" style="border: 1px black solid;" alt="toolbox in stage" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>