> 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! :)
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 :)
- Kai Uwe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106783/#review20136
-----------------------------------------------------------
On Oct. 10, 2012, 2:36 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, 2:36 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
> powerdevil/kcmodule/global/generalPage.ui 7840526
>
> 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
[email protected]
https://mail.kde.org/mailman/listinfo/kde-hardware-devel
