> On Jul 27, 2017, at 4:07 PM, Lubomir I. Ivanov <[email protected]> wrote: > > On 28 July 2017 at 01:54, Dirk Hohndel <[email protected]> wrote: >>> >>> The best thing about this map widget is that it can be used for both >>> the mobile and desktop versions. The mobile version support is not >>> implemented, as it needs some decision making. The desktop version >>> support begins with the class dekstop-widgets/mapwidget.cpp which is a >>> QQuickWidget that loads >>> a QML file mobile-widgets/qml/MapWidget.qml. All the NO_MARBLE >>> abstraction is taken care of and MapWidget methods are used instead of >>> Globe methods. The code in mapwidget.cpp code is minimal as this class >>> desktop-version specific. The magic happens in the class >>> mobile-widgets/qmlmapwidgetheloper.cppwhich is the backend of the map >> >> So why is this in mobile-widgets and not in core ? >> If we run this in the desktop version as well, it shouldn't be in >> mobile-widgets > > because Qt Location has mobile as it's main platform target. > also, this code works for both mobile and desktop.
We have other code like this under core, e.g. core/subsurface-qt or core/downloadfromdcthread.cpp One might argue that the code for the map maybe should get its own directory, like profile-widget Fundamentally, when building for desktop, nothing in mobile-widgets should be needed and vice versa for desktop-widgets when building Subsurface-mobile This isn't a show stopper, we can move things later. Just saying that this would be my preference (unless there is some QML weirdness that prevents that). > as some of the few early commits suggest we can move this to a new > folder - e.g. shared-widgets. > but i wonder how much my commit history will break with this > change...maybe a patch on top can do that in the cleanest fashion. Yes, that's fine. I'm just starting the review - let's see if there are things that have me more worried :-) >>> For the mobile version the infrastructure should be already in place. >>> the MapWidget needs to instantiated somewhere, some extra signal/slot >>> handling needs to be added and it should be good to go (hopefully). >> >> Famous last words :-) > > indeed. > i just removed some stuff from the PR, as the mobile version was broken. > > which confirms that i haven't tested it all. Ha :-) /D _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
