<table><tr><td style="">ngraham accepted this revision.<br />ngraham added a comment.<br />This revision is now accepted and ready to land.
</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/D18658">View Revision</a></tr></table><br /><div><div><p>Looks great to me. Please wait to land this until <a href="https://phabricator.kde.org/p/aacid/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@aacid</a> or another Okular developer is okay with bumping the Frameworks dependency to 5.56.</p>
<p>BTW <a href="https://phabricator.kde.org/p/aacid/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@aacid</a>, what's your objection about doing this, out of curiosity? People on rolling release distros will surely have 5.56 by the time Applications 19.04.0 is released, and once time discrete release distros get around to shipping 19.04.0, I expect that they'll have already shipped a new enough version of Frameworks, if not even newer.</p>
<p>Also <a href="https://phabricator.kde.org/p/ognarb/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@ognarb</a>, now that I see all these patches, I'm wondering if it might make sense to create two new Kirigami components, each based on your very nice <tt style="background: #ebebeb; font-size: 13px;">ActionTextField</tt>:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item"><tt style="background: #ebebeb; font-size: 13px;">SearchField</tt>, which includes all the search field boilerplate that you're adding here (keyboard shortcut, clear button, placeholder text, etc)</li>
<li class="remarkup-list-item"><tt style="background: #ebebeb; font-size: 13px;">PasswordField</tt>, which has a clear button as well as a "show password" button.</li>
</ul>
<p>This would be very Kirigami-ish, in the spirit of providing high-level controls so that apps don't need to reinvent the wheel all the time. And then we would have total UI consistency for all instances of these fields.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>BRANCH</strong><div><div>arcpatch-D18658</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D18658">https://phabricator.kde.org/D18658</a></div></div><br /><div><strong>To: </strong>ognarb, Okular, ngraham<br /><strong>Cc: </strong>aacid, ngraham, okular-devel, tfella, darcyshen<br /></div>