[Marble-devel] Review Request: Current Location interface simplification and extension

[Torsten Rahn]

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



/trunk/KDE/kdeedu/marble/src/lib/MarbleControlBox.h
<http://reviewboard.kde.org/r/3549/#comment4401>

    centerOnCurrentLocation()



/trunk/KDE/kdeedu/marble/src/lib/MarbleControlBox.cpp
<http://reviewboard.kde.org/r/3549/#comment4402>

    "in" is a bit non-descriptive IMHO.



/trunk/KDE/kdeedu/marble/src/lib/MarbleControlBox.ui
<http://reviewboard.kde.org/r/3549/#comment4400>

    "projectionComboBox"? Is this maybe the "positionTrackingComboBox" ? Or is this just due to moved source code that suddenly becomes visible in the patch?



/trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdPositionProviderPlugin.cpp
<http://reviewboard.kde.org/r/3549/#comment4403>

    I think that there is a rule that GUI entries (e.g. in menus etc.) should always start capitalized. Also "gpsd" looks a bit geeky :-/ "GPS Daemon ("gpsd") maybe?



/trunk/KDE/kdeedu/marble/src/plugins/positionprovider/maemo/MaemoPositionProviderPlugin.cpp
<http://reviewboard.kde.org/r/3549/#comment4404>

    "Maemo" is also a bit short imho, hm.. . 


- Torsten


On 2010-04-10 16:05:16, Dennis Nienhüser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3549/
> -----------------------------------------------------------
> 
> (Updated 2010-04-10 16:05:16)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> In short: 1) Possibility to choose GPS source in the UI 2) Condense the UI 3) Allow jumping to the current position
> 
> The patch changes the "Current Location" widget in the control box to preserve some whitespace (important on Maemo). It also removes the custom fonts and the wrong tooltips claiming that the map would be re-centered automatically. The unused label for the fuzzy location description goes away as well, although I plan to add it back at some point with reverse geocoding information.
> 
> The "navigation mode" checkbox gets replaced with a combobox that let's you disable position tracking (default at startup) or select one of the available plugins. I favor this for several reasons: You get control over GPS and see what is installed. In case more than one plugin is available, you can switch between them. You can disable GPS, which is most important on embedded devices (GPS drains the battery).
> 
> The longitude and latitude value in the label are links, activating any of them centers the map on the current position.
> 
> 
> This addresses bug 202134.
>     https://bugs.kde.org/show_bug.cgi?id=202134
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleControlBox.h 1113282 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleControlBox.cpp 1113282 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleControlBox.ui 1113282 
>   /trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp 1113282 
>   /trunk/KDE/kdeedu/marble/src/lib/PluginManager.cpp 1113282 
>   /trunk/KDE/kdeedu/marble/src/lib/gps/GpsLayer.h 1113282 
>   /trunk/KDE/kdeedu/marble/src/lib/gps/GpsLayer.cpp 1113282 
>   /trunk/KDE/kdeedu/marble/src/lib/gps/PositionTracking.h 1113282 
>   /trunk/KDE/kdeedu/marble/src/lib/gps/PositionTracking.cpp 1113282 
>   /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/geoclue/GeoCluePositionProviderPlugin.cpp 1113282 
>   /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdPositionProviderPlugin.cpp 1113282 
>   /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/maemo/MaemoPositionProviderPlugin.cpp 1113282 
> 
> Diff: http://reviewboard.kde.org/r/3549/diff
> 
> 
> Testing
> -------
> 
> Using gpsfake on my desktop system and the internal GPS of my N900.
> 
> 
> Screenshots
> -----------
> 
> marble-current-location-ui.png
>   http://reviewboard.kde.org/r/3549/s/353/
> 
> 
> Thanks,
> 
> Dennis
> 
>