[Marble-devel] Review Request 124781: gsoc: integrates the OsmTagEditorWidget and OsmRelationManagerWidget to the annotate editors

[Dennis Nienhüser]

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



src/lib/marble/EditPlacemarkDialog.h (line 156)
<https://git.reviewboard.kde.org/r/124781/#comment58305>

    Could this be a const reference also?



src/lib/marble/EditPlacemarkDialog.cpp (line 72)
<https://git.reviewboard.kde.org/r/124781/#comment58308>

    I think both m_osmTagEditorWidget and m_osmRelationManagerWidget need to be initialized to nullptr in the ctor of EditPlacemarkDialog::Private or EditPlacemarkDialog since EditPlacemarkDialog creates them conditionally only.



src/lib/marble/EditPlacemarkDialog.cpp (line 91)
<https://git.reviewboard.kde.org/r/124781/#comment58306>

    also delete m_osmRelationManagerWidget here?
    
    Technically possibly no difference since it has EditPlacemarkDialog as parent, but would be more consistent.



src/lib/marble/EditPlacemarkDialog.cpp (line 125)
<https://git.reviewboard.kde.org/r/124781/#comment58307>

    I wonder if we ever discussed whether a "Placemark" is a term we assume users understand. Otherwise we might want to go for "Untitled Place" instead.
    
    @Torsten: Any opinion on this?



src/plugins/render/annotate/AnnotatePlugin.h (line 134)
<https://git.reviewboard.kde.org/r/124781/#comment58309>

    const OsmPlacemarkData &



src/plugins/render/annotate/EditPolygonDialog.h (line 52)
<https://git.reviewboard.kde.org/r/124781/#comment58310>

    const OsmPlacemarkData &



src/plugins/render/annotate/EditPolygonDialog.cpp (line 54)
<https://git.reviewboard.kde.org/r/124781/#comment58311>

    please initialize to nullptr also, same for m_osmRelationManagerWidget



src/plugins/render/annotate/EditPolylineDialog.h (line 46)
<https://git.reviewboard.kde.org/r/124781/#comment58312>

    const OsmPlacemarkData &



src/plugins/render/annotate/EditPolylineDialog.cpp (line 43)
<https://git.reviewboard.kde.org/r/124781/#comment58313>

    please initialize to nullptr, same for m_osmRelationManagerWidget


- Dennis Nienhüser


On Aug. 19, 2015, 5:07 p.m., Marius Stanciu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124781/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2015, 5:07 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> !! This file has the CMakeList diff for all the previous patches that weren't added to the CMake
> 
> 
> ===TagEditor===
> Tags can now be edited for text, polyline and polygon annotations.
> The style changing occurs in the following manner:
> 
> Text annotations:
> 
> If the icon link is empty, an icon is chosen from the current tags ( if available )
> If the icon link is not empty, the custom icon is used.
> 
> Polylines and polygons:
> 
> If the default style is initialized( #polygon and #polyline ) and there is a tag-based style available, the tag-based style is used.
> If the style is not default, then the custom one is used.
> 
> ===RelationEditor===
> The annotate plugin keeps a hash of all the relations loaded( from parsing, newly created relations )
> Relations from this list are suggested in the Relations tab for all placemarks.
> 
> Normal use case:
> 1) Create a placemark
> 2) Add a new relation in the Relations tab
> 3) Create a new placemark
> 4) Add the relation created eariler for this placemark as well
> 
> Done! A relation with two members has been created
> 
> 
> This might not be the final behaviour. Please do suggest a better rendering behaviour!
> 
> 
> !TODO: gotta modify the parser and the writer to deal with these relations ( they can't be imported/exported atm ). Will do this in the next patch.
> 
> 
> Diffs
> -----
> 
>   src/lib/marble/EditPlacemarkDialog.h 1ff59c2 
>   src/lib/marble/EditPlacemarkDialog.cpp ef295bb 
>   src/lib/marble/FileViewWidget.cpp b1c77c7 
>   src/lib/marble/TourItemDelegate.cpp 834f1a7 
>   src/lib/marble/osm/CMakeLists.txt 2aa67f7 
>   src/plugins/render/annotate/AnnotatePlugin.h dd29ee6 
>   src/plugins/render/annotate/AnnotatePlugin.cpp e98043b 
>   src/plugins/render/annotate/EditPolygonDialog.h 6c58caa 
>   src/plugins/render/annotate/EditPolygonDialog.cpp 28815a5 
>   src/plugins/render/annotate/EditPolylineDialog.h ae9177f 
>   src/plugins/render/annotate/EditPolylineDialog.cpp bacc6df 
> 
> Diff: https://git.reviewboard.kde.org/r/124781/diff/
> 
> 
> Testing
> -------
> 
> Created placemarks, polylines, polygons, added tags, removed tags, changed styles... works as expected.
> Created placemarks, created relations, added placemarks to relations, removed relations, edited relations.... works as expected.
> 
> 
> Thanks,
> 
> Marius Stanciu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20150822/9ee95020/attachment-0001.html>