----------------------------------------------------------- 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 > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
![http://wstaw.org/m/2014/07/10/plasma-desktoppn2275.png"](http://wstaw.org/m/2014/07/10/plasma-desktoppn2275.png"){kind=link}
![http://i.imgur.com/0c7S4Y3.png"](http://i.imgur.com/0c7S4Y3.png"){kind=link}
