Battery Monitor revamp

[Kai Uwe Broulik]

Hi,

> tried it out and Marco (who is visiting for a few days) and i looked it
> over. see these screenshots for some layouting issues:
I didn't see those at first because I am using bigger icon sizes and bigger 
font sizes.
I guess using Math.max(48, iconSizes.dialog) would fix that? 48 is the icon 
size I use here where it looks great.

> also, after expanding a battery to see detailed information, collapsing it
> leaves an empty space. that should re-collapse, similar to thow the
> notifications plasmoid does.
Okay, will try to adapt the code that does that in notifications.

> overall it is showing some massive improvements over what we've had
> previously, so kudos for that.
Thanks.

> this looks pretty odd in the tooltip .. is that information actually useful
> in the tooltip, which is mean to be for quick-glance information, or does
> it just confront the user with jargon?
Since we cannot have an icon next to the battery in the tooltip, we only have 
text, which makes it harder to recognize at first sight. Didn't want to 
introduce inconsistency by naming things different in the tooltip than the 
popup. Ideas?

> also, would be nice to align the content in the tooltip with a (i know,
> horrible:) html table. we do this elsewhere and it looks a lot nicer.
> currently there is not much in the way of alignment:
Wasn't aware we could use a table there, yes, that would improve the situation 
significantly.
Will also have to add support for detecting whether an AC adapter is actually 
*present* rather than plugged in or not. There's no use in having it show up 
on the desktop. Imho we could also just drop that "AC Adapter: plugged in" 
text from the tooltip. The battery icon already reflects that.

> very nice..
I actually had an idea to show a text next to the battery when its "broken" 
(ie. low capacity) but on the other hand PowerDevil already "annoys" the user 
about that on every session start.

> sadly my wireless mouse battery is not shown. it doesn't show up in upower
> so it's not the fault of powerdevil .. but i wish i could see this :)
Have you tried UPower from git? My Logitech Trackball (using an USB 
transceiver) has recently been added and now happily reports its battery 
status.

> >   - Support for keyboard backlight brightness
> oddly,
?

> very nice; one thing that Marco noted was that it should only be animating
> when the popup is open (haven't checked if that's the case).
Oh, I need to take care of that? It doesn't check that right now.
 
> to me, the easing curve also seems a bit .. not right. after changing the
> InQuad to InCubic in BatteryItem.qml line 117 it feels a lot more like it is
> charging.
Is there some sort of "Run animation alternating" setting? ie. at the end of 
the animation it runs it reversed and forward and reversed? So I don't need 
those two sequential animations?

>  one visit to youtube can ruin your expectations ;)
Exactly. Can vary from 5 hours to 30 minutes with ease.

> we noticed these buttons were gone as well, and we're not sure they need to
> be there. i really have a hard time justifying those buttons being there:
> we have them in the application launcher (kickoff), we have them in
> krunner, we have plasmoids that provide just those buttons, etc. suspend is
> also linked to lid close on laptops by default.
Those where my thoughts too. But I have found myself a few times actually 
using them. Since they unfortunately don't appear in the Shutdown menu (ie. 
what you get when you right click the desktop and say "Leave", I don't use 
Kickoff). On the other hand I could as well just press Fn+F1 or close the lid. 

> so we'd vote for leaving them out.
Fine with me.

> we'd also vote for getting rid of the option to show the percentage overlay
> on the battery icon.
Hmm, I actually always miss that option on my Android phone. The icon is not 
accurate enough for me. It's not really pretty but given system tray 
limitations (only one icon with fixed size) there's not much improvement we 
could do there. Shrinking the battery icon horizontally and showing the 
percentage right of it (without the % symbol) using the same style the clock 
uses (ie. white with gradient, if possible with QML) would be cool, though.

> it could be shown on desktop when the plasmoid gets to
> a certain size, and instead of floating in the middle of the battery (which
> looks not so nice) it could perhaps appear in a small strip that overlays
> the battery across the full width at the very bottom?
Agreed.

Did you test the monitor's behavior on a desktop machine where there is no 
battery at all or only a battery from a mouse? I am not sure what to do there.
Averaging all the power supply batteries (eg. you have 2 batteries in your 
laptop) is fine for those. But averaging mouse and keyboard doesn't make 
sense. My idea was then to show the percentage of the lowest available 
battery, for example: Your keyboard is 90% and your mouse 30%, it shows 30%, 
so you're attention is drawn to the mouse. On the other hand, I wouldn't want 
to have a battery icon sitting in my tray on a desktop machine all the time. 
So maybe it should be always passive then? I have really no idea.

> the problem is in powerdevil?
It's actually broken in another way. With 4.11 PowerDevil backlight controls 
are moved out of PowerDevil core and so the PM dataengine talks to the 
backlight action instead of PowerDevil core. So the idea was, if it couldn't 
connect to the backlight action brightnessChanged signal - which doesn't exist 
if the action isn't supported - backlight controls are unavailable and thus 
the slider should be hidden.
There are two problems though:
 - Backlight controls are *always* claimed to be available by the PowerDevil 
UPower backend. It checks for various things but doesn't claim false if all 
checks fail. I fixed that in the branch too but I am not sure whether it has 
further consequences. It does then show an error message on every start 
telling you it cannot load the BrightnessControl action (which isn't loaded 
because it is telling the core to not be supported). There are numerous 
attempts to fix that message but none have been accepted/shipped yet. But that 
really needs to happen before this can go in.
 - QDBusConnection connect seems to always returns true, even if the object is 
not existing. So the checks that were build in there fail and always say that 
backlight controls are available. I worked around this by unclaiming support 
as soon as it tries to access the backlight (which it does on init()) and 
QDBusReply returns an "Object not found" error. This works, but really isn't 
elegant. And the current code also doesn't notice if, say, the backlight 
handling is not ready when the dataengine is initialized or it comes available 
later. So this really needs fixing on the Solid side.

> Switches are not to be used on Desktop. For 4.11 the QML switch component
> appears as a checkbox, so you really don't have a choice in the end.
Did you announce this somewhere? There's a few plasmoids (especially the 
printer manager) that use switches that would really be ruined if the switches 
were suddenly checkboxes.
 
> * consistency
The way Plasma looks compared to the rest of the applications is really all 
about consistency, right?
> * input device appropriateness
You can click them both :)
> * people use them poorly on desktop (just look at UIs that use them
> extensively on desktop and what the layouts end up looking like ..
Actually I find many Gnome settings dialogs really attractive.

But I won't start an argument about these switches here. I will use a checkbox 
there, or so, and that's it. I don't care as long as the option is there and 
works.

Thanks for your long response!