[Kde-hardware-devel] Review Request: Fix some visual and user experience glitches in Power Management notifications

[Kai Uwe Broulik]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106670/
-----------------------------------------------------------

(Updated Oct. 1, 2012, 1:37 p.m.)


Review request for Solid.


Changes
-------

Sorry, screwed up my diff :)


Description
-------

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?


This addresses bugs 261336 and 261890.
    http://bugs.kde.org/show_bug.cgi?id=261336
    http://bugs.kde.org/show_bug.cgi?id=261890


Diffs (updated)
-----

  powerdevil/daemon/actions/bundled/suspendsession.cpp 28dc2d6 
  powerdevil/daemon/backends/hal/powerdevilhalbackend.h 4112bdd 
  powerdevil/daemon/backends/hal/powerdevilhalbackend.cpp 18b38be 
  powerdevil/daemon/powerdevilcore.h fd75311 
  powerdevil/daemon/powerdevilcore.cpp fe2c5b0 
  powerdevil/powerdevil.notifyrc 7bc5312 

Diff: http://git.reviewboard.kde.org/r/106670/diff/


Testing
-------

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.


Screenshots
-----------

Right before, Left after
  http://git.reviewboard.kde.org/r/106670/s/749/


Thanks,

Kai Uwe Broulik

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20121001/0ac592ce/attachment.html>