Review Request 119527: Added feature of creating report's items of specific size at adding time

Sat Aug 2 22:48:00 BST 2014

> On July 29, 2014, 6:19 p.m., Jarosław Staniek wrote:
> > libs/koreport/items/check/KoReportDesignerItemCheck.cpp, line 53
> > <https://git.reviewboard.kde.org/r/119527/diff/1/?file=293998#file293998line53>
> >
> >     1. How about moving this code to KoReportDesignerItemRectBase ctor?
> >     
> >     And add virtual QSizeF KoReportDesignerItemRectBase::minimumSize() const = 0; which you'll implement in subclasses. There, sometimes you want to return getTextRect().size(), sometimes yoyu want to return QSizeF(5, 5), etc.
> >     
> >     2. m_userWidth/m_userHeight are not needed if you can just use r->countHeight() and r->countWidth(). That's typical overuse of member variables.
> >     
> >     3. Give the fixes I propsed below, m_user* members can be equal to -1. Check this and if either is -1, use default size.
> 
> Wojciech Kosowicz wrote:
>     This is the situaion that occurs: 
>     1. setRectScene must be called in constructor's init. Before all the values are set for 0
>     2. KoReportDesignerItemRectBase::minimumSize() I propose to return QRectF
>     3. To return QRectF I would need two more member variables     
>         m_userHeight = r->countHeight();
>         m_userWidth = r->countWidth();
>         m_pressX = r->m_pressX;
>         m_pressY = r->m_pressY;
>     theoretically by modyfying init(giving it specific pointer) method I could  use them without writing theirs values to member variables but it be would rather significant change in reports. and I would still need pressX, pressY members :)
>     4. Regarding second point I agree to initialize members to -1 but I think the way I proposed of checking for minimum size by comparison to minimum value is enought
>     
>     By the way when I learnt about design patterns :)about year and also thought of using template method here :) I didn't propose it as I wanted your opinion but I'm glad it came up :)
>     I would like to share your opinions regarding the points presented above as any change here goes with changes within about 20 files so I would like to have it more or less clear when starting modification:)
>     
>     Regards :)
>     Wojtek
> 
> Wojciech Kosowicz wrote:
>     The solution above would also require two new getters for m_pressX and m_pressY

setRectScene must be called in constructor's init. Before all the values are set for 0

Then it can be moved to KoReportDesignerItemRectBase::init(), right?

    KoReportDesignerItemRectBase::minimumSize() I propose to return QRectF

If the position of the rect is always equal to (r->m_pressX, r->m_pressY), then there is no point in returing rect, right? Size is enough.
Or do you expect some implementations will return some other position?

    To return QRectF I would need two more member variables
    m_userHeight = r->countHeight();
    m_userWidth = r->countWidth();
    m_pressX = r->m_pressX;
    m_pressY = r->m_pressY;

    theoretically by modyfying init(giving it specific pointer) method I could use them without writing theirs values to member variables but it be would rather significant change in reports. and I would still need pressX, pressY members :)

No opinion for now untill we decide if we return QSizeF() or not, see above.

    Regarding second point I agree to initialize members to -1 but I think the way I proposed of checking for minimum size by comparison to minimum value is enought

OK

    By the way when I learnt about design patterns :)about year and also thought of using template method here :) I didn't propose it as I wanted your opinion but I'm glad it came up :)
    I would like to share your opinions regarding the points presented above as any change here goes with changes within about 20 files so I would like to have it more or less clear when starting modification:)

OK!

- Jarosław

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

On July 29, 2014, 12:24 a.m., Wojciech Kosowicz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119527/
> -----------------------------------------------------------
> 
> (Updated July 29, 2014, 12:24 a.m.)
> 
> 
> Review request for Calligra, Adam Pigg and Jarosław Staniek.
> 
> 
> Bugs: 334967
>     http://bugs.kde.org/show_bug.cgi?id=334967
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> If the specified dimensions are improper or too small elements get their defaults size
> 
> 
> Diffs
> -----
> 
>   libs/koreport/items/check/KoReportDesignerItemCheck.cpp 5bebc6c 
>   libs/koreport/items/field/KoReportDesignerItemField.cpp 0dc9325 
>   libs/koreport/items/image/KoReportDesignerItemImage.cpp 87e46a1 
>   libs/koreport/items/label/KoReportDesignerItemLabel.cpp 8974a80 
>   libs/koreport/items/text/KoReportDesignerItemText.cpp 51e12d3 
>   libs/koreport/wrtembed/KoReportDesigner.h adc712a 
>   libs/koreport/wrtembed/KoReportDesigner.cpp 63152a7 
>   libs/koreport/wrtembed/KoReportDesignerItemLine.h 894cec9 
>   libs/koreport/wrtembed/KoReportDesignerItemLine.cpp 1b0474d 
>   libs/koreport/wrtembed/KoReportDesignerItemRectBase.h b53a58e 
>   libs/koreport/wrtembed/KoReportDesignerItemRectBase.cpp 53c3727 
>   libs/koreport/wrtembed/reportsceneview.h cc3ab9f 
>   libs/koreport/wrtembed/reportsceneview.cpp 2b679a4 
>   plugins/reporting/barcode/KoReportDesignerItemBarcode.cpp b4fb621 
>   plugins/reporting/maps/KoReportDesignerItemMaps.cpp f624db3 
>   plugins/reporting/web/KoReportDesignerItemWeb.cpp 63e1cc9 
> 
> Diff: https://git.reviewboard.kde.org/r/119527/diff/
> 
> 
> Testing
> -------
> 
> When adding any element in kexi report it will the same size as specified by the user at the moment of adding
> 
> 
> Thanks,
> 
> Wojciech Kosowicz
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20140802/3910649a/attachment.htm>