Review Request: adds a VectorShapeConfigWidget which gets shown on shape creation (like with PictureShape)

Inge Wallin inge at lysator.liu.se
Fri Jul 20 11:31:17 BST 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105624/#review16148
-----------------------------------------------------------

Ship it!


Looks really good!  You can commit after you fixed the small issues that I point out below.


plugins/vectorshape/VectorShapeFactory.cpp
<http://git.reviewboard.kde.org/r/105624/#comment12729>

    ...and SVM (Starview metafile).
    
    SVG should be really simple to add, considering the SVG renderer in Qt.



plugins/vectorshape/VectorShapeFactory.cpp
<http://git.reviewboard.kde.org/r/105624/#comment12730>

    Normally I use FIXME instead of TODO.  Probable best not to mix them since it's easier to miss them then.


- Inge Wallin


On July 20, 2012, 2:59 a.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105624/
> -----------------------------------------------------------
> 
> (Updated July 20, 2012, 2:59 a.m.)
> 
> 
> Review request for Calligra and Inge Wallin.
> 
> 
> Description
> -------
> 
> Other than with the picture shape, after adding a new vector shape one first has to activate the vector tool, then click the open button.
> Not very user-friendly and also inconsistent -> isn't that a bug from your, Inge's, POV? :) (\me thinks of zooming behaviour in Stage).
> 
> Attached patch fixes this by adding a configwidget also to the vector shape, copied from the picture shape and adapted.
> The patch also fixes the icon for the shape and the tool to an existing one.
> And adds "(EMF/WMF)" to the tooltip of the shape, to make it more obvious what this shape is about (especially in Karbon).
> 
> Q1: Is that vector shape just for EMF, WMF officially? What about SVG (not only as in SVM from OOo)?
> 
> Q2: Seems there is no official mimetype for WMF and EMF. Wikipedia say image/x-wmf and image/x-emf, and so does the XDG database (that's why these are used for the filedialog, while Oxygen icons use application-x-wmf, like the code in VectorShape.cpp with application/x-wmf and application/x-emf. Is the code correct here?
> 
> The updated patch turned to use image/x-wmf and image/x-emf for the manifest file mimetypes, at least LO seems to not care.
> It also sets the flag estimateByContent for context.odfLoadingContext().mimeTypeForPath(...), like recently added for the SVG loading, because at least LO 3.5.3 does not write any mimetype for WMF as well.
> And makes the code avoid some unneeded compression roundtrips, by
> * remembering the type in the ChangeVectorDataCommand
> * for rendering using a separate uncompressed copy of the content
> 
> Okay to backport to 2.5, without the tooltip string change?
> 
> 
> Diffs
> -----
> 
>   plugins/vectorshape/CMakeLists.txt da25dbb 
>   plugins/vectorshape/ChangeVectorDataCommand.h 349e630 
>   plugins/vectorshape/ChangeVectorDataCommand.cpp b9424eb 
>   plugins/vectorshape/VectorShape.h dc98d80 
>   plugins/vectorshape/VectorShape.cpp 44bfd1a 
>   plugins/vectorshape/VectorShapeConfigWidget.h PRE-CREATION 
>   plugins/vectorshape/VectorShapeConfigWidget.cpp PRE-CREATION 
>   plugins/vectorshape/VectorShapeFactory.cpp 07e02ae 
>   plugins/vectorshape/VectorTool.cpp f2a131f 
>   plugins/vectorshape/VectorToolFactory.cpp 04212c6 
> 
> Diff: http://git.reviewboard.kde.org/r/105624/diff/
> 
> 
> Testing
> -------
> 
> Edited ODT files with WMF files (adding, removing, changing) and did roundtrips with LO 3.5.3
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120720/267ff371/attachment.htm>


More information about the calligra-devel mailing list