----------------------------------------------------------- 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 > >
_______________________________________________ Kde-hardware-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-hardware-devel
