[Konversation-devel] Review Request: port tray icon to knotificationitem

[darklight.xdarklight at googlemail.com]

> On 2009-10-24 21:07:28, Travis McHenry wrote:
> > trunk/extragear/network/konversation/CMakeLists.txt, line 41
> > <http://reviewboard.kde.org/r/1965/diff/1/?file=13283#file13283line41>
> >
> >     Are these commented out? I'm not that great with cmake but i thought they were important. :-/

oops. you're right - they should not be commented out


> On 2009-10-24 21:07:28, Travis McHenry wrote:
> > trunk/extragear/network/konversation/src/viewer/trayiconknotificationitem.cpp, line 35
> > <http://reviewboard.kde.org/r/1965/diff/1/?file=13289#file13289line35>
> >
> >     We no longer control blinking in application, this was discussed at length with the plasma people and the conclusion was reached that this should be controlled by the systray itself (even though the option isn't available yet). Before 1.3 we'll decide how to leave the option in for the old style and explain it's absence in the new, but for now just change this if-else to a "setStatus(NeedsAttention)"

I'll do, thanks


> On 2009-10-24 21:07:28, Travis McHenry wrote:
> > trunk/extragear/network/konversation/src/viewer/trayiconknotificationitem.cpp, line 50
> > <http://reviewboard.kde.org/r/1965/diff/1/?file=13289#file13289line50>
> >
> >     As I mentioned in an earlier comment, this doesn't work, and can be cut out.

it does work but I'll remove it (due to the reason you mentioned earlier) ;)


> On 2009-10-24 21:07:28, Travis McHenry wrote:
> > trunk/extragear/network/konversation/src/mainwindow.cpp, line 668
> > <http://reviewboard.kde.org/r/1965/diff/1/?file=13286#file13286line668>
> >
> >     I briefly told you on IRC, but right here we should not be hiding the tray icon, or showing the tray icon, we should be constructing or destructing the tray icon objects. IOW we shouldn't create the tray icon object at run time (especially with the old specification as this increases startup time by a reported .5 seconds). Also, I'm pretty sure that "Passive" doesn't hide the icon, it just makes it one of the icons that gets hidden with the arrow button (which doesn't coincide with what the preference is) so it needs to be destroyed.

okay, I'll work on that asap


> On 2009-10-24 21:07:28, Travis McHenry wrote:
> > trunk/extragear/network/konversation/src/viewer/trayiconksystemtray.cpp, line 84
> > <http://reviewboard.kde.org/r/1965/diff/1/?file=13290#file13290line84>
> >
> >     this works, but isn't the greatest way to do it if we need to support the old style, constructing and destructing at preference change is the best option imo


- xdarklight


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


On 2009-10-24 18:38:49, xdarklight wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1965/
> -----------------------------------------------------------
> 
> (Updated 2009-10-24 18:38:49)
> 
> 
> Review request for konversation.
> 
> 
> Summary
> -------
> 
> this ports the tray icon to knotificationitem
> 
> if knotificationitem is not available (on build time) the old implementation (using KSystemTray) will automatically be used as fallback
> -> this means konversation will still compile on every system
> 
> note: Sho and I had problems when applying the patch to our svn trees
> it seems that this was some bug in 'patch' (the application ;))
> make sure that after applying the patch the content of trayiconknotificationitem.cpp and trayiconksystemtray.cpp is not multiple times in them.
> 
> all items from word's list are done - except #4 (which should be a seperate patch):
> http://pastebin.ca/1613175
> 
> 
> Diffs
> -----
> 
>   trunk/extragear/network/konversation/CMakeLists.txt 1039836 
>   trunk/extragear/network/konversation/config-konversation.h.cmake 1039836 
>   trunk/extragear/network/konversation/src/CMakeLists.txt 1039836 
>   trunk/extragear/network/konversation/src/mainwindow.cpp 1039836 
>   trunk/extragear/network/konversation/src/viewer/trayicon.h 1039836 
>   trunk/extragear/network/konversation/src/viewer/trayicon.cpp 1039836 
>   trunk/extragear/network/konversation/src/viewer/trayiconknotificationitem.cpp PRE-CREATION 
>   trunk/extragear/network/konversation/src/viewer/trayiconksystemtray.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/1965/diff
> 
> 
> Testing
> -------
> 
> I've been using this patch for some days now ;)
> 
> 
> Thanks,
> 
> xdarklight
> 
>