<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/124627/">https://git.reviewboard.kde.org/r/124627/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't like the redundancy this patch introduces by maintaining a second route request model. This would become a nightmare to maintain. Instead there should only be one model -- the C++ based RouteRequestModel. All code on top needs to work on the very same data e.g. like declarative/RouteRequestModel does. The problem you describe sounds like being related to the redundancy already, and another place where things are diverting already is on startup when I previous route is loaded: RouteRequestModel has all the data and the route is drawn fine, but the placemarks do not show up because the second model has not been populated. Let's avoid these issues in the first place.
Using a delegate similar to https://git.reviewboard.kde.org/r/124550/ should work much nicer.</p></pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/124627/diff/2/?file=390574#file390574line13" style="color: black; font-weight: bold; text-decoration: underline;">src/apps/marble-maps/Search.qml</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">13</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nx">signal</span> <span class="nx"><span class="hl">navigation</span>Requested</span><span class="p">()</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">13</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nx">signal</span> <span class="nx"><span class="hl">setRoutingButtonVisible</span>Requested</span><span class="p">()</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'd rather call the signal itemSelected. It's up to the connected slot to determine what to do with it.</p></pre>
</div>
</div>
<br />
<p>- Dennis Nienhüser</p>
<br />
<p>On August 6th, 2015, 7:36 a.m. UTC, Gábor Péterffy wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Marble.</div>
<div>By Gábor Péterffy.</div>
<p style="color: grey;"><i>Updated Aug. 6, 2015, 7:36 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
marble
</div>
<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; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch adds the following:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Redesigned routing UI</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Markers on the map</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Waypoint order editing</li>
</ul></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; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It works fine except one thing: when you call the redrawRoute() it draws the route until the first placemark, but if you changes the routing profile it draws the full road. Routing instructions are also missing in this case.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>data/android/drawable-xxxhdpi/delete.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>data/android/drawable-xxxhdpi/down.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>data/android/drawable-xxxhdpi/place_white.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>data/android/drawable-xxxhdpi/up.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/apps/marble-maps/FloatingMenuButton.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/apps/marble-maps/MainScreen.qml <span style="color: grey">(b8dc7b0)</span></li>
<li>src/apps/marble-maps/MarbleMaps.qrc <span style="color: grey">(0168893)</span></li>
<li>src/apps/marble-maps/NavigationSetup.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/apps/marble-maps/ProfileSelectorMenu.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/apps/marble-maps/RoutingManager.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/apps/marble-maps/Search.qml <span style="color: grey">(9b712c8)</span></li>
<li>src/apps/marble-maps/Waypoint.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/apps/marble-maps/WaypointOrderManager.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/marble/declarative/MarbleQuickItem.h <span style="color: grey">(cefb0c0)</span></li>
<li>src/lib/marble/declarative/MarbleQuickItem.cpp <span style="color: grey">(f8c396f)</span></li>
<li>src/lib/marble/declarative/Placemark.h <span style="color: grey">(8c17b4f)</span></li>
<li>src/lib/marble/declarative/Placemark.cpp <span style="color: grey">(27a9579)</span></li>
<li>src/lib/marble/declarative/Routing.cpp <span style="color: grey">(dbbcb80)</span></li>
<li>src/lib/marble/declarative/SearchBackend.h <span style="color: grey">(4981c31)</span></li>
<li>src/lib/marble/declarative/SearchBackend.cpp <span style="color: grey">(fdef702)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124627/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/c34d1f1c-cdb8-4f4b-9c0f-7c9fc479c5a5__place_white.png">place_white.png</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/0dd988f0-189e-4ce6-8dd8-5f4e8f07713a__up.png">up.png</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/f0557e37-7c1b-4447-93d2-201c8b05efe4__down.png">down.png</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/fb26b212-cfc0-40d4-90ac-29bbe514dec4__delete.png">delete.png</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/85a13541-3168-499b-9c6a-776f889fe5d7__Screenshot_2015-08-06-01-44-58.png">Screenshot I</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/2dca53bf-7941-4bd0-8ec2-1e517e915ae8__Screenshot_2015-08-06-01-43-41.png">Screenshot II</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/08/05/7b24796b-a13c-4508-87c7-9861f779a15c__Screenshot_2015-08-06-01-44-51.png">Screenshot III</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>