[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-kimageshop
Subject: Re: Review Request: added a baseclass for all selection tools
From: Sven Langkamp <sven.langkamp () gmail ! com>
Date: 2009-12-19 2:46:47
Message-ID: 478b087a0912181846p676780c3pafc6a8ad32e2f5db () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Sat, Dec 19, 2009 at 3:04 AM, Adam <nospam@xibo.at> wrote:
> On Fri, 18 Dec 2009 23:09:11 +0100, Sven Langkamp <sven.langkamp@gmail.com>
> wrote:
>
> On Fri, Dec 18, 2009 at 6:40 PM, Adam <nospam@xibo.at> wrote:
>>
>> Somehow this notification didn't get through..
>>>
>>> -----------------------------------------------------------
>>> This is an automatically generated e-mail. To reply, visit:
>>> http://reviewboard.kde.org/r/2415/
>>> -----------------------------------------------------------
>>>
>>> Review request for KOffice.
>>>
>>>
>>> Summary
>>> -------
>>>
>>> I added KisToolSelectBase as a base class for all selection tools and
>>> refactored the tools in the selectiontools directory to use it. If this
>>> patch will be accepted, I will also move KisToolSelectSimilar to the
>>> selectiontools dir, add the new parent and implement the junior job
>>> "implement shortcuts to switch between selection modes (add, subtract,
>>> replace) for the Krita selection tools".
>>>
>>> I noticed 2 bugs (crash in path select and lines not visible in
>>> polygonal,
>>> already in the old version present), which I will also try to fix.
>>> KisToolSelectPolygonal will need a new shortcut for deactivating.
>>>
>>>
>> Looks good. I think it would be better to put the new base class in
>> krita/ui/tool, makes it easier reusable.
>>
>> For the temporary line painting have a look at the shape tools. Lukas did
>> already fix it there.
>> While you are working on the actions you could also switch the comboboxes
>> in
>> that widget to button groups.
>>
>
> ok..
> I'm unsure, what you mean by button group. Something like this? [see
> attachment]
> Imo such a group box needs very much space, but otherwise this toolbox
> won't be visible the whole time, so it shouldn't be a problem.
>
Yes, like that. See QButtonGroup.
Instead of putting a box around them just align them in a row like Gimp
does. For the action button we have to replace the text by icons so that
they fit into one row too.
I have seen some mockups about future plans for that like
http://kexi-project.org/pics/2.2/alpha1/kexi-2.2-alpha1-groupbutton-mockup3.png,
but for now simple buttons will do.
[Attachment #5 (text/html)]
<div class="gmail_quote">On Sat, Dec 19, 2009 at 3:04 AM, Adam <span dir="ltr"><<a \
href="mailto:nospam@xibo.at">nospam@xibo.at</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt \
0pt 0.8ex; padding-left: 1ex;"> <div><div></div><div class="h5">On Fri, 18 Dec 2009 \
23:09:11 +0100, Sven Langkamp <<a href="mailto:sven.langkamp@gmail.com" \
target="_blank">sven.langkamp@gmail.com</a>> wrote:<br> <br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); \
margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> On Fri, Dec 18, 2009 at 6:40 PM, Adam \
<<a href="mailto:nospam@xibo.at" target="_blank">nospam@xibo.at</a>> wrote:<br> \
<br> <blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, \
204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> Somehow this notification \
didn't get through..<br> <br>
-----------------------------------------------------------<br>
This is an automatically generated e-mail. To reply, visit:<br>
<a href="http://reviewboard.kde.org/r/2415/" \
target="_blank">http://reviewboard.kde.org/r/2415/</a><br>
-----------------------------------------------------------<br>
<br>
Review request for KOffice.<br>
<br>
<br>
Summary<br>
-------<br>
<br>
I added KisToolSelectBase as a base class for all selection tools and<br>
refactored the tools in the selectiontools directory to use it. If this<br>
patch will be accepted, I will also move KisToolSelectSimilar to the<br>
selectiontools dir, add the new parent and implement the junior job<br>
"implement shortcuts to switch between selection modes (add, subtract,<br>
replace) for the Krita selection tools".<br>
<br>
I noticed 2 bugs (crash in path select and lines not visible in polygonal,<br>
already in the old version present), which I will also try to fix.<br>
KisToolSelectPolygonal will need a new shortcut for deactivating.<br>
<br>
</blockquote>
<br>
Looks good. I think it would be better to put the new base class in<br>
krita/ui/tool, makes it easier reusable.<br>
<br>
For the temporary line painting have a look at the shape tools. Lukas did<br>
already fix it there.<br>
While you are working on the actions you could also switch the comboboxes in<br>
that widget to button groups.<br>
</blockquote>
<br></div></div>
ok..<br>
I'm unsure, what you mean by button group. Something like this? [see \
attachment]<br> Imo such a group box needs very much space, but otherwise this \
toolbox won't be visible the whole time, so it shouldn't be a \
problem.<br></blockquote><div><br>Yes, like that. See QButtonGroup.<br>Instead of \
putting a box around them just align them in a row like Gimp does. For the action \
button we have to replace the text by icons so that they fit into one row too.<br> \
</div></div><br>I have seen some mockups about future plans for that like <a \
href="http://kexi-project.org/pics/2.2/alpha1/kexi-2.2-alpha1-groupbutton-mockup3.png" \
>http://kexi-project.org/pics/2.2/alpha1/kexi-2.2-alpha1-groupbutton-mockup3.png</a>, \
> but for now simple buttons will do.<br>
_______________________________________________
kimageshop mailing list
kimageshop@kde.org
https://mail.kde.org/mailman/listinfo/kimageshop
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic