Add d-ptr to kReportItemBase and port KReportItemLine
ClosedPublic

Authored by piggz on Nov 6 2016, 10:15 PM.

Details

Summary

Add a dptr and required accessors to KReportItemBase
and port KReportItemLine

Implement review comments

Removed need for KReportPosition and KReportSize classes

Implement more comments

Further porting of items

Further porting, Label designer item ported.

Some more structural changes, mainly around KReportDeisignerItemBase
gaining a d-ptr, and a pointer to the KReportItemBase so there is no
need to pass in individual pointers to the propertyset, size and
position.

Port remaining items to new api

Test Plan

Tested by opening/saving POI reports. rendering is as expected. compared XML before and after and size and position are the same

Diff Detail

Repository
R14 KReport
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
piggz updated this revision to Diff 7948.Nov 6 2016, 10:15 PM
piggz retitled this revision from to Add d-ptr to kReportItemBase and port KReportItemLine.
piggz updated this object.
piggz edited the test plan for this revision. (Show Details)
piggz added a reviewer: staniek.
piggz added a project: KReport.
piggz added a comment.Nov 6 2016, 10:17 PM

Adds d-ptr to
KReportItemBase
KReportDesigneriIemBase
KReportDesignerRectItemBase

Removed use of KReportSize and KReportPosition

Ported all items to new API

piggz added a comment.Nov 6 2016, 10:20 PM

Needs extensive testing....

So far ive tested that it builds, and that adding items works.

Items are added ok, and react correctly to size/position property changes. (works best with kproperty unit-option patch)
Havnt tested loading/saving of existing documents.

staniek requested changes to this revision.Nov 6 2016, 10:54 PM
staniek edited edge metadata.

Good job!

examples/window.cpp
128 ↗(On Diff #7948)

OK but later maybe add #ifdef or something?

src/common/KReportItemBase.cpp
85–97

do we need underlines here?

200

When we're at this, how about changing d->zIndex to d->z for consistency?

213

size

233

missing Q_UNUSED

src/common/KReportItemLine.cpp
26

Will move m_start etc here?

src/common/KReportItemLine.h
62

Move to the end (convention)

src/common/KReportUtils.h
93

const QPointF &pos, const QSizeF &size?

or

const QRect &rect?

src/items/check/KReportDesignerItemCheckBox.cpp
95–97

In such cases I am using

QSizeF sceneSize = this->sceneSize(size());

and it works portable

src/wrtembed/KReportDesignerItemRectBase.cpp
58

what about this?

This revision now requires changes to proceed.Nov 6 2016, 10:54 PM
piggz added inline comments.Nov 7 2016, 9:09 AM
src/common/KReportItemBase.cpp
85–97

Probably not, it's just me ensuring there are no clashes with function names etc. Will change.

233

I know about these , my plan was a separate review concentrating on build warnings. Ok?

src/wrtembed/KReportDesignerItemRectBase.cpp
58

Will remove , no longer required

staniek added inline comments.Nov 7 2016, 11:03 AM
src/common/KReportItemBase.cpp
233

OK!

piggz marked 11 inline comments as done.Nov 7 2016, 11:01 PM
piggz added inline comments.
src/common/KReportItemLine.cpp
26

yes, i'll do this

piggz updated this revision to Diff 7998.Nov 7 2016, 11:03 PM
piggz edited edge metadata.
  • Implement review comments
piggz updated this revision to Diff 8033.Nov 8 2016, 8:37 PM
piggz edited edge metadata.
  • Fix map rendering
piggz edited the test plan for this revision. (Show Details)Nov 8 2016, 8:40 PM
staniek requested changes to this revision.Nov 8 2016, 11:04 PM
staniek edited edge metadata.

Good, the code reads much nicer!

Minor stuff left.

examples/window.cpp
35 ↗(On Diff #7998)

Can we move this to KReportUtils_p.h?

128 ↗(On Diff #8033)

at least let's use kreportDebug() everywhere

src/wrtembed/KReportDesignerItemBase.h
81–82

I don't see where are these two members used

src/wrtembed/KReportDesignerItemRectBase.cpp
106

8X

109

8X

src/wrtembed/KReportDesignerItemRectBase.h
51–52

8X

81–82

8X ?

This revision now requires changes to proceed.Nov 8 2016, 11:04 PM
piggz marked 10 inline comments as done.Nov 9 2016, 7:29 PM
piggz added inline comments.
examples/window.cpp
35 ↗(On Diff #7998)

Just deleted it as it was a temp debugging aid

128 ↗(On Diff #8033)

deleted

src/common/KReportItemLine.cpp
26

How about intead making this a non exported class .... i think this is exported by accident as no other items are.

src/wrtembed/KReportDesignerItemBase.h
81–82

removed these and related unimplemented functions!

piggz updated this revision to Diff 8051.Nov 9 2016, 7:31 PM
piggz edited edge metadata.
piggz marked 4 inline comments as done.
  • Implement review comments and...
piggz updated this revision to Diff 8052.Nov 9 2016, 7:53 PM
piggz edited edge metadata.
  • Private not required here
staniek accepted this revision.Nov 9 2016, 8:03 PM
staniek edited edge metadata.

Perfect ☺

src/common/KReportItemLine.h
34

Good catch.

This revision is now accepted and ready to land.Nov 9 2016, 8:03 PM
This revision was automatically updated to reflect the committed changes.