----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119222/#review71086 -----------------------------------------------------------
Almost there! Mostly notes on certain techniques. libs/koreport/items/label/KoReportDesignerItemLabel.h <https://git.reviewboard.kde.org/r/119222/#comment49653> 1. This is a QObject, let's use the coding standard from this lib: "public slots:" 2. API design guidelines: in public methods don't use slot prefixes. I proposed enterInlineEditingMode(), exitInlineEditingMode(). libs/koreport/wrtembed/reportscene.cpp <https://git.reviewboard.kde.org/r/119222/#comment49654> our C++ convention, practiced in koreport too: use (!itemUnderCursor) libs/koreport/wrtembed/reportscene.cpp <https://git.reviewboard.kde.org/r/119222/#comment49655> Use foreach(QGraphicsItem *it, items()); no need to declare list separately in line 142. BTW, are we sure we need to use items() and not selectedItems()? I guess selection has not changes yet at this point? libs/koreport/wrtembed/KoReportDesignerItemRectBase.h <https://git.reviewboard.kde.org/r/119222/#comment49656> We don't need this to be a slot, do we? It's perfectly allowed to change virtual method to virtual slot in the subclass (actually only in KoReportDesignerItemLabel so far). Only KoReportDesignerItemLabel uses the method as slot, and it can since it introduces slots; ReportScene performs direct call. This is called 'interfaces for slots', and is recommended, used in Kexi Forms quite a bit too. The methods could be even pure virtual, but that would not be convenient since we're only implementing them in one element so far. - Jarosław Staniek On Nov. 29, 2014, 7:04 p.m., Adam Pigg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119222/ > ----------------------------------------------------------- > > (Updated Nov. 29, 2014, 7:04 p.m.) > > > Review request for Calligra and Jarosław Staniek. > > > Bugs: 336825 > http://bugs.kde.org/show_bug.cgi?id=336825 > > > Repository: calligra > > > Description > ------- > > Usage: > Insert label > Double click to enter inline-edit mode > -Label text is selected and can be overwritten > Click away from item to exit edit mode > -Item text is updated with editor text > > > Diffs > ----- > > libs/koreport/items/label/BoundedTextItem.h PRE-CREATION > libs/koreport/items/label/BoundedTextItem.cpp PRE-CREATION > libs/koreport/items/label/KoReportDesignerItemLabel.h > 469de9d3626c58e55cc9ad4a2b4e7b61af7a7c7f > libs/koreport/items/label/KoReportDesignerItemLabel.cpp > af21e56dd6fbc22e80c72eae9c5b166369f3c904 > libs/koreport/items/text/KoReportDesignerItemText.cpp > da2d4f5e8c8ca40713eb9936438355f7a5ee6c59 > libs/koreport/renderer/KoReportPrintRenderer.cpp > 08428520a1e8f8d319069ade7dd4df96dfa2edb3 > libs/koreport/renderer/KoReportScreenRenderer.cpp > d19835b05a2f22c7db02f06af1fd0e149864dc2d > libs/koreport/renderer/odtframe/KoOdtFrameReportCheckBox.cpp > 95aa265f874a77841c7b76820ef7bbcdbe89b3e9 > libs/koreport/wrtembed/KoReportDesigner.cpp > 48a667032a3a8049b69792c16d3cfc1727636fe7 > libs/koreport/wrtembed/KoReportDesignerItemRectBase.h > 3fddb9bc54243be61665f9e4e2b9ddf3d7360e23 > libs/koreport/wrtembed/KoReportDesignerItemRectBase.cpp > ae86cf99899fa29fbf8e7e7abd39357afccb03e4 > libs/koreport/wrtembed/reportscene.cpp > 2e9c54d29c135c041db75d2a843ea5bd8318e1c1 > libs/koreport/wrtembed/reportsection.h > 4f445ba045d5e858d610230aa1b445427525f0f4 > libs/koreport/items/field/KoReportDesignerItemField.cpp > 54baaa9f84879e7bba1db1b7a8da2cec7722c919 > libs/koreport/items/image/KoReportDesignerItemImage.cpp > 7857466c4d7aa82a4be97d546cc77dc5333bd3c3 > kexi/plugins/reports/kexireportdesignview.cpp > 25b2eb10315a12ff9a9053b72ef507627da3fc48 > libs/koreport/CMakeLists.txt d8aea281ad0536e1423210014f09f616ee09f9af > libs/koreport/items/check/KoReportDesignerItemCheck.cpp > 5762247fc783d77ccd141d46c601e669b942e4b8 > > Diff: https://git.reviewboard.kde.org/r/119222/diff/ > > > Testing > ------- > > Inserted label on new report and checked usage > > Loaded existing report and ensured labels are editiable > > > Thanks, > > Adam Pigg > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
