<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/123833/">https://git.reviewboard.kde.org/r/123833/</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 25th, 2015, 8:45 a.m. UTC, <b>Dmitry Kazakov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="https://git.reviewboard.kde.org/r/123833/diff/3/?file=370740#file370740line45" style="color: black; font-weight: bold; text-decoration: underline;">krita/ui/input/kis_alternate_invocation_action.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">KisAlternateInvocationAction::KisAlternateInvocationAction()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">45</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">shortcuts</span><span class="p">.</span><span class="n">insert</span><span class="p">(</span><span class="n">i18n</span><span class="p">(</span><span class="s">"Tertiary Alternate Mode"</span><span class="p">),</span> <span class="n">TertiaryAlternateModeShortcut</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<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;">Hi, Michael!</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Could you remove "Alternate" word from these strings? It is excessive here. It prevents the text from fitting into the list widget. And the user already knows that it is related to the Alternale action because they are shown in "Alternate" group already.</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">Well, the "Alternate" is already part of the dialog itself -- it's in the heading. Adding it in every string makes the string very wide, too.</p></pre>
<br />
<p>- Boudewijn</p>
<br />
<p>On May 25th, 2015, 1:57 a.m. UTC, Michael Abrahams 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.</div>
<div>By Michael Abrahams.</div>
<p style="color: grey;"><i>Updated May 25, 2015, 1:57 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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This refactors polygonal, elliptical, and rectangular selection tools to use a basic selection tool template which unifies previously duplicated code. The template overrides the ability to execute alternate actions, but none of those tools supported alternate actions previously and the ellipse and rectangle were already overriding the modifier keys to begin with. </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Shift: add to selection
Alt: subtract from selection
Shift+Alt: intersect current selection
Ctrl: replace selection</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Certain key combinations allow users the ability to expose the modifier keys to the base tool, i.e. to make proportional / translated / scaled alterations using ctrl/alt/shift.
1) If the user <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">clicks first and then presses modifier keys</em>, those modifier keys will only alter the selection method.
2) If the user <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">presses modifier keys first and then clicks</em>, the selection method is locked in, and subsequent modifier keystrokes will change how the selection is drawn.
3) If the underlying tool <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">never takes modifier keys</em>, modifier keys will always alter the selection method. </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">These rules can be defined systematically by modifying the template.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Things to do later:
+ Basic functionality is implemented in KisToolSelectBase, which covers the outline, contiguous, similar color and path selection tools which inherit from KisToolSelectBase. A more complete refactoring might define KisToolSelectBase via the selection tool template, but it is not simple for the path selection tool which uses a more complicated delegated design pattern.
+ The tools need new icons (e.g. little plus/minus symbols) to give users visual feedback when they activate the modifiers.
+ The Ctrl key should switch temporarily to the move tool, as in Photoshop.
+ Check idiomatic naming conventions, style, etc.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There are no tests targeting the individual selection tools, but the tests for other individual tools passed.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The ability to add, subtract and intersect complicated shapes quickly exposes some bugs in how the selection marquees are drawn: sometimes extra lines are drawn, sometimes lines fail to be drawn.</p></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>krita/ui/tool/kis_tool_polyline_base.cpp <span style="color: grey">(6071f76)</span></li>
<li>krita/ui/tool/kis_tool_polyline_base.h <span style="color: grey">(f681fd8)</span></li>
<li>krita/ui/tool/kis_tool_paint.h <span style="color: grey">(128dce9)</span></li>
<li>krita/ui/tool/kis_tool.h <span style="color: grey">(e852311)</span></li>
<li>krita/ui/tool/kis_tool.cc <span style="color: grey">(b299b81)</span></li>
<li>krita/ui/input/kis_alternate_invocation_action.cpp <span style="color: grey">(48723bf)</span></li>
<li>krita/ui/input/kis_alternate_invocation_action.h <span style="color: grey">(b47c59e)</span></li>
<li>krita/ui/CMakeLists.txt <span style="color: grey">(1caef14)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_similar.cc <span style="color: grey">(b2c51d9)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_similar.h <span style="color: grey">(f701986)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_rectangular.cc <span style="color: grey">(331c6a4)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_rectangular.h <span style="color: grey">(5e88766)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_polygonal.cc <span style="color: grey">(9acca50)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_polygonal.h <span style="color: grey">(feee9cb)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_path.cc <span style="color: grey">(9f1a65c)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_path.h <span style="color: grey">(a67b584)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_outline.cc <span style="color: grey">(46cca47)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_outline.h <span style="color: grey">(4756870)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_elliptical.cc <span style="color: grey">(999f1a0)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_elliptical.h <span style="color: grey">(7b2cd2f)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_contiguous.cc <span style="color: grey">(5bd4d2f)</span></li>
<li>krita/plugins/tools/selectiontools/kis_tool_select_contiguous.h <span style="color: grey">(26310e2)</span></li>
<li>krita/image/kis_selection.h <span style="color: grey">(6376f874)</span></li>
<li>CMakeFiles/2.8.12.1/CMakeDetermineCompilerABI_CXX.bin <span style="color: grey">(PRE-CREATION)</span></li>
<li>krita/ui/tool/kis_tool_rectangle_base.h <span style="color: grey">(a0b470c)</span></li>
<li>krita/ui/tool/kis_tool_rectangle_base.cpp <span style="color: grey">(8e091d0)</span></li>
<li>krita/ui/tool/kis_tool_select_base.h <span style="color: grey">(500d6dd)</span></li>
<li>krita/ui/tool/kis_tool_select_base.cpp <span style="color: grey">(40779ad)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123833/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>