[Kde-hardware-devel] Review Request 121508: Allow turning off keyboard in dim display action

[Sebastian Kügler]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121508/#review72043
-----------------------------------------------------------


One comment inline.

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.


daemon/actions/bundled/dimdisplayconfig.cpp
<https://git.reviewboard.kde.org/r/121508/#comment50232>

    This key name is misleading. It doesn't turn off the keyboard, but its LEDs behind it.


- Sebastian Kügler


On Dec. 14, 2014, 11:57 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121508/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2014, 11:57 p.m.)
> 
> 
> Review request for Solid and KDE Usability.
> 
> 
> Repository: powerdevil
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   daemon/actions/bundled/dimdisplay.h 1564df1 
>   daemon/actions/bundled/dimdisplay.cpp e1bb4f0 
>   daemon/actions/bundled/dimdisplayconfig.h acf5071 
>   daemon/actions/bundled/dimdisplayconfig.cpp 2fe67dc 
> 
> Diff: https://git.reviewboard.kde.org/r/121508/diff/
> 
> 
> Testing
> -------
> 
> 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.
> 
> 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). 
> 
> 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?!
> 
> 
> File Attachments
> ----------------
> 
> Settings in action
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/12/14/f4650b04-42c1-4d4b-b772-710623300fa9__dimkeyboard.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20141215/4b92239f/attachment.html>