----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123833/#review81066 -----------------------------------------------------------
For the record: https://bugs.kde.org/show_bug.cgi?id=348234 is where we're tracking the issues with this patch. It's looking good, but there are still a few! - Boudewijn Rempt On May 26, 2015, 8:47 a.m., Michael Abrahams wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123833/ > ----------------------------------------------------------- > > (Updated May 26, 2015, 8:47 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 > ----- > > krita/ui/tool/kis_tool_polyline_base.cpp 6071f76 > krita/ui/tool/kis_tool_polyline_base.h f681fd8 > krita/ui/tool/kis_tool_paint.h 128dce9 > krita/ui/tool/kis_tool.h e852311 > krita/ui/tool/kis_tool.cc b299b81 > krita/ui/input/kis_alternate_invocation_action.cpp 48723bf > krita/ui/input/kis_alternate_invocation_action.h b47c59e > krita/ui/CMakeLists.txt 1caef14 > krita/plugins/tools/selectiontools/kis_tool_select_similar.cc b2c51d9 > krita/plugins/tools/selectiontools/kis_tool_select_similar.h f701986 > krita/plugins/tools/selectiontools/kis_tool_select_rectangular.cc 331c6a4 > krita/plugins/tools/selectiontools/kis_tool_select_rectangular.h 5e88766 > krita/plugins/tools/selectiontools/kis_tool_select_polygonal.cc 9acca50 > krita/plugins/tools/selectiontools/kis_tool_select_polygonal.h feee9cb > krita/plugins/tools/selectiontools/kis_tool_select_path.cc 9f1a65c > krita/plugins/tools/selectiontools/kis_tool_select_path.h a67b584 > krita/plugins/tools/selectiontools/kis_tool_select_outline.cc 46cca47 > krita/plugins/tools/selectiontools/kis_tool_select_outline.h 4756870 > krita/plugins/tools/selectiontools/kis_tool_select_elliptical.cc 999f1a0 > krita/plugins/tools/selectiontools/kis_tool_select_elliptical.h 7b2cd2f > krita/plugins/tools/selectiontools/kis_tool_select_contiguous.cc 5bd4d2f > krita/plugins/tools/selectiontools/kis_tool_select_contiguous.h 26310e2 > krita/image/kis_selection.h 6376f874 > CMakeFiles/2.8.12.1/CMakeDetermineCompilerABI_CXX.bin PRE-CREATION > 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
