<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/122048/">https://git.reviewboard.kde.org/r/122048/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Januar 14th, 2015, 12:29 nachm. UTC, <b>Sebastian Kügler</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I really like this feature.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">However, I think that UI wise, it looks a bit unpolished. It adds two checkboxes with long texts, that even I, being a domain expert, need some thinking to understand.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I wonder if we should offer these actions at all, or whether we should enable the features by default. They do seem useful, I can construct a use-case pretty easily, but we may get away without the UI options, still keeping the config keys, perhaps. Then wait until someone complains or reports unexpected behavior, and then rethink adding the options.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Inline, a few niggles about naming when reading the code and trying to understand it, nothing really major.</p></pre>
</blockquote>
<p>On Januar 14th, 2015, 12:49 nachm. UTC, <b>Kai Uwe Broulik</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That's why I added the usability group. :)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You are right that actually neither of these features really needs to be optional. Turning off that "Do not inhibit on lid close" (I guess nobody really knew from the UI what that really does anyway) is just tupid in my opinion. Why would you ever <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">not</em> want your notebook to suspend when you, all by yourself, close the lid?! That's no automatism interrupting you, it's you closing the lid.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Usecases I could imagine you not wanting to suspend when closing the lid is while watching a movie on your TV or when you put your notebook into a docking station, but that's what the other option is for. Using your laptop as a jukebox on a party might want you to play music, close it so nobody spills beer on the keyboard and not have it suspend (Amarok eg. allows to inhibit suspend during playback) but I think most people would just opt to having the lid opened a bit rather than messing with complicated settings they probably don't know exist in the first place.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Usability team, what do you think? Do we need settings for either of them?</p></pre>
</blockquote>
<p>On Januar 14th, 2015, 1:07 nachm. UTC, <b>Heiko Tietze</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It makes sense to hide options that nobody will use. But as you said there might be some use cases. So what's about a setting like 'use it in this way (on/off)' but 'allow application to override (on/off)'. I have OpenGL antialiasing or the like in mind.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So my suggestion is
Close lid: [Hibernate/Suspend/Shut down/Start Activity]
[x] Allow applications to override setting</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This makes KScreen responsible to handle the issue when an external display is connected. </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">About the label, HIG for checkboxes contains the advice "Checking a check box should always "enable" an option or change the state of an option to "on". Checking a negative or disabling option is a double negative and causes confusion and errors." (https://techbase.kde.org/Projects/Usability/HIG/Check_Box)</p></pre>
</blockquote>
<p>On Januar 14th, 2015, 1:51 nachm. UTC, <b>Sebastian Kügler</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Bit problematic here is that "inhibit" is already a negation. That may be adding to possible confusion.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I' still in favor of not offering the option in the UI for now. Let's see if someone needs it and lets us know.</p></pre>
</blockquote>
<p>On Januar 16th, 2015, 7:53 vorm. UTC, <b>Achim Bohnet</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Instead of 'hiding' this option in the kcm that the average user maybe never visit, how about also delivering it to the user eyes just in the right moment?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For every external monitor connected: only on the first lid close, ask the user what to do right now and on future lid close:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> [Suspend] [Ignore]
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">with a 'standard' 30 sec countdown. What's selected by default could be controlled with a config option without a kcm item. If the user
changes it's mind, the user has to visit the kscreen kcm to change the default action for this monitor.</p></pre>
</blockquote>
<p>On Januar 16th, 2015, 11:40 vorm. UTC, <b>Thomas Pfeiffer</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Achim's idea makes a lot of sense to me!
I'd suggest the following dialog:
"Laptop lid was closed. Suspending in 10 seconds"
with visuals like the "Confirm logout" dialog and the options "Do not suspend" and "Suspend" and a checkbox "Remember my decision".
That way users can either decide each time or keep the same decision. This should then be synced with the KCM.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In this situation, I think giving users a choice does make sense, because even people with a second monitor may want to use lid close as a quick way to suspend when they're away from the computer, but then when they're indeed e.g. watching a movie with the lid closed, they don't want it to suspend in that situation.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Alternatively we might remove the option in the KCM as well as the "Remember my decision" checkbox and just give the user the option to cancel the suspend each time, but that might annoy users who never want to suspend (while it should be fine with those who always want to suspend as they can just leave the computer alone and it will suspend 10 seconds later.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Discussion with Dan showed that it's quite difficult to have it remember that setting based on screen configuration and have it play nicely with PowerDevil; also he mentioned that it's quite difficult to "unlearn" things from KScreen.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I suggest we keep an option in settings to enable/disable that feature (wording "[X] when an external monitor is connected" ?) for users of eg. docking stations and leave the default at "don't suspend". Ideally that option could be tied to activities so your work activity suspends when closing the lid whereas your home activity does not. A regular notification/prompt asking for what to do won't neccessarily work since it'll most likely show up on the then closed primary monitor, and showing a "Log out screen style" dialog massively complicates things.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What I could imagine is adding a hint to battery monitor "Your notebook currently won't turn off when closing the lid because an external monitor is connected" similar to what we do with applications now.</p></pre>
<br />
<p>- Kai Uwe</p>
<br />
<p>On Januar 13th, 2015, 11:16 nachm. UTC, Kai Uwe Broulik wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Solid, KDE Usability, Àlex Fiestas, and Daniel Vrátil.</div>
<div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated Jan. 13, 2015, 11:16 nachm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
powerdevil
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Less sensational headline: Skip lid action when external monitor is connected.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This brings the KScreen killer feature in the 4.x times back. Now you can watch movies and safely close the lid again!</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The confusing "Never prevent an action on lid close" is also moved to the main page since it only affects the lid action and is used nowhere else. I'm not happy with the wording but "inhibition" is a difficult thing to describe for the average user.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Laptop only, monitor option on -> Suspend
Laptop only, monitor option off -> Suspend
TV connected, monitor option on -> No action
TV connected, monitor option off -> Suspend</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">PM enabled, inhibit option on -> Suspend
PM disabled, inhibit option on -> No suspend
PM enabled, inhibit option off -> Suspend
PM disabled, inhibit option off -> Suspend</p></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>CMakeLists.txt <span style="color: grey">(27f162c)</span></li>
<li>PowerDevilSettings.kcfg <span style="color: grey">(1bc7ce1)</span></li>
<li>daemon/CMakeLists.txt <span style="color: grey">(f1e6efb)</span></li>
<li>daemon/actions/bundled/handlebuttonevents.h <span style="color: grey">(8ea23f6)</span></li>
<li>daemon/actions/bundled/handlebuttonevents.cpp <span style="color: grey">(ac280f4)</span></li>
<li>daemon/actions/bundled/handlebuttoneventsconfig.h <span style="color: grey">(a55bca7)</span></li>
<li>daemon/actions/bundled/handlebuttoneventsconfig.cpp <span style="color: grey">(92f0cef)</span></li>
<li>kcmodule/global/GeneralPage.cpp <span style="color: grey">(5d9ff10)</span></li>
<li>kcmodule/global/generalPage.ui <span style="color: grey">(26204cb)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122048/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/01/13/e0956548-0a0f-4c41-b3dd-68a5f17815a5__powerdevilstuff.png">Config option</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>