Review Request 119222: Make labels inline-editable

[Jarosław Staniek]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20141129/6b367b01/attachment.htm>