<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/110431/">http://git.reviewboard.kde.org/r/110431/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma and Viranch Mehta.</div>
<div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated May 15, 2013, 3:04 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Remove "Show the state for each battery present".
Now, when it is constrained (ie. panel or systray) you get one icon which shows the power supply average.
When placed on the desktop it shows an icon for each battery present next to each other.
i.e. the code is now like if the option was always enabled.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This patch improves the situation with multiple batteries in the battery monitor by:
- Showing the device name instead of just "Battery", ie. your Mouse, if it has a name, will say "Your awesome bluetooth mouse"
- Not adding non-powersupply-batteries (eg. mice and others) to the total percentage
Non-power-supply don't get the "(charging)" suffix as, according to the spec, the chargeState property is not available for such devices and they're always considered discharging, eliminating the usefulness of this label.
I removed the "Battery 1", "Battery 2" naming from the Popup since it didn't work in the first place (only showed "Battery" here) and I wanted to make it as smart as the tooltip, which, if there is only one battery without a name, it says "Battery" instead of "Battery 1" and if there are, it doesn't just show "Battery $index" but "Battery 1", "Battery 2", but I didn't know how to do this when it's inside a model. Can I have a variable there in QML, like:
Repeater {
…
batteryNumer: model["Name"] ? batteryNumber++ : batteryNumber
}
so I can skip the named ones and don't end up with Battery 1, My mouse battery, Battery 3?
I'm also not sure about the "Show battery percentage for each individual battery" setting. Not showing non-powersupply-batteries if unchecked is certainly not an option, but if you just collapse powersupply batteries, you will still end up with more than one battery in the popup, despite its name. So maybe we should drop it altogether and always show all batteries in the popup but only show one icon (that is the sum of all powersupply batteries) on the icon? It shows multiple icons (ie. one for each battery) when placed on the desktop anyway …</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I only have a notebook with a bluetooth mouse, so I couldn't test how it behaves on a desktop machine that doesn't have an internal battery but just a mouse battery attached, or how it behaves with more than one primary battery. More testing is needed.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>plasma/generic/applets/batterymonitor/contents/code/logic.js <span style="color: grey">(974694a)</span></li>
<li>plasma/generic/applets/batterymonitor/contents/config/main.xml <span style="color: grey">(fc31b3e)</span></li>
<li>plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml <span style="color: grey">(3ffb15f)</span></li>
<li>plasma/generic/applets/batterymonitor/contents/ui/batterymonitor.qml <span style="color: grey">(c69c3a5)</span></li>
<li>plasma/generic/applets/batterymonitor/contents/ui/config.ui <span style="color: grey">(3df38e2)</span></li>
<li>plasma/generic/dataengines/powermanagement/powermanagementengine.h <span style="color: grey">(1809c02)</span></li>
<li>plasma/generic/dataengines/powermanagement/powermanagementengine.cpp <span style="color: grey">(7882f75)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110431/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/05/14/bluetoothmouse4.png">Popup Dialog</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>