<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/115512/">https://git.reviewboard.kde.org/r/115512/</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">in genral i think the approach looks sane enough, except the undo/redo - why create new shape after undo redo - commands further down the stack will probably expect the same shape pointer to still be valid (just like you expect the containers to still be valid on the second call to redo</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think this should be fixed</p></pre>
<br />
<p>- Camilla Boemann</p>
<br />
<p>On October 21st, 2014, 5:43 a.m. UTC, Yue Liu 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 Calligra and Camilla Boemann.</div>
<div>By Yue Liu.</div>
<p style="color: grey;"><i>Updated Oct. 21, 2014, 5:43 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</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;">Added "Add Text" action to popup menu in DefaultTool, if an application don't want it appear in popup menu, set AllowAddTextToShapePopupAction=false in rc file. Currently disabled in Words, Sheets, and Kexi.
There is a problem that KoTosContainer doesn't have API to delete text shapes in it, so undo doesn't work.
Another thing is this action supports adding text to multiple shapes, after texts are added, should it activate text tool automatically? If there are multiple shapes, which one should have text tool activated? The first in list, or the one under cursor? If the shape under cursor is not a KoTosContainer, then which KoTosContainer should have TextTool activated? My current idea is if the shape under cursor is KoTosContainer, then activate TextTool on it, otherwise don't activate TextTool.</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;">Works on OSX
You can also test through branch add-text-popup-action-yue</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>kexi/data/kexirc <span style="color: grey">(4c73bd0)</span></li>
<li>libs/flake/CMakeLists.txt <span style="color: grey">(839b861)</span></li>
<li>libs/flake/commands/KoTosContainerAddTextCommand.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/flake/commands/KoTosContainerAddTextCommand.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/defaultTools/defaulttool/DefaultTool.h <span style="color: grey">(dca6ae8)</span></li>
<li>plugins/defaultTools/defaulttool/DefaultTool.cpp <span style="color: grey">(453a727)</span></li>
<li>sheets/sheetsrc <span style="color: grey">(9b622fb)</span></li>
<li>words/part/wordsrc <span style="color: grey">(4cd2801)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115512/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>