<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/121508/">https://git.reviewboard.kde.org/r/121508/</a>
</td>
</tr>
</table>
<br />
<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;">One comment inline.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Like others, I don't see why we need a checkbox for this. Who, with a sane mind, would say: "dim the backlight as much as possible, but leave it on?". I don't think that's a usecase we should support. Switching off the light is simply the sensible thing to do, no need to create additional maintenance and testing work with a checkbox.</p></pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/121508/diff/1/?file=333152#file333152line51" style="color: black; font-weight: bold; text-decoration: underline;">daemon/actions/bundled/dimdisplayconfig.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">DimDisplayConfig::~DimDisplayConfig()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">51</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">configGroup</span><span class="p">().</span><span class="n">writeEntry</span><span class="p">(</span><span class="s">"turnOffKeyboard"</span><span class="p">,</span> <span class="n">m_keyboardCheck</span><span class="o">-></span><span class="n">isChecked</span><span class="p">());</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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;">This key name is misleading. It doesn't turn off the keyboard, but its LEDs behind it.</p></pre>
</div>
</div>
<br />
<p>- Sebastian Kügler</p>
<br />
<p>On December 14th, 2014, 11:57 p.m. 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 and KDE Usability.</div>
<div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated Dec. 14, 2014, 11:57 p.m.</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;">One thing that always bugged me is that my keyboard backlight stays on even when the screen got turned off/dimmed by power management. This patch adds a checkbox (enabled by default) that turns off the keyboard as soon as it starts dimming the display.
It does not gradually taper down the brightness since keyboard backlights usually aren't that finegrained anyway.</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;">Works as expected, I forgot to make it honor the setting, though. Waited a bit for it to start dimming the display and keyboard turned off. After moving my mouse it was restored to its previous value.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I didn't find a way to conditionally disable the checkbox in case no keyboard backlight was present. But I didn't want to add yet another separate action (so I could use the isSupported() thing). </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">But then I'm unsure whether it should be an option at all. Why would you want to have your screen dim but not your keyboard?!</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>daemon/actions/bundled/dimdisplay.h <span style="color: grey">(1564df1)</span></li>
<li>daemon/actions/bundled/dimdisplay.cpp <span style="color: grey">(e1bb4f0)</span></li>
<li>daemon/actions/bundled/dimdisplayconfig.h <span style="color: grey">(acf5071)</span></li>
<li>daemon/actions/bundled/dimdisplayconfig.cpp <span style="color: grey">(2fe67dc)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121508/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/2014/12/14/f4650b04-42c1-4d4b-b772-710623300fa9__dimkeyboard.png">Settings in action</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>