Review Request: Replace the current half baked style selection widgets with a home brewed comboBox

[Pierre Stirnweiss]

> On Jan. 16, 2012, 6:36 a.m., C. Boemann wrote:
> > I've committed three fixes.
> > 
> > I also note that:
> >  -paragraph combo always has + sign
> >  -default paragraph style should have a name (Default? None?)
> >  -launching stylemanager doesn't launch it with selected style
> >  -the combo should popup nomatter where you click in it (imho)
> > 
> > However I think we have come to the point where this code is better suited to live in master. There are so many other style issues in master and being able to work on it all in one place increases productivity. Besides it's a HUGE improvement already
> > 
> > I would prefer if we don't merge the branch but rather take the entire and make 2-3 new commits out of it. What do you think.
> >

- Where are the fixes? I don't seem to find them as a new revision of the diff.
- paragraph shows +: In which file is the paragraph always showing a + sign. It doesn't here.
- default should have a name: Well, there are both a "Standard" and a "Default" paragraph style. There is also a "" one which is selected at the begining. When you had your "non-visible" char patch applied, "Standard" was selected in the comboBox, as it is the style the template specifies for the blank document. I think this problem is outside the scope of the combo. Also when the "Special button" was used, nothing was selected ti start with.
- launching style manager: Indeed it doesn't, but in order to do that some changes are needed in the styleManager. I didn't want to start with this one on top of the comboBox. I was planning on working on the styleManager soon. There is also a StylesWidget in it, that needs replacing with a QListView, so we can get rid of the StylesWidget altogether.
- combo popup: i wondered about that one too. shouldn't be too hard i guess. (like emit the signal in the StylesComboPreview, and connect it in the StylesCombo).

About the merging, what would 3 commits bring? I actually doubt we would be able to cut this branch in 2-3 self contained (ie compilable and runnable) commits without quite a tiresome "git add --interactive" exercise. Everything is intertwined. I have made sure the branch compiles and run without major problems. I think a complete merge is the safest thing to do.


- Pierre


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


On Jan. 15, 2012, 7:41 p.m., Pierre Stirnweiss wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103677/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2012, 7:41 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> Merge request.
> Merge branch textshape_stylesWidget_pierreSt into master.
> The branch provides a replacement for the style selection widgets. The new provided widgets are a subclass of QComboBox.
> They provide a style preview both in the main area and in the dropdown menu.
> The dropdown menu provides a "show style manager" button, which call the whole style management dialog.
> The dropdown also provides a "delete style" button, although this is for the moment disabled/commented out, because deleting a style is not currently really supported.
> The combo also allows to create a style based on the current text format if it does not correspond to the original style (ie. the user changed a property).
> 
> 
> Diffs
> -----
> 
>   libs/kotext/KoTextEditor.h 0fc4ff4 
>   libs/kotext/KoTextEditor.cpp 3ef31d6 
>   libs/kotext/styles/KoCharacterStyle.h 898ba43 
>   libs/kotext/styles/KoCharacterStyle.cpp e575b0d 
>   libs/kotext/styles/KoParagraphStyle.h 12ab672 
>   libs/textlayout/CMakeLists.txt af6d64a 
>   libs/textlayout/FrameIterator.h c73d580 
>   libs/textlayout/KoStyleThumbnailer.h 8645089 
>   libs/textlayout/KoStyleThumbnailer.cpp 3529bbb 
>   plugins/textshape/CMakeLists.txt fad7c04 
>   plugins/textshape/TextTool.h 17a687b 
>   plugins/textshape/TextTool.cpp 3ba9d33 
>   plugins/textshape/dialogs/SimpleCharacterWidget.h ad944a5 
>   plugins/textshape/dialogs/SimpleCharacterWidget.cpp 6b2bb66 
>   plugins/textshape/dialogs/SimpleCharacterWidget.ui e4f6d3b 
>   plugins/textshape/dialogs/SimpleParagraphWidget.h bc167b4 
>   plugins/textshape/dialogs/SimpleParagraphWidget.cpp cbfacfe 
>   plugins/textshape/dialogs/SimpleParagraphWidget.ui b2e9adf 
>   plugins/textshape/dialogs/StylesCombo.h PRE-CREATION 
>   plugins/textshape/dialogs/StylesCombo.cpp PRE-CREATION 
>   plugins/textshape/dialogs/StylesComboPreview.h PRE-CREATION 
>   plugins/textshape/dialogs/StylesComboPreview.cpp PRE-CREATION 
>   plugins/textshape/dialogs/StylesDelegate.h f5d216f 
>   plugins/textshape/dialogs/StylesDelegate.cpp d1021f5 
>   plugins/textshape/dialogs/StylesModel.h dbd17b4 
>   plugins/textshape/dialogs/StylesModel.cpp 94deb24 
>   plugins/textshape/dialogs/StylesWidget.cpp 4f31a01 
>   plugins/textshape/dialogs/TableOfContentsStyleModel.cpp 48ee7c3 
> 
> Diff: http://git.reviewboard.kde.org/r/103677/diff/diff
> 
> 
> Testing
> -------
> 
> The functionality exclusively linked to the comboBox seems to be working properly. Some defects have been identified (like no paragraph style is selected at startup), but these are actually outside the scope of this widget and affecting the current widget also.
> 
> 
> Thanks,
> 
> Pierre Stirnweiss
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120116/9f908f1b/attachment.htm>