Review Request: Stage: Enhace behaviour of copy/paste slides in slide sorter

[Thorsten Zachmann]

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


I nearly had your other patch integrated. So I also added some comments about stuff I noticed when I was working on that.

Zooming works quite nicely. 

I'm not sure the standard min and max zoom level in this mode makes sense. What so you think about this? 

Should the zoom level when we enter the slide sorter mode be stored and then restored when we leave it to not be in a very big/small zoom afterwards?

The patch contains some code style problems mostly it is blank after ( and before ). Also there should be a blank after if and before {. It would be nice if you could update the patch accordingly.

Once you have updated the patch I will integrate. So please don't add new features to that patch so that we can get it in. You can then work on what is already in.


kpresenter/part/KPrSlidesSorterDocumentModel.h
<http://git.reviewboard.kde.org/r/100672/#comment1541>

    Does the class needs to be exported?



kpresenter/part/KPrSlidesSorterDocumentModel.h
<http://git.reviewboard.kde.org/r/100672/#comment1540>

    Please move the includes to the cpp file and use a forward declaration in the header.



kpresenter/part/KPrSlidesSorterDocumentModel.h
<http://git.reviewboard.kde.org/r/100672/#comment1539>

    Please move the implementation into the cpp file
    



kpresenter/part/KPrSlidesSorterDocumentModel.cpp
<http://git.reviewboard.kde.org/r/100672/#comment1542>

    I think we should use the slide name here and only if no slide name is set use the default Slide %1.
    
    Have a look at KoPADocumentModel::data
    



kpresenter/part/KPrViewModeSlidesSorter.h
<http://git.reviewboard.kde.org/r/100672/#comment1543>

    Please move the include to the cpp file



kpresenter/part/KPrViewModeSlidesSorter.cpp
<http://git.reviewboard.kde.org/r/100672/#comment1544>

    The file is already includes in the header so it can be removed here.


- Thorsten


On March 9, 2011, 10:46 p.m., Paul Mendez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100672/
> -----------------------------------------------------------
> 
> (Updated March 9, 2011, 10:46 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> Correct how slides sorter updates when add/delete/copy/paste a slide. Add some copy/paste extra context menu, and the supr key for delete slides in slides sorter.
> 
> Any suggestion for add Contrl C/V/X support?
> 
> 
> Diffs
> -----
> 
>   libs/kopageapp/KoPADocumentStructureDocker.cpp 08fb66a 
>   libs/kopageapp/KoPAView.h 55a6b7d 
>   kpresenter/part/KPrViewModeSlidesSorter.cpp 24e6c1e 
>   kpresenter/part/CMakeLists.txt 61c9848 
>   kpresenter/part/KPrSlidesSorterDocumentModel.h e69de29 
>   kpresenter/part/KPrSlidesSorterDocumentModel.cpp e69de29 
>   kpresenter/part/KPrViewModeSlidesSorter.h 743fbe9 
> 
> Diff: http://git.reviewboard.kde.org/r/100672/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Paul
> 
>

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