Review Request 119527: Added feature of creating report's items of specific size at adding time
Wojciech Kosowicz
pcellix at gmail.com
Sun Aug 3 15:52:16 BST 2014
> On July 29, 2014, 4: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
>
> Jarosław Staniek wrote:
> 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!
Thanks!! I agree size is enough to return from minimum size method. What is now left is whether init method arguments should be modified or should two more member variables and getter be introduced :)
- Wojciech
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119527/#review63448
-----------------------------------------------------------
On July 28, 2014, 10:24 p.m., Wojciech Kosowicz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119527/
> -----------------------------------------------------------
>
> (Updated July 28, 2014, 10:24 p.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/20140803/89a529b5/attachment.htm>
More information about the calligra-devel
mailing list