[Marble-devel] Review Request 124627: Markers on map in Marble Maps

Wed Aug 12 19:25:25 UTC 2015

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


src/apps/marble-maps/ProfileSelectorMenu.qml (line 34)
<https://git.reviewboard.kde.org/r/124627/#comment57956>

    the names aren't really usable as identifiers. At least put qsTr() around them since they will be translated.


src/apps/marble-maps/RoutingManager.qml (line 54)
<https://git.reviewboard.kde.org/r/124627/#comment57957>

    + qsTr() each


src/lib/marble/declarative/Routing.cpp (line 122)
<https://git.reviewboard.kde.org/r/124627/#comment57958>

    isn't keys() enough? See http://doc.qt.io/qt-5/qmap.html#uniqueKeys


src/lib/marble/declarative/Routing.cpp (line 123)
<https://git.reviewboard.kde.org/r/124627/#comment57959>

    shouldn't this be a for loop? The amount of waypoints might change by more than one in between updates.


src/lib/marble/declarative/Routing.cpp (line 136)
<https://git.reviewboard.kde.org/r/124627/#comment57960>

    same here, shouldn't this be a for loop? The amount of waypoints might change by more than one in between updates.


src/lib/marble/declarative/Routing.cpp (line 144)
<https://git.reviewboard.kde.org/r/124627/#comment57961>

    This seems wrong (in general, see comments above.)


- Dennis Nienhüser


On Aug. 8, 2015, 9:38 a.m., Gábor Péterffy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124627/
> -----------------------------------------------------------
> 
> (Updated Aug. 8, 2015, 9:38 a.m.)
> 
> 
> Review request for Marble.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> This patch adds the following:
> 
> - Redesigned routing UI
> - Markers on the map
> - Waypoint order editing
> 
> 
> Diffs
> -----
> 
>   data/android/drawable-xxxhdpi/delete.png PRE-CREATION 
>   data/android/drawable-xxxhdpi/down.png PRE-CREATION 
>   data/android/drawable-xxxhdpi/place_green.png PRE-CREATION 
>   data/android/drawable-xxxhdpi/place_orange.png PRE-CREATION 
>   data/android/drawable-xxxhdpi/place_red.png PRE-CREATION 
>   data/android/drawable-xxxhdpi/place_white.png PRE-CREATION 
>   data/android/drawable-xxxhdpi/up.png PRE-CREATION 
>   src/apps/marble-maps/FloatingMenuButton.qml PRE-CREATION 
>   src/apps/marble-maps/MainScreen.qml fafd183 
>   src/apps/marble-maps/MarbleMaps.qrc 713e414 
>   src/apps/marble-maps/NavigationSetup.qml eb7c1dd 
>   src/apps/marble-maps/ProfileSelectorMenu.qml PRE-CREATION 
>   src/apps/marble-maps/RoutingManager.qml PRE-CREATION 
>   src/apps/marble-maps/Search.qml 14088b8 
>   src/apps/marble-maps/WaypointImage.qml PRE-CREATION 
>   src/apps/marble-maps/WaypointOrderManager.qml PRE-CREATION 
>   src/lib/marble/declarative/MarbleQuickItem.h 4b1f729 
>   src/lib/marble/declarative/MarbleQuickItem.cpp feecb00 
>   src/lib/marble/declarative/Placemark.h 8c17b4f 
>   src/lib/marble/declarative/Placemark.cpp 27a9579 
>   src/lib/marble/declarative/RouteRequestModel.cpp c67eb51 
>   src/lib/marble/declarative/Routing.h 51a1be7 
>   src/lib/marble/declarative/Routing.cpp 7a0d8aa 
>   src/lib/marble/declarative/SearchBackend.h 86924f2 
>   src/lib/marble/declarative/SearchBackend.cpp b215d7c 
>   src/lib/marble/routing/RouteRequest.h 60b768b 
>   src/lib/marble/routing/RouteRequest.cpp 441972b 
> 
> Diff: https://git.reviewboard.kde.org/r/124627/diff/
> 
> 
> Testing
> -------
> 
> It works fine except one thing: when you call the redrawRoute() it draws the route until the first placemark, but if you changes the routing profile it draws the full road. Routing instructions are also missing in this case.
> 
> 
> File Attachments
> ----------------
> 
> place_white.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/c34d1f1c-cdb8-4f4b-9c0f-7c9fc479c5a5__place_white.png
> up.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/0dd988f0-189e-4ce6-8dd8-5f4e8f07713a__up.png
> down.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/f0557e37-7c1b-4447-93d2-201c8b05efe4__down.png
> delete.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/fb26b212-cfc0-40d4-90ac-29bbe514dec4__delete.png
> Screenshot I
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/85a13541-3168-499b-9c6a-776f889fe5d7__Screenshot_2015-08-06-01-44-58.png
> Screenshot II
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/2dca53bf-7941-4bd0-8ec2-1e517e915ae8__Screenshot_2015-08-06-01-43-41.png
> Screenshot III
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/7b24796b-a13c-4508-87c7-9861f779a15c__Screenshot_2015-08-06-01-44-51.png
> place_orange.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/07/323cdff6-7ee6-4d05-a8f1-15e92c6365b6__place_orange.png
> place_green.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/07/7221416f-7b47-45cf-b5df-ac435b1a3b04__place_green.png
> place_red.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/07/8decaaeb-8819-400e-8589-e37e9fa7e98d__place_red.png
> 
> 
> Thanks,
> 
> Gábor Péterffy
> 
>

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