> On May 19, 2015, 10:15 a.m., Boudewijn Rempt wrote:
> > krita/ui/input/kis_alternate_invocation_action.cpp, line 43
> > <https://git.reviewboard.kde.org/r/123833/diff/1/?file=369772#file369772line43>
> >
> >     These renames seem to be problematic, we need to get rid of the 
> > "Alternate" in the string.
> >     
> >     It's weird though, since it suggests we're using translated texts in 
> > our config files, which is a big problem.
> 
> Michael Abrahams wrote:
>     I don't think the names are the problem, I think it's the fuzzy thinking 
> behind the names.  What is alternate?  What is secondary? What types of 
> actions are appropriate for what sorts of modifier keys? Those terms have 
> rigorous meaning in terms of the KOTool API, but are of no interest to the 
> user in the context of Krita. 
>     
>     A user cares about: what can I customize and what can't I? Is there 
> anything annoying enough that I'm going to try to tweak it? How 
> straightforward is it to do these customizations?
>     
>     You'll notice this patch does not hook into the current customization 
> system.  There is one button for each selection mode and they are hard coded. 
>  I plan to bind Ctrl to the "move" tool.  I can't see anybody complaining 
> about this, it is simply the standard used by every other program.

Perhaps my wording was a little harsh.  The point is that "Primary" and 
"Alternate" are perfectly useful terms defined in terms of the API, but as nice 
a function as it is, i18n() does not translate API into English.


- Michael


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123833/#review80619
-----------------------------------------------------------


On May 25, 2015, 1:38 a.m., Michael Abrahams wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123833/
> -----------------------------------------------------------
> 
> (Updated May 25, 2015, 1:38 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> 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. 
> 
> Shift: add to selection
> Alt: subtract from selection
> Shift+Alt: intersect current selection
> Ctrl: replace selection
> 
> 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 *clicks first and then presses modifier keys*, those modifier 
> keys will only alter the selection method.
> 2) If the user *presses modifier keys first and then clicks*, the selection 
> method is locked in, and subsequent modifier keystrokes will change how the 
> selection is drawn.
> 3) If the underlying tool *never takes modifier keys*, modifier keys will 
> always alter the selection method. 
> 
> These rules can be defined systematically by modifying the template.
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   CMakeFiles/2.8.12.1/CMakeDetermineCompilerABI_CXX.bin PRE-CREATION 
>   krita/image/kis_selection.h 6376f874 
>   krita/plugins/tools/selectiontools/kis_tool_select_contiguous.h 26310e2 
>   krita/plugins/tools/selectiontools/kis_tool_select_contiguous.cc 5bd4d2f 
>   krita/plugins/tools/selectiontools/kis_tool_select_elliptical.h 7b2cd2f 
>   krita/plugins/tools/selectiontools/kis_tool_select_elliptical.cc 999f1a0 
>   krita/plugins/tools/selectiontools/kis_tool_select_outline.h 4756870 
>   krita/plugins/tools/selectiontools/kis_tool_select_outline.cc 46cca47 
>   krita/plugins/tools/selectiontools/kis_tool_select_path.cc 9f1a65c 
>   krita/plugins/tools/selectiontools/kis_tool_select_polygonal.h feee9cb 
>   krita/plugins/tools/selectiontools/kis_tool_select_polygonal.cc 9acca50 
>   krita/plugins/tools/selectiontools/kis_tool_select_rectangular.h 5e88766 
>   krita/plugins/tools/selectiontools/kis_tool_select_rectangular.cc 331c6a4 
>   krita/plugins/tools/selectiontools/kis_tool_select_similar.h f701986 
>   krita/plugins/tools/selectiontools/kis_tool_select_similar.cc b2c51d9 
>   krita/ui/CMakeLists.txt 1caef14 
>   krita/ui/input/kis_alternate_invocation_action.h b47c59e 
>   krita/ui/input/kis_alternate_invocation_action.cpp 48723bf 
>   krita/ui/tool/kis_tool.h e852311 
>   krita/ui/tool/kis_tool.cc b299b81 
>   krita/ui/tool/kis_tool_paint.h 128dce9 
>   krita/ui/tool/kis_tool_polyline_base.h f681fd8 
>   krita/ui/tool/kis_tool_polyline_base.cpp 6071f76 
>   krita/ui/tool/kis_tool_rectangle_base.h a0b470c 
>   krita/ui/tool/kis_tool_rectangle_base.cpp 8e091d0 
>   krita/ui/tool/kis_tool_select_base.h 500d6dd 
>   krita/ui/tool/kis_tool_select_base.cpp 40779ad 
> 
> Diff: https://git.reviewboard.kde.org/r/123833/diff/
> 
> 
> Testing
> -------
> 
> There are no tests targeting the individual selection tools, but the tests 
> for other individual tools passed.
> 
> 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.
> 
> 
> Thanks,
> 
> Michael Abrahams
> 
>

_______________________________________________
calligra-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/calligra-devel

Reply via email to