<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/110430/">http://git.reviewboard.kde.org/r/110430/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 15th, 2013, 6:06 a.m. UTC, <b>Aaron J. Seigo</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 am not in favour of this patch for a couple of reasons. First, there is a port underway to QML which does not need to be delayed further by adding more features. More importantly, however, "middle click" is a special case of "not the first two mouse buttons" (should we support N button mice?) and it adds yet more configuration to an already complex and full configuration dialog. It also conflicts with the meaning of middle click to expand / collapse groups (a fairly hidden feature I also was not particularly happy with tbh).</pre>
</blockquote>
<p>On May 15th, 2013, 12:54 p.m. UTC, <b>Albert Vaca Cintora</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;">Hello Aaron and thank for your reply.
Let me defend the inclusion of this patch from the problems you mention:
- Difficulty to port to QML: This feature is implemented in a ~10 lines switch (not counting the GUI-related code), so porting it should not be a problem (I could do it, if needed).
- Support for N button mice: The desktop should adapt to the current hardware, and nowadays most mice have 3 buttons (not N, but neither only 2). Moreover, lots of apps have adopted the behaviour of closing tabs with a middle click, so adding this feature for applications in the taskbar is consistent with them.
- Complexity of the configuration dialog: I agree that we should try to keep all the configuration dialogs simple, but not adding new features because of that reminds me of Gnome3 ;) I think this is not solution for the long-discussed problem of the user-friendliness.
Finally, and more important, it has 2 open bugs (+ 4 duplicates) so there are real users requesting it. If it's not going to be added, those bugs should be marked as wontfix.</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;">"porting it should not be a problem (I could do it, if needed)."
that is definitely a point in your favor. assuming this feature addition is accepted: there's little point to putting it in the c++ version, however, only to put it later in the qml version (which is supposed to be in 4.11). so putting it directly in the QML version would make the most sense.
"Moreover, lots of apps have adopted the behaviour of closing tabs with a middle click"
to point out the obvious: windows are not tabs. this also implies that middle click to close is actually a *good* feature. i think it is for power users .. but i have seen too many instances where these kinds of magic "click that button and magically something happens" features lead to confusion, and confusion leads to distrust of the system as a whole.
yes, the default is to do nothing in your patch (+1 for that), but if sitting down to someone else's system results in wildly unpredictable behaviour .... ... keep in mind this is not a random component, but a default part of every plasma desktop installation, so it has to work better than most.
how much faster is middle click versus right-click->close? fast enough to warrant the risk of surprising behaviour for people who aren't expecting it?
"Complexity of the configuration dialog: I agree that we should try to keep all the configuration dialogs simple"
currently that page has 11 settings. ad-hoc testing i've done in the past hints that many of these settings are already not understandable to many users. i really don't want to make this worse. several of the plasmoids have "room" for more options. the taskbar, folderview, clock and system tray are four that really don't. adding feature over feature is leading to an increasing mess that nobody cares to clean up.
if this patch introduced a re-think of the configuration presentation so that it is easier to understand and more approachable, i would consider that a fair trade for accepting a feature i'm not particularly in favour of :)
"but not adding new features because of that reminds me of Gnome3"
for future reference: when i see this kind of statement made, i usually add -1 to my overall weighting. i do not agree with many design decisions in other projects, but design can not be done well if it is primarily directed by "not being that other group". common sense and reasoning should be applied to each case without the justification of "not like them", otherwise we just create the opposite kind of error.
"it has 2 open bugs (+ 4 duplicates) so there are real users requesting it."
for any product with a large enough user base, given enough time and an open feature request tracker, any possible feature will be requested (usually multiple times). this means that any feature, regardless of intrinsic value, can be justified using this argument.
(the least useful case of this i've seen is when people submit the feature request, then later a patch and then use the feature request as evidence it is wanted ;)
</pre>
<br />
<p>- Aaron J.</p>
<br />
<p>On May 14th, 2013, 10:35 p.m. UTC, Albert Vaca Cintora wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kde-workspace and Plasma.</div>
<div>By Albert Vaca Cintora.</div>
<p style="color: grey;"><i>Updated May 14, 2013, 10:35 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 adds a feature present in KDE3 and requested by some users (see open bugs), that is performing an action when a taskbar entry is middle-clicked. I have added three possible actions: Close the application, hide it or move it to the current desktop, where the first two were user requests.</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;">Manual testing.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=182689">182689</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=190951">190951</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>plasma/desktop/applets/tasks/tasks.h <span style="color: grey">(fe79a12)</span></li>
<li>plasma/desktop/applets/tasks/tasks.cpp <span style="color: grey">(0a86dcf)</span></li>
<li>plasma/desktop/applets/tasks/tasksConfig.ui <span style="color: grey">(6f3ff18)</span></li>
<li>plasma/desktop/applets/tasks/windowtaskitem.cpp <span style="color: grey">(f840076)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110430/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>