> On Oct. 10, 2012, 2:46 p.m., Dario Freddi wrote: > > Besides the fact that I like the principle of the idea, I am not sure > > whether the approach of hiding works better than disabling. I am saying > > this since it's quite common to have people unplugging their battery at > > times to save some of its lifecycle. Hence, the fact that batteryCount == 0 > > might also mean that the battery is simply unplugged, and maybe if that's > > the case having the fields simply disabled might make more sense. What do > > you think? > > Kai Uwe Broulik wrote: > Correct me if I am wrong but I think in 90% of cases when there is no > battery present, it is probably a desktop computer where you don’t need that > option at all. And since you cannot configure those options anyway if there > is no battery, I think hiding it is okay. I actaully cannot think of a person > that I know that removes the battery when the computer is running in a > docking station powered all the time. > Maybe it is better (if possible?) to change the check to check for "Is > Notebook" or "Is Desktop Computer" instead of looking for a present battery? > I don’t know how this works but at least kickoff shows a notebook icon on my > notebook and a computer on my desktop. :) > > Dario Freddi wrote: > Definitely, that would be spot-on. We should definitely investigate the > Kickoff model, otherwise your rationale definitely makes sense and this patch > might be good to go. What I'm gonna do is add a "ship it!" anyway since > you're totally right - if you fancy having a look to what Kickoff does (but I > think it is probably following the same battery logic), feel free to do so, > otherwise this patch is good to me. > > P.S.: Thanks for these small papercuts, they're really, really > appreciated! :) > > Kai Uwe Broulik wrote: > Just had a look at the kickoff code and yes, it uses: > QList<Solid::Device> batteryList = > Solid::Device::listFromType(Solid::DeviceInterface::Battery, QString()); > > if (batteryList.isEmpty()) { > return KIcon("computer"); > } else { > return KIcon("computer-laptop"); > } > And I apologize for not yet having committed all my other review requests > but I have a new notebook that doesn’t yet have commit access (and I am not > at home atm). Don’t worry - and there’s still a few days left for feature > freeze :) > > Dario Freddi wrote: > Perfect, then the ship it remains. Btw, should you have any problems with > committing the RRs in time, ping me ;) > > Alex Fiestas wrote: > It may not be the best solution (but surely it is slightly better) > > Check if there is a LID (laptop screen). If there is no LID then it is no > laptop :) > > Kai Uwe Broulik wrote: > For Solid we need to check for the batteries because the check is for > disabling or enabling the warning threshold configs. > For Kickoff there needs to be a more elegant way. I think we need > something like this in Solid so we can check for thisDevice() == > Solid::DesktopComputer, or LaptopComputer or Tablet, etc. and this handles > the checks for eg. a lid, or uses DMI or so.
Alex, desktop computers that uses UPS have a battery but no lid. This configuration is related to batteries not to laptops in particular. - Lamarque Vieira ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106783/#review20136 ----------------------------------------------------------- On Oct. 10, 2012, 3:35 p.m., Kai Uwe Broulik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106783/ > ----------------------------------------------------------- > > (Updated Oct. 10, 2012, 3:35 p.m.) > > > Review request for Solid. > > > Description > ------- > > This little patch hides the config options for battery levels altogether when > there is no battery present. Reducses visual clutter. > > > Diffs > ----- > > powerdevil/kcmodule/global/GeneralPage.cpp 5ff3f4d > > Diff: http://git.reviewboard.kde.org/r/106783/diff/ > > > Testing > ------- > > Didn’t test whether the elements are really hidden in the real condition > (Have a notebook only here, but I know that if works) but commented out the > if and the elements are all properly removed. > > > Thanks, > > Kai Uwe Broulik > >
_______________________________________________ Kde-hardware-devel mailing list Kde-hardware-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-hardware-devel