<table><tr><td style="">hein requested changes to this revision.<br />hein added a comment.<br />This revision now requires changes to proceed.
</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/D3931" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>I'm not entirely happy with the way this works, I think the history may need to be kept on the QML side instead, then:</p>

<p>(a) You can properly evict the history when the URL in the config changes<br />
(b) Tie the visibility of the Back button to whether the history is empty or not</p>

<p>So basically before QML runs FolderModel::cd write down the URL, then when back is pressed pop it off.</p>

<p>FolderModel can't know when to evict the history because it can't distinguish between setUrl coming from cd() or from a config change, only the applet code can.</p>

<p>Having history functionality in FolderModel is otherwise attractive, so this is a bit of a dilemma. Putting the history into QML however keeps FolderModel "dumber" in the sense that it doesn't have to reason about intent and just does what it's told, and the higher level situation is captured on the side of the user of the model instead.</p></div></div><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/D3931#inline-15685" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">foldermodel.cpp:487</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: #ffd0d0;">    <span class="bright">    </span><span class="n"><span class="bright">setUrl</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">up</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">toString</span></span><span class="bright"></span><span class="p"><span class="bright">());</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">    <span class="bright"></span><span style="color: #aa4000"><span class="bright">if</span></span><span class="bright"> </span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">backUrl</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">isValid</span></span><span class="bright"></span><span class="p"><span class="bright">())</span></span><span class="bright"> </span><span class="p"><span class="bright">{</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">        <span class="n">setUrl</span><span class="p">(</span><span class="n">backUrl</span><span class="p">.</span><span class="n">toString</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">        <span class="n">m_history</span><span class="p">.</span><span class="n">removeLast</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">For future reference: This can be simplified to takeLast().</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R119 Plasma Desktop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D3931" rel="noreferrer">https://phabricator.kde.org/D3931</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>chinmoyr, hein, Plasma<br /><strong>Cc: </strong>plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas<br /></div>