[Marble-devel] Review Request: Interface for Small Screen Devices

Dennis Nienhüser earthwings at gentoo.org
Mon Aug 16 21:40:27 CEST 2010 

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



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7224>

    Here is a memory leak. Better store the grid layout as a class member and clear it instead of creating a new one each time.



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7225>

    To be safe you should check whether showRoutingItem is called repeatedly with the same value. Currently this would result in multiple signal/slot connections, which is bad.



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7231>

    This method looks quite long and a bit repetitive. Leave it like this for now, but maybe add a TODO to split it up a bit?
    



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7226>

    preferably using the HOUR2MIN and other constants



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7229>

    hr (no capital)



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7228>

    min (no capital)



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7227>

    %n minutes



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7230>

    %n hours
    
    %n minutes



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7232>

    Mark the instruction bold instead? That's what the user is watching out for.



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7233>

    I think an int is fine for meters, if not too detailed already. A double with high precision is not needed.



/trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp
<http://reviewboard.kde.org/r/5011/#comment7234>

    Precision of 1 should be enough, I think.


- Dennis


On 2010-08-15 19:05:04, Siddharth Srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/5011/
> -----------------------------------------------------------
> 
> (Updated 2010-08-15 19:05:04)
> 
> 
> Review request for marble and Dennis Nienhüser.
> 
> 
> Summary
> -------
> 
> An interface of Routing Plugin for Small Screen Devices is added. The Routing Plugin for Small Screen Device contains options for navigation( recenter, auto zoom), routing information widget, zoom in and zoom out functionalities.
> The routing information widget for desktop version of marble, now shows the next instruction as well as the distance remaining to reach it.
> 
> The instruction point now grows in two levels when the gps location gets closer to it instead of displaying the instruction text at it. 1) The instruction point going to be visited grows in size 2) When the gps device is at certain time away from the instruction point, it grows a bit further.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/src/lib/CurrentLocationWidget.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/CurrentLocationWidget.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/CurrentLocationWidget.ui 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/DownloadRegionDialog.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/DownloadRegionDialog.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleControlBox.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleModel.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/gps/PositionTracking.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/gps/PositionTracking.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/graphicsview/ScreenGraphicsItem_p.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/graphicsview/WidgetGraphicsItem.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/AdjustNavigation.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/AdjustNavigation.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingLayer.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingManager.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingManager.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingModel.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingModel.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/marble.qrc 1164052 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/navigation/NavigationFloatItem.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/navigation/NavigationFloatItem.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/progress/ProgressFloatItem.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/routing/CMakeLists.txt 1164052 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingInformationWidget.ui PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingInformationWidgetSmall.ui PRE-CREATION 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingItemWidget.ui 1164052 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.h 1164052 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingPlugin.cpp 1164052 
>   /trunk/KDE/kdeedu/marble/src/plugins/render/routing/RoutingWidgetSmall.ui PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/5011/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> Routing Information Widget for Desktop version of marble
>   http://reviewboard.kde.org/r/5011/s/478/
> Routing Widget for small screen devices. This screen shot shows the routing information widget which appears on clicking "R" on the routing widget
>   http://reviewboard.kde.org/r/5011/s/479/
> navigation menu for routing widget for small screen devices
>   http://reviewboard.kde.org/r/5011/s/480/
> [New]RoutingInformation for Desktop Marble
>   http://reviewboard.kde.org/r/5011/s/483/
> [new] Routing information for Small Screen Devices
>   http://reviewboard.kde.org/r/5011/s/484/
> 
> 
> Thanks,
> 
> Siddharth
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20100816/c3965f49/attachment-0001.htm

More information about the Marble-devel mailing list