Review Request: Adds a page/slide navigator control for the statusbar, used in Stage & Flow

Thorsten Zachmann t.zachmann at zagge.de
Fri Jun 8 06:23:16 BST 2012


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

Ship it!


Please commit after fixing the details I noticied. If I miss someting on the inline stuff lets discuss it before committing. 

One thing I noticed which is not due to your code but only visible when your code is there is that if you enter the first/last page in the widget the actions do not get enabled/disabled and therefore it shows the old state of the actions, e.g. when entering the last page the next and last action should be disabled and the first/previous should be enabled.


libs/kopageapp/widgets/KoPageNavigator.h
<http://git.reviewboard.kde.org/r/105180/#comment11467>

    Please remove 4 spaces at the start of the line to be in line with the other classes we use.



libs/kopageapp/widgets/KoPageNavigatorButton_p.h
<http://git.reviewboard.kde.org/r/105180/#comment11468>

    Is there a specific reason to use inline here. I think the inline is not needed as it is definitely no performance critical code and without inlining the code side should get smaller ot do I miss something? 
    Please move the code to the cpp file.


- Thorsten Zachmann


On June 7, 2012, 6:14 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105180/
> -----------------------------------------------------------
> 
> (Updated June 7, 2012, 6:14 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> Currently in Stage/Flow the status bar just has a read-only display for current page number and pages count.
> Which also does not tell if the current pages are Master pages or not.
> 
> Attached patch fixes that, by turning the label into a display that turns into an editor, like the zoom controller.
> 
> Commit log:
>     It has 2 states:
>     in first state it displays
>     * type of slide/page (incl. if Master one)
>     * the number of the current page/slide
>     * the number of the page/slide count
>     in second state it offers control to
>     * go to first/previous/next/last slide/page
>     * go by entered number to slide/page
>     
>     Does not use the icons of the original go* actions, as they look to complex
> 
> 
> Diffs
> -----
> 
>   libs/kopageapp/CMakeLists.txt a353892 
>   libs/kopageapp/KoPAView.h 01fbdf1 
>   libs/kopageapp/KoPAView.cpp 6fccdb0 
>   libs/kopageapp/widgets/KoPageNavigator.h PRE-CREATION 
>   libs/kopageapp/widgets/KoPageNavigator.cpp PRE-CREATION 
>   libs/kopageapp/widgets/KoPageNavigatorButton.cpp PRE-CREATION 
>   libs/kopageapp/widgets/KoPageNavigatorButton_p.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105180/diff/
> 
> 
> Testing
> -------
> 
> Used in Flow and Stage.
> 
> 
> Screenshots
> -----------
> 
> Display mode
>   http://git.reviewboard.kde.org/r/105180/s/600/
> Edit mode
>   http://git.reviewboard.kde.org/r/105180/s/601/
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120608/69b876b3/attachment.htm>


More information about the calligra-devel mailing list