<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/106670/">http://git.reviewboard.kde.org/r/106670/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 1st, 2012, 3:51 p.m., <b>Sebastian Kügler</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ship It!</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Umm, now with or without the remaining battery time thing? :)</pre>
<br />
<p>- Kai Uwe</p>
<br />
<p>On October 1st, 2012, 1:37 p.m., Kai Uwe Broulik wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Solid.</div>
<div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated Oct. 1, 2012, 1:37 p.m.</i></p>
<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 review addresses the following issues:
- Bug 261336 "Battery warnings are looking bad"
The icon was passed to KNotification::event as 20x20 pixmap. I moved the Icon code to the notifyrc so the system can choose the size accordingly.
Also, I used more specialized icons for the notifications rather than a generic dialog-warning.
I used dialog-warning for critical battery (we need a battery icon with an exclamation mark as the battery-low icon isn't really showing urgence) and battery-caution for low instead of battery-low
- Power Management notifications are now just "Power Management system" rather than "Notifications for KDE Power Management", and I shortened and paraphrased some of the notifications to be more easily recognizeable (and less text :P)
- I added a new function emitRichNotification (could not overload the emitNotification) which also sets a title because I think the generic "KDE Power Management System" title says nothing to the average user. So, eg. the Battery Low notification has "12% Remaining" as title. I wanted to add something like "12% Remaining (0:12)" but since we removed the remaining time from the plasmoid, it doesn't make sense to have it here.
- Removed "Profile changed" notification as there are no longer profiles and the notification is not used anywhere
- Removed "Warning battery" notification as it is not used anywhere (and removed from HAL backend)
- Added Notification for "Battery full" (Bug 261890)
Do we really need that "doingjob" notification? From what I can tell it is only used to show the "Screen is being locked" option and has no effect on anything else?</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;">Unplugged and plugged in my AC adaptor, notification worked and looks beautiful. Played around with battery critical and low settings and those notifications are also looking good now. Did not test if the battery full notification works (my notebook is at 10% right now :P). Also could not test the battery service/broken ones.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=261336">261336</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=261890">261890</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>powerdevil/daemon/actions/bundled/suspendsession.cpp <span style="color: grey">(28dc2d6)</span></li>
<li>powerdevil/daemon/backends/hal/powerdevilhalbackend.h <span style="color: grey">(4112bdd)</span></li>
<li>powerdevil/daemon/backends/hal/powerdevilhalbackend.cpp <span style="color: grey">(18b38be)</span></li>
<li>powerdevil/daemon/powerdevilcore.h <span style="color: grey">(fd75311)</span></li>
<li>powerdevil/daemon/powerdevilcore.cpp <span style="color: grey">(fe2c5b0)</span></li>
<li>powerdevil/powerdevil.notifyrc <span style="color: grey">(7bc5312)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106670/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/106670/s/749/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/10/01/pmredesign_400x100.png" style="border: 1px black solid;" alt="Right before, Left after" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>