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

[Boudewijn Rempt]

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


Nice cleanup. I found some places to niggle, but apart from that, I'm fine with this. It applies, compiles, works and is an improvement, imo.


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

    Why the change in order? Changing the order of items in CMakeLists.txt often makes for difficult merges.



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

    updateWidget probably needs to be a bit more expressively named, since it is, I guess, the old updateStyle. Maybe updateWidgetStyleSettings or something like that.



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

    Why all the space between type and variable name?



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

    Er, yes, that doesn't come as a surprise?



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

    This looks weird, and is one of the places were if you want to push it like this I really would like a comment.


- Boudewijn Rempt


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/e9c647a7/attachment.htm>