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


In general I must say that I like this. I have worked with it a little and I 
like the feeling. I also like the fact that the scrollbars are not present when 
they don't need to be.

The only issue I found while working with it is that if I move a shape to the 
slide's border so that it's halfway outside and halfway inside, then the slide 
border is shown through it (or the border is painted on top of it). I think 
this is a really minor issue but if it's easy to fix then I guess it should be.

I had some code issues as well, but also minor ones. Maybe the naming issue I 
point out below should be rethought.


libs/kopageapp/KoPACanvas.cpp
<http://git.reviewboard.kde.org/r/104928/#comment10936>

    This is totally obscure to me.  I think there should be a comment to 
explain what's happening.



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

    This is not the size of the document is it? AFAIU, it's the size of the 
slide contents and the viewport combined, right? If so the name is misleading.



libs/widgets/KoZoomController.h
<http://git.reviewboard.kde.org/r/104928/#comment10938>

    That's not your email :)



libs/widgets/KoZoomController.h
<http://git.reviewboard.kde.org/r/104928/#comment10939>

    While 'document' may be what it's called in the zoom controller, I think it 
is confusing. Granted, the size of the document is not clearly defined in a 
page app, but I still think it makes more sense to call it workareaSize() or 
something. At least that's not possible to confuse with any size from 
KoDocument (if there is anything like that). In general, reusing names for 
different concapts is a reciepe for confusion.


- Inge Wallin


On May 13, 2012, 1:31 a.m., C. Boemann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104928/
> -----------------------------------------------------------
> 
> (Updated May 13, 2012, 1:31 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> KoPageApp applications Stage and Flow used to have extra space reserved 
> around the page/slide so objects could be put there. However that made it 
> really hard for the user to work with.
> 
> This patch changes that so there si no space around the page/slide. But.. 
> instead you can drag objects outside the page/slide without any limit at all
> 
> 
> Diffs
> -----
> 
>   libs/kopageapp/KoPACanvas.cpp 4a4748c 
>   libs/kopageapp/KoPAView.h 01fbdf1 
>   libs/kopageapp/KoPAView.cpp 4d13dfa 
>   libs/kopageapp/KoPAViewMode.cpp 6981382 
>   libs/widgets/KoZoomController.h 4f4c6a2 
>   libs/widgets/KoZoomController.cpp e10c5b4 
>   stage/part/KPrViewModeNotes.h b707770 
>   stage/part/KPrViewModeNotes.cpp c951ade 
> 
> Diff: http://git.reviewboard.kde.org/r/104928/diff/
> 
> 
> Testing
> -------
> 
> I've tested zooming in and out in Stage normal, notes, and sorter viewmodes, 
> and in Flow.
> 
> I've tested that running presentations still work, however I've not tested 
> presenterview.
> 
> I've tested moving shapes outside the page/slide in both Stage and Flow.
> 
> 
> Thanks,
> 
> C. Boemann
> 
>

_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

Reply via email to