Review Request 122266: split KWRootAreaProvider into 3 classes for better overview and solving text flwing between textboxes

Camilla Boemann cbr at boemann.dk
Tue Jan 27 09:10:28 GMT 2015



> On Jan. 26, 2015, 11:38 p.m., Inge Wallin wrote:
> > words/part/KWRootAreaProviderBase.cpp, line 157
> > <https://git.reviewboard.kde.org/r/122266/diff/1/?file=345325#file345325line157>
> >
> >     I think instead that all layout should be in shape coordinates, i.e. from (0, 0) to (sizeX, sizeY). This handling of world coordinates inside a shape is an abomination to all that is good in linear algebra.
> >     
> >     But I understand that this is perhaps not the time and place to change that...

well it's about finding other shapes that overlap, so I see nothing wrong with doing that in world coordinates. In fact see it as the most logical choice tbh


> On Jan. 26, 2015, 11:38 p.m., Inge Wallin wrote:
> > words/part/KWRootAreaProviderBase.cpp, line 64
> > <https://git.reviewboard.kde.org/r/122266/diff/1/?file=345325#file345325line64>
> >
> >     This needs to be handled by a method in KoBorder.  But this is not enough, is it?  I see you take care about padding below, but you also need to take into account margin, right?

a utility method in KoBorder would be nice yes, but this is okay for me though.

Margin is handled inside the library iirc, but I am actually a little uncertain


> On Jan. 26, 2015, 11:38 p.m., Inge Wallin wrote:
> > words/part/KWRootAreaProviderBase.h, line 24
> > <https://git.reviewboard.kde.org/r/122266/diff/1/?file=345324#file345324line24>
> >
> >     The base class for a root area provider should hardly include a Words specific datatype.

that is why its a words class and not a library class


> On Jan. 26, 2015, 11:38 p.m., Inge Wallin wrote:
> > words/part/CMakeLists.txt, lines 32-33
> > <https://git.reviewboard.kde.org/r/122266/diff/1/?file=345321#file345321line32>
> >
> >     These files shouldn't really be in Words, should they?  Text boxes are useful in all ODF applications.

these are handling words specific stuff so yes they should be here.

The library should in a perfect world not even know about shapes


- Camilla


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122266/#review74797
-----------------------------------------------------------


On Jan. 26, 2015, 11:17 p.m., Camilla Boemann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122266/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 11:17 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> split kwrootareaprovider in 3 classes for better overview and solving text flwing between textboxes
> 
> 
> Diffs
> -----
> 
>   words/part/CMakeLists.txt 386431e 
>   words/part/KWRootAreaProvider.h 54deaf3 
>   words/part/KWRootAreaProvider.cpp d26f519 
>   words/part/KWRootAreaProviderBase.h PRE-CREATION 
>   words/part/KWRootAreaProviderBase.cpp PRE-CREATION 
>   words/part/KWRootAreaProviderTextBox.h PRE-CREATION 
>   words/part/KWRootAreaProviderTextBox.cpp PRE-CREATION 
>   words/part/frames/KWFrame.cpp f88f902 
>   words/part/frames/KWTextFrameSet.h 9106758 
>   words/part/frames/KWTextFrameSet.cpp c8ccdb9 
> 
> Diff: https://git.reviewboard.kde.org/r/122266/diff/
> 
> 
> Testing
> -------
> 
> ran cstester and did some manual testing, but should eally do it again
> 
> 
> Thanks,
> 
> Camilla Boemann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20150127/46c10693/attachment.htm>


More information about the calligra-devel mailing list