<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://reviewboard.kde.org/r/4461/">http://reviewboard.kde.org/r/4461/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 27th, 2010, 12:12 a.m., <b>Markus Slopianka</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre>Great to see that feature being worked on. I'm not a programmer, so I can't comment on the source quality, but I'd like to comment on the wording of the button.
I think that "&Edit Map Externally" is not a good phrase. I think plain "&Edit Map" is sufficient and while the map is technically edited with an external app, the full phrase sounds (to me at least) a bit awkard and discouraging.
If you think that the users should be warned that an external app will open (I don't think that's necessary) a message box could notify the user the first time (s)he clicks on the button.</pre>
</blockquote>
<p>On June 27th, 2010, 6:17 a.m., <b>Torsten Rahn</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre>I agree with Markus - for a different reason. Remember that "Edit Map Externally" probably becomes 3x as long in other languages (like french or hm, German "Editiere Karte außerhalb von Marble" ;-).
Another issue that the message box would solve are the required instructions that the user needs to perform: because that's really something that I consider a major issue. Is there a chance that we do this automatically via dbus for Mercator in the future?
Also: What's the problem with disabling the menu entry for all other map themes? :-) You just need to connect to the widget's mapThemeChanged() signal and depending on the result enable or disable the action.
</pre>
</blockquote>
</blockquote>
<pre>Well it would be "Karte extern bearbeiten" in German, but I wasn't too happy with the long version either. The edit button is disabled now for the other themes, no problem with that, just the code wasn't written yet.
I'll look at some first-run dialog in the next days (to be shown when no editor preference was set yet). Note that neither josm nor merkaartor *require* you to run their servers. Marble can still start and have them download the appropriate map region. The servers are just used to re-use an already running josm/merkaartor instance instead of starting a new one each time "Edit Map" is called in Marble.
@Torsten: I'm not sure whether I understood "Another issue that the message box would solve are the required instructions that the user needs to perform: because that's really something that I consider a major issue. Is there a chance that we do this automatically via dbus for Mercator in the future?" correctly: Do you mean to set up the merkaartor server via dbus? I don't want to sneak that in behind the users back, but have them enable it explicitly. It opens a port. Would be fine to have a merkaartor dbus interface instead of having to use a server though to talk to it.
</pre>
<br />
<p>- Dennis</p>
<br />
<p>On June 27th, 2010, 11:53 a.m., Dennis Nienhüser wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for marble.</div>
<div>By Dennis Nienhüser.</div>
<p style="color: grey;"><i>Updated 2010-06-27 11:53:56</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">Adds an "Edit Map Externally" menu item in the main application (currently Qt only) that does either:
- open a web browser using potlatch (flash based osm builtin map editor) at the current view (default)
- execute josm or merkaartor (whichever is enabled in settings)
- update a running josm or merkaartor if they opened a server at localhost:8111
The choice which application to use (potlatch, josm, merkaartor) is left to the user. A running server is detected automatically if josm or merkaartor is selected by the user. The default choice is potlatch because it will be usable for most users out of the box (requirements are a web browser with flash support).
Most of the implementation is done in ControlView, i.e. outside lib/, but easily shareable between qt and kde main applications. I can implement it in its own class if you think it clutters ControlView too much.
The edit button is visible always currently. It may be better to hide it for map views other than openstreetmap to avoid confusing users.
Please note that josm and merkaartor should not be too outdated, especially for their server applications.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">Tested with konqueror for potlatch. Tested Merkaartor 0.16.1. Merkaartor 0.16.0 may work (not tested), older versions lack the server. Tested josm version 3329. Note that you need to enable the server explicitly for merkaartor (tools => settings => network) and josm (edit => preferences => plugins, install and enable the remotecontrol plugin)</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=240968">240968</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdeedu/marble/src/ControlView.h <span style="color: grey">(1143093)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/ControlView.cpp <span style="color: grey">(1143093)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/QtMainWindow.h <span style="color: grey">(1143093)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/QtMainWindow.cpp <span style="color: grey">(1143093)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleNavigationSettingsWidget.ui <span style="color: grey">(1143093)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/QtMarbleConfigDialog.h <span style="color: grey">(1143093)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/QtMarbleConfigDialog.cpp <span style="color: grey">(1143093)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4461/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>