Review Request: PATCH: Configuration Dialog for Turning off Notifications in the Panel

[guitarist198 at yahoo.com]

> On 2009-01-17 10:17:37, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/applet.cpp, line 413
> > <http://reviewboard.vidsolbach.de/r/331/diff/3/?file=1777#file1777line413>
> >
> >     missing space: } else

I have added the parents to both checkboxes and added the missing spaces to both else statements.  I also added a description to the top of the configuration dialog that reads "Select the types of application feedback that should be integrated with the system tray:"

This text may need to be updated in a subsequent patch to reflect the future discussions on this topic.  I have also added a new screenshot which shows how this looks in the dialog.

And as a final note:  I applied for an svn account last night.  So either I will commit this patch when I receive the account or someone else can do so if they would like this patch in sooner.

Thank you Aaron for taking the time to review the patch.


- distortedlogic


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.vidsolbach.de/r/331/#review318
-----------------------------------------------------------


On 2009-01-17 11:14:51, distortedlogic wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.vidsolbach.de/r/331/
> -----------------------------------------------------------
> 
> (Updated 2009-01-17 11:14:51)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch allows one to turn on and off jobs and notifications in the panel via the systemtray configuration dialog.  This is in response to Aaron Seigo's blog entry: http://aseigo.blogspot.com/2009/01/todays-tip-turning-off-fancy-schmancy.html
> 
> Within manager.cpp, functions to unregister the notification and job protocols was created, as well as a function to unset a protocol (disconnect it).  These were based on the already present functions to register and setup protocols.
> 
> Within applet.cpp, a new page was added to the configuration dialog called "Notifications."  This page includes the two check boxes that allow one to turn on and turn off notifications.  A possible area where that may need to be fixed is the strings for the check boxes.  Presently they are "Show Jobs" and "Show Notifications."
> 
> Thank you for your time and review.
> 
> Update:  I have attached a screenshot of what the configuration dialog looks like and I also made a change to the patch:  I added the i18n function around the check box label strings which I had forgotten the first time around.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/applet.cpp
>   /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/core/manager.h
>   /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/core/manager.cpp
> 
> Diff: http://reviewboard.vidsolbach.de/r/331/diff
> 
> 
> Testing
> -------
> 
> Testing of suppression of job progress.
> Testing of suppression of notifications.
> Testing of allowing job progress.
> Testing of allowing notifications. 
> 
> 
> Screenshots
> -----------
> 
> Configuration Dialog
>   http://reviewboard.vidsolbach.de/r/331/s/101/
> Configuration Dialog With Description Text
>   http://reviewboard.vidsolbach.de/r/331/s/105/
> 
> 
> Thanks,
> 
> distortedlogic
> 
>