Review Request: Separate the so called style docker (stroke/fill docker) into a docker and a widget

[Inge Wallin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106633/#review19605
-----------------------------------------------------------


Here's my response to all the issues in the two reviews so far (thanks!).  Version 3 - with a better filename :) - will come up RSN.


plugins/dockers/CMakeLists.txt
<http://git.reviewboard.kde.org/r/106633/#comment15598>

    It is an aesthetic thing: First the factory, then the class then the helper classes.



plugins/dockers/styledocker/StrokeFillWidget.h
<http://git.reviewboard.kde.org/r/106633/#comment15599>

    Yes, this is the old updateStyle() which was badly named.  Update which style?  Inside the docker? In the selected objects? Somewhere else?
    
    I think updateWidget is more clear since it obviously updates the widget (from the object). If you have an even better name I'm all open to it.



plugins/dockers/styledocker/StrokeFillWidget.cpp
<http://git.reviewboard.kde.org/r/106633/#comment15600>

    It did to me very much when I first saw it. Now I know it since long but I think it's still surprising to non experts.



plugins/dockers/styledocker/StrokeFillWidget.cpp
<http://git.reviewboard.kde.org/r/106633/#comment15601>

    No, it is correct. To see this, insert a standard rectangle with the green gradient. Then press the "Solid color" button. The color selector changes to a color but the rectangle itself does not get a solid color. The rectangle is not changed until the color selector is activated.



plugins/dockers/styledocker/StyleDocker.h
<http://git.reviewboard.kde.org/r/106633/#comment15602>

    Hmm, good point.  I didn't notice it was a base class. 



plugins/dockers/styledocker/StyleDocker.cpp
<http://git.reviewboard.kde.org/r/106633/#comment15603>

    Indeed.  It was something that I came upon in an early stage and then forgot about.


- Inge Wallin


On Sept. 29, 2012, 2:35 p.m., Inge Wallin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106633/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2012, 2:35 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> This patch fulfills a very old wish of ours, namely the separation of the stroke/fill docker into a widget and an embedding docker. This makes the stroke/fill widget reusable in other places, e.g. a style manager for graphic styles.
> 
> I have tried to make the user experience exactly the same as for the old version even though there are lots of misfeatures with it.  Improving the user experience will come as future patches, and I don't want to do that without discussing it with other people.
> 
> 
> Diffs
> -----
> 
>   plugins/dockers/CMakeLists.txt c9b4f44 
>   plugins/dockers/styledocker/StrokeFillWidget.h PRE-CREATION 
>   plugins/dockers/styledocker/StrokeFillWidget.cpp PRE-CREATION 
>   plugins/dockers/styledocker/StyleDocker.h b0301e6 
>   plugins/dockers/styledocker/StyleDocker.cpp 376ffef 
> 
> Diff: http://git.reviewboard.kde.org/r/106633/diff/
> 
> 
> Testing
> -------
> 
> Extensive manual testing.
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120929/1d737af0/attachment.htm>