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

[Commit Hook]

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


This review has been submitted with commit 24249065e4f3b021a258967f859600e0e5f398e0 by Kai Uwe Broulik to branch master.

- Commit Hook


On Oct. 1, 2012, 4:32 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106670/
> -----------------------------------------------------------
> 
> (Updated Oct. 1, 2012, 4:32 p.m.)
> 
> 
> Review request for Solid.
> 
> 
> 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
> -----
> 
>   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/f9543f2f/attachment.html>