<table><tr><td style="">mart added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D28383">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28383#inline-164526">View Inline</a><span style="color: #4b4d51; font-weight: bold;">tst_pagerouter.qml:56</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">id: router</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">initialRoute:</span> <span style="color: #766510">"/home"</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">columnView:</span> <span style="color: #004012">root</span><span class="p">.</span><span style="color: #004012">columnView</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I find all the "/" in the examples confusing, not sure is a good naming convention (and works prefectly without having that /, and i don't necessarly care what the convention in other frameworks is).<br />
in the hend you can't push directly something like "/home/login" as a single string so doesn't really make sense as paths, i would prefer examples had simple strings as route names, so your route would be indicated as ["home", "login"]</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28383#inline-164524">View Inline</a><span style="color: #4b4d51; font-weight: bold;">pagerouter.cpp:214</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #aa4000">void</span> <span class="n">PageRouter</span><span style="color: #aa2211">::</span><span class="n">navigateToRoute</span><span class="p">(</span><span class="n">QJSValue</span> <span class="n">route</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I wouldn't expect a verb like navigate to be always destructive.<br />
if the route happens to be the same, then the function should be a no-op<br />
if the current route is<br />
home/foo/bar and i want to navigate to home/foo/baz i would expect to remove and destroy only bar, keeping the first two pages</p>
<p style="padding: 0; margin: 8px;">if the new route is foo/bar/baz, should just push the new one keeping all is there</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28383#inline-164522">View Inline</a><span style="color: #4b4d51; font-weight: bold;">pagerouter.h:180</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Q_PROPERTY</span><span class="p">(</span><span class="n">ColumnView</span><span style="color: #aa2211">*</span> <span class="n">columnView</span> <span class="n">MEMBER</span> <span class="n">m_columnView</span> <span class="n">NOTIFY</span> <span class="n">columnViewChanged</span><span class="p">)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I know we are fixed to columnview now and its fine, but i would prefer the property being called pageStack like in applicationwindow (in reality i would like both being called pageview but it's too late) which doesn't assume which it is, if sme day we would like to support stackview or whatever else we wouldn't have a sore point in the api</p>
<p style="padding: 0; margin: 8px;">also, to the property assigning the pagerow (which will look for the columnview internally, but not exposed trough the api)</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28383#inline-163448">View Inline</a><span style="color: #4b4d51; font-weight: bold;">pagerouter.h:357</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">QML_DECLARE_TYPEINFO</span><span class="p">(</span><span class="n">PageRouter</span><span class="p">,</span> <span class="n">QML_HAS_ATTACHED_PROPERTIES</span><span class="p">)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">newlines!</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R169 Kirigami</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28383">https://phabricator.kde.org/D28383</a></div></div><br /><div><strong>To: </strong>cblack, Kirigami, mart, davidedmundson<br /><strong>Cc: </strong>ahiemstra, davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, mart<br /></div>