<table><tr><td style="">ngraham added a comment.
</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/D18939">View Revision</a></tr></table><br /><div><div><p>Thanks! The UI and behavior are now good. Code looks pretty sane too, just a few style nitpicks:</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/D18939#inline-105595">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kmenuedit.cpp:30</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; "><span style="color: #304a96">#include</span> <span class="cpf"><QIcon></span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">
</div><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #304a96">#include</span> <span class="cpf"><KLocalizedString></span><span style="color: #304a96"></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Unrelated change.</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/D18939#inline-105597">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kmenuedit.cpp:141</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">m_searchLine</span><span style="color: #aa2211">-></span><span class="n">setPlaceholderText</span><span class="p">(</span><span class="n">i18n</span><span class="p">(</span><span style="color: #766510">"Search..."</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">m_searchLine</span><span style="color: #aa2211">-></span><span class="n">setToolTip</span><span class="p">(</span><span class="n">i18n</span><span class="p">(</span><span style="color: #766510">"Type to filter applications below"</span><span class="p">));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Since the placeholder text says "Search..." we should probably not use the word "Filter" in the tooltip. Also "Type to" isa not really necessary since that's the only thing you can do with a search field. :)</p>

<p style="padding: 0; margin: 8px;">How about something like "Search through the list of applications below"?</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/D18939#inline-105598">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kmenuedit.cpp:143</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">auto</span> <span class="n">treeLayout</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">new</span> <span class="n">QVBoxLayout</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">treeLayout</span><span style="color: #aa2211">-></span><span class="n">addWidget</span><span class="p">(</span><span class="n">m_searchLine</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">No need to use <tt style="background: #ebebeb; font-size: 13px;">auto</tt> here</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/D18939#inline-105596">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kmenuedit.cpp:146</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">treeLayout</span><span style="color: #aa2211">-></span><span class="n">addWidget</span><span class="p">(</span><span class="n">m_tree</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #aa4000">auto</span> <span class="n">treeFrame</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">new</span> <span class="n">QFrame</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">treeFrame</span><span style="color: #aa2211">-></span><span class="n">setLayout</span><span class="p">(</span><span class="n">treeLayout</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">No need to use <tt style="background: #ebebeb; font-size: 13px;">auto</tt> here</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/D18939#inline-105599">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kmenuedit.cpp:157</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; ">    <span class="n">connect</span><span class="p">(</span><span class="n">m_tree</span><span class="p">,</span> <span class="n">SIGNAL</span><span class="p">(</span><span class="n">entrySelected</span><span class="p">(</span><span class="n">MenuFolderInfo</span><span style="color: #aa2211">*</span><span class="p">)),</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Unrelated change</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/D18939#inline-105602">View Inline</a><span style="color: #4b4d51; font-weight: bold;">treeview.cpp:1922</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 style="color: #74777d"> * Workaround to set expansion of categories when search updates</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> */</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I'm not sure this is actually working around anything; it's just a nice UI behavior! :)</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/D18939#inline-105600">View Inline</a><span style="color: #4b4d51; font-weight: bold;">treeview.h:208</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; "><span style="color: #a0a000">protected</span><span class="p">:</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Unrelated change</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R103 KMenu Editor</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D18939">https://phabricator.kde.org/D18939</a></div></div><br /><div><strong>To: </strong>tuxxi, ngraham, Plasma, cfeck<br /><strong>Cc: </strong>ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>