Review Request: GSoC, Stage: Add a tabbar to switch between view modes

[Thorsten Zachmann]

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



kpresenter/part/KPrView.cpp
<http://git.reviewboard.kde.org/r/102198/#comment4947>

    Is there a reason why it can not be initialized as in the commented out line 72. 



kpresenter/part/KPrView.cpp
<http://git.reviewboard.kde.org/r/102198/#comment4968>

    I think it would be good if that would be done in the view modes themselfs



kpresenter/part/KPrView.cpp
<http://git.reviewboard.kde.org/r/102198/#comment4963>

    The opening { should be at the same line as the switch. The {} in the case statements are not needed. And if I remember correctly the case statements should be at the same indent level as the switch statement.



libs/kopageapp/KoPAView.cpp
<http://git.reviewboard.kde.org/r/102198/#comment4964>

    In Private class the m_ prefix is not used. Also it better to not appreviate the variable name. Please rename to horizontalTabBar and the like.
    
    I'm not sure we need 2 tab bars. One should  be enough and there should be a method to change the position which will then place it at a different place in the grid.



libs/kopageapp/KoPAView.cpp
<http://git.reviewboard.kde.org/r/102198/#comment4965>

    It is not very clear what is meant by this variable. Please rename to something that makes it more clear like tabBarLayout.



libs/kopageapp/KoPAView.cpp
<http://git.reviewboard.kde.org/r/102198/#comment4966>

    Is there a need for this variables?



libs/kopageapp/KoPAView.cpp
<http://git.reviewboard.kde.org/r/102198/#comment4967>

    This API is not very clear. If a user calles 3 times replaceCentralWidget without calling restoreCentralWidget there will be unwanted side effects. Any change to improve that?


- Thorsten


On Aug. 8, 2011, 9:36 p.m., Paul Mendez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102198/
> -----------------------------------------------------------
> 
> (Updated Aug. 8, 2011, 9:36 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> Add a tabbar to Stage on top of views, to switch between normal, notes and slides sorter view.
> 
> 
> Diffs
> -----
> 
>   kpresenter/part/KPrView.h 8c9d46158171b38363fdd9e716f4ab3d3b193200 
>   kpresenter/part/KPrView.cpp fb34504c8143c6cfd7cf5e6c11908897fa99cce9 
>   kpresenter/part/KPrViewModeSlidesSorter.cpp 9941f4bbde74d4cc3566eecdf314815876c482ab 
>   libs/kopageapp/KoPAView.h ee9b7950cebad23b2303bbce72e5164e4f42e1e5 
>   libs/kopageapp/KoPAView.cpp db61b647477194f062afd4f4f6ef9b6010b2f9a9 
>   libs/kopageapp/KoPAViewMode.h f47e07a2d1587e88f8d73d39ce902e4dc76c7942 
>   libs/kopageapp/KoPAViewMode.cpp cb9829a8287e1998e968ed196b98846ac2aef16f 
> 
> Diff: http://git.reviewboard.kde.org/r/102198/diff
> 
> 
> Testing
> -------
> 
> build and run
> 
> 
> Thanks,
> 
> Paul
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110810/35057f0c/attachment.htm>