[Marble-devel] Review Request 110019: KML Editor Dock Widget

[Dennis Nienhüser]

> On April 17, 2013, 5:44 p.m., Thibaut Gridel wrote:
> > Interesting Patch, please provide the details of what gets changed in AnnotationPlugin for commenting.
> > 
> > From the commit, I see that the DockWidget is created from the Plugin class and all the UI belongs there.
> > I think that code should move to another class to separate concerns: the Plugin deals with master data and plugin lifecycle,
> > then a possible widget class uses data from the Plugin.
> 
> Adrian Draghici wrote:
>     This is the diff for the AnnotatePlugin class: http://paste.kde.org/726710/
>     
>     Having the virtual function createDockWidget() in RenderPlugin and overriding it in each plugin for a custom widget seems fairly clean to me.
>     
>     How do you suggest refactoring it?
> 
> Bernhard Beschow wrote:
>     @Thibaut: Yes, I see that as a problem, too. In particular, I think that adding *application-specific* logic to the generic RenderPlugin class (which is supposed to render some features onto the map) is a sign for an architectural issue.

How shall we move on here? Are there objections against the Dock Widget approach in general? If not I'd suggest moving the code from the plugin somewhere next to ControlView (outside of the library and plugins, to be shared by the Marble Qt and KDE application). In my opinion *all* code from the plugin could move to the applications (the UI related parts) and the library (geographics scene related parts).
Shall we remove, or at least deprecate the existing toolbar functionality in RenderPlugin as well? It's application specific as well and in Marble only used by this plugin.


- Dennis


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


On April 16, 2013, 7:01 p.m., Adrian Draghici wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110019/
> -----------------------------------------------------------
> 
> (Updated April 16, 2013, 7:01 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Description
> -------
> 
> KML Editor Dock Widget replacing the previous toolbar used to display the actions provided by the annotate plugin
> 
> 
> This addresses bug 317791.
>     http://bugs.kde.org/show_bug.cgi?id=317791
> 
> 
> Diffs
> -----
> 
>   src/QtMainWindow.h b02cd36 
>   src/QtMainWindow.cpp cc69642 
>   src/lib/BranchFilterProxyModel.h 3ad87a5 
>   src/lib/RenderPlugin.h 330730a 
>   src/lib/RenderPlugin.cpp b9b95c9 
>   src/plugins/render/annotate/AnnotatePlugin.h e6111c2 
>   src/plugins/render/annotate/AnnotatePlugin.cpp 72d2ab3 
> 
> Diff: http://git.reviewboard.kde.org/r/110019/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Adrian Draghici
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20130501/2ac25a75/attachment.html>