<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/103618/">http://git.reviewboard.kde.org/r/103618/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 3rd, 2012, 3:03 p.m., <b>Marco Martin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">hiding the selection + icon should happen automatically when doubleclick is disabled, i don't see a valid use case to be able to configure it idependently</pre>
</blockquote>
<p>On January 3rd, 2012, 3:12 p.m., <b>Shaun Reich</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think/hope you meant that the selection icon should be hidden when double is *enabled*. i.e. singleclick is on. Since the selection icon is the most useful when you can't click to select (single-click mode only).
That said, the option has existed since forever in Dolphin, if we're going to bring a feature over, we'd better bring the whole thing and not just bits and pieces to make it inconsistent (as the patcher duly noted.</pre>
</blockquote>
<p>On January 3rd, 2012, 3:18 p.m., <b>Marco Martin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">yes, selection icon enabled when double click is disabled.
but honestly, i rather see it as a problem of dolphin if it lets configure such a thing.
only thing that would make sense (since removing that from dolphin would probably cause a revolution as the removal of anything) is to read that setting from the dolphin configuration itself rather than the applet own one</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well, given how stringent Peter is on settings, I'm sure he didn't add it without quite some thought and good reasons ;)
But yes, for unification's sake, it definitely makes the most sense to make it as global as can be. Otherwise they have crap to configure in dolphin and then every folderview they own. Plus it keeps it clean; good idea.</pre>
<br />
<p>- Shaun</p>
<br />
<p>On January 3rd, 2012, 1:03 p.m., Ignat Semenov wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma and Aaron J. Seigo.</div>
<div>By Ignat Semenov.</div>
<p style="color: grey;"><i>Updated Jan. 3, 2012, 1:03 p.m.</i></p>
<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;">This patch implements a proposal from the Netrunner project (netrunner-os.com), a KUbuntu derivative, aimed at polishing the KDE desktop, as part of my work for the project.
This feature allows the user to hide the "+" selection marker in the FolderView applet. It is absolutely symmetrical to the "Click to view folder" feature. The code is similar as well. One thing I had to change is the button hiding logic in ActionOverlay. The selection marker is positioned above the folder peek button, so it needs a QGraphicsGridLayout::removeItem() / ::addItem() instead of a QWidget::hide(), performed every time on the entered() event. As the ::removeItem() has to be called only once, I had to implement a public function that would toggle the button hiding / unhiding.
Then, to be symmetrical, I changed the folder peek button hiding code to use the same functions, with the same public interface in the ActionAoverlay class.
I hope the function addition is OK and does not violate neither incapsultion nor the KDE class design principles.</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;">Works fine.
The only problem is that following a recent Aaron's commit regarding configChanged() / configAccepeted() in the FolderView applet circa 2 weeks ago, the settings in this applet are applied only on Plasma restart and not on the fly after hitting the Appy or Ok buttons. Aaron, please, have a look into this. This happens with any settings, not onyl the ones I added. (My settings code is absolutely identical to the "Click to view folder" settigns code).</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>plasma/applets/folderview/actionoverlay.h <span style="color: grey">(056c83b)</span></li>
<li>plasma/applets/folderview/actionoverlay.cpp <span style="color: grey">(430e6dc)</span></li>
<li>plasma/applets/folderview/folderview.h <span style="color: grey">(c8869b4)</span></li>
<li>plasma/applets/folderview/folderview.cpp <span style="color: grey">(d620a7d)</span></li>
<li>plasma/applets/folderview/folderviewDisplayConfig.ui <span style="color: grey">(e7a5e46)</span></li>
<li>plasma/applets/folderview/iconview.h <span style="color: grey">(677aa76)</span></li>
<li>plasma/applets/folderview/iconview.cpp <span style="color: grey">(abf775e)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103618/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>