Review Request 119222: Make labels inline-editable

Thu Nov 27 13:22:02 GMT 2014

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

Much better, close to final. Thanks! The note in KoReportDesignerItemLabel.cpp is a most important TODO.

Updated statuses, checked all DEFECTs:

"DEFECT1 after showing the label is not resized to the size of parent - see http://wstaw.org/m/2014/07/10/plasma-desktoppn2275.png"

Fixed!

"DEFECT2 dashed frame is not needed then (see the same screenshot)"

Fixed!

"DEFECT3 the font is not copied"

Fixed!

"DEFECT4 clicking on any area of the label during editing does not change cursor, maybe z-order isn't proper?"

NOW fixed!

"DEFECT5 would be nice if F2 shortcut and double click also entered into the edit mode"

Fixed!

"DEFECT6 when I use the property editor to enter the caption, after clicking outside the caption text is reset to the original value; now the only way to enter the text is to use the inline editor"

Fixed!

"DEFECT7 inserting a new label (or copy-pasting) crashes, exactly 6 times result of dynamic_cast<KoReportItemBase*> does not seem to be compared with 0"

Fixed!

"DEFECT8 when double clicking it's supereasy to accidentally move the item by a few pixels; on displays with even more density it will be even more noticeable"

Fixed!

"DEFECT9 the edited text alignment (horizontal/vertical) should match alignment of the original label"

Not fixed but it's not a showstopper. We can leave it as a junior-job.
Hints how to resolve:
http://www.qtcentre.org/threads/24814-QGraphicsTextItem-Vertical-text-alignment
http://www.cesarbs.org/blog/2011/05/30/aligning-text-in-qgraphicstextitem/

"DEFECT10 bg/fg color not copied to the edited text"

NOW fixed! Nice!

"DEFECT11 clicking on any area of the label during editing still does not change cursor's position; instead just ends the editing mode"

NOW fixed!

"DEFECT12 when in editing mode, if I switch to other report and switch back, the BoundedTextItem is apparently still present but the cursor is invisible; it's also impossible to enter text; the only solution is to click BoundedTextItem again; could we set focus on BoundedTextItem when needed?"

Not fixed. We can leave it as a junior-job if you don't to now.
Hint: on focus out for the entire report (probably ReportSceneView) we would like to accept editing by calling exitInlineEditingMode() method for selected item; this method is explained elsewhere in this review.

"DEFECT13 escape key shall exit from the edit mode"

Fixed

"DEFECT14: when the BoundedTextItem gets displayed and focused, parent section's property set is activated in the property editor, this shouldn't be the case; property set shouldn't change at all."

Fixed!

"DEFECT15: clicking outside of the BoundedTextItem instance does not hide it; see http://i.imgur.com/0c7S4Y3.png; the only way to hide the BoundedTextItem instance is to click the LabelEditButton; that shouldn't be the solution. Even if I click click the LabelEditButton"

Fixed!

"DEFECT16 I would prefer not to display the LabelEditButton. For quite typical sizes it looks as follows: http://i.imgur.com/XRAOq1d.png. This makes it hard to click and drag the element. Instead of using the LabelEditButton, I propose to do the same as we do with Kexi form labels and with shapes in the word processor: initiate editing on double click and F2 key (see DEFECT5)."

Fixed!

"DEFECT17 Resize handles are covered by the BoundedTextItem; see http://i.imgur.com/0c7S4Y3.png"

Not fixed. We can leave it as a junior-job.

"DEFECT18 Inline editing mode should be activated automatically when the label is interactively inserted (using the Insert)"

Not fixed. MOST IMPORTANT TODO. Shall be easy. But we can leave it as a junior-job.

"DEFECT19 Exiting the inline editing mode (actually only possible by clicking the LabelEditButton again) should keep the label element selected. Currently the parent section gets selected (visible in property editor)."

Fixed!

"DEFECT21: Property editor's Caption property is not updated in real time while I am using inline editing to enter text."

Not Fixed. (At least Escape accepts it and Property Editor updates). We can leave it as a junior-job.

libs/koreport/items/label/BoundedTextItem.cpp
<https://git.reviewboard.kde.org/r/119222/#comment49592>

    This works but by convention it's enough/safer to use stack:

    QStyleOptionGraphicsItem opt(*o);

libs/koreport/items/label/KoReportDesignerItemLabel.cpp
<https://git.reviewboard.kde.org/r/119222/#comment49598>

    Now that we changed behaviour of ReportScene::mousePressEvent(QGraphicsSceneMouseEvent*), this connection needs replacement or be removed.

    1. I propose call slotSwitchToViewMode() from KoReportDesignerItemRectBase::mousePressEvent() because it's the real place when selection changes.

    But for this, slotSwitchToViewMode() would have to be a virtual method of KoReportDesignerItemRectBase 
    and KoReportDesignerItemLabel should implement it. This also helps to implement inline editing for other report elements.

    2. If so, I also propose to rename slotSwitchToViewMode() to exitInlineEditingMode().

    3. For symmetry slotSwitchToEditMode() shall be renamed to enterInlineEditingMode().

libs/koreport/wrtembed/reportscene.cpp
<https://git.reviewboard.kde.org/r/119222/#comment49597>

    Better, now we're catching clicks on the section's surface. But what if we clicked on another item, ie. itemAt(e->scenePos()) != 0? 

    Then the previous editing isn't cancelled:
    http://i.imgur.com/ZdyVaV1.png

    See my hint related to slotSwitchToViewMode(), further fix shall be performed there.

- Jarosław Staniek

On Nov. 19, 2014, 11: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. 19, 2014, 11: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/reportscene.cpp 2e9c54d29c135c041db75d2a843ea5bd8318e1c1 
>   libs/koreport/items/check/KoReportDesignerItemCheck.cpp 5762247fc783d77ccd141d46c601e669b942e4b8 
>   libs/koreport/items/field/KoReportDesignerItemField.cpp 54baaa9f84879e7bba1db1b7a8da2cec7722c919 
>   libs/koreport/items/image/KoReportDesignerItemImage.cpp 7857466c4d7aa82a4be97d546cc77dc5333bd3c3 
>   libs/koreport/CMakeLists.txt d8aea281ad0536e1423210014f09f616ee09f9af 
> 
> 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/20141127/9882bfd4/attachment.htm>