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


I don't like the redundancy this patch introduces by maintaining a second route 
request model. This would become a nightmare to maintain. Instead there should 
only be one model -- the C++ based RouteRequestModel. All code on top needs to 
work on the very same data e.g. like declarative/RouteRequestModel does. The 
problem you describe sounds like being related to the redundancy already, and 
another place where things are diverting already is on startup when I previous 
route is loaded: RouteRequestModel has all the data and the route is drawn 
fine, but the placemarks do not show up because the second model has not been 
populated. Let's avoid these issues in the first place.
Using a delegate similar to https://git.reviewboard.kde.org/r/124550/ should 
work much nicer.


src/apps/marble-maps/Search.qml (line 13)
<https://git.reviewboard.kde.org/r/124627/#comment57706>

    I'd rather call the signal itemSelected. It's up to the connected slot to 
determine what to do with it.


- Dennis Nienhüser


On Aug. 6, 2015, 7:36 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. 6, 2015, 7:36 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_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 b8dc7b0 
>   src/apps/marble-maps/MarbleMaps.qrc 0168893 
>   src/apps/marble-maps/NavigationSetup.qml PRE-CREATION 
>   src/apps/marble-maps/ProfileSelectorMenu.qml PRE-CREATION 
>   src/apps/marble-maps/RoutingManager.qml PRE-CREATION 
>   src/apps/marble-maps/Search.qml 9b712c8 
>   src/apps/marble-maps/Waypoint.qml PRE-CREATION 
>   src/apps/marble-maps/WaypointOrderManager.qml PRE-CREATION 
>   src/lib/marble/declarative/MarbleQuickItem.h cefb0c0 
>   src/lib/marble/declarative/MarbleQuickItem.cpp f8c396f 
>   src/lib/marble/declarative/Placemark.h 8c17b4f 
>   src/lib/marble/declarative/Placemark.cpp 27a9579 
>   src/lib/marble/declarative/Routing.cpp dbbcb80 
>   src/lib/marble/declarative/SearchBackend.h 4981c31 
>   src/lib/marble/declarative/SearchBackend.cpp fdef702 
> 
> 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
> 
> 
> Thanks,
> 
> Gábor Péterffy
> 
>

_______________________________________________
Marble-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/marble-devel

Reply via email to