Review Request: port tray icon to knotificationitem

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

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


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

_______________________________________________
Konversation-devel mailing list
Konversation-devel@...
https://mail.kde.org/mailman/listinfo/konversation-devel

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

(Updated 2009-10-24 18:38:49.268878)


Review request for konversation.


Changes
-------

update description


Summary (updated)
-------

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

_______________________________________________
Konversation-devel mailing list
Konversation-devel@...
https://mail.kde.org/mailman/listinfo/konversation-devel

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



trunk/extragear/network/konversation/CMakeLists.txt
<http://reviewboard.kde.org/r/1965/#comment2161>

    Are these commented out? I'm not that great with cmake but i thought they were important. :-/



trunk/extragear/network/konversation/src/mainwindow.cpp
<http://reviewboard.kde.org/r/1965/#comment2162>

    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.



trunk/extragear/network/konversation/src/viewer/trayiconknotificationitem.cpp
<http://reviewboard.kde.org/r/1965/#comment2163>

    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)"



trunk/extragear/network/konversation/src/viewer/trayiconknotificationitem.cpp
<http://reviewboard.kde.org/r/1965/#comment2164>

    As I mentioned in an earlier comment, this doesn't work, and can be cut out.



trunk/extragear/network/konversation/src/viewer/trayiconksystemtray.cpp
<http://reviewboard.kde.org/r/1965/#comment2165>

    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


- Travis


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
>
>

_______________________________________________
Konversation-devel mailing list
Konversation-devel@...
https://mail.kde.org/mailman/listinfo/konversation-devel

> 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
>
>

_______________________________________________
Konversation-devel mailing list
Konversation-devel@...
https://mail.kde.org/mailman/listinfo/konversation-devel

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

(Updated 2009-10-24 22:03:32.376269)


Review request for konversation.


Changes
-------

updated my patch to fix the stuff that word wanted me to fix :)

there's one known regression with my patch:
if I disable the tray icon and restart konversation (thus on restart no m_trayIcon is created) there are config entries missing in the settings menu


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 (updated)
-----

  trunk/extragear/network/konversation/CMakeLists.txt 1039902
  trunk/extragear/network/konversation/cmake/modules/FindLibKNotificationItem-1.cmake PRE-CREATION
  trunk/extragear/network/konversation/config-konversation.h.cmake 1039902
  trunk/extragear/network/konversation/src/CMakeLists.txt 1039902
  trunk/extragear/network/konversation/src/mainwindow.cpp 1039902
  trunk/extragear/network/konversation/src/viewer/trayicon.h 1039902
  trunk/extragear/network/konversation/src/viewer/trayicon.cpp 1039902
  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

_______________________________________________
Konversation-devel mailing list
Konversation-devel@...
https://mail.kde.org/mailman/listinfo/konversation-devel

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

(Updated 2009-10-31 00:24:12.346443)


Review request for konversation.


Changes
-------

final fixes


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 (updated)
-----

  trunk/extragear/network/konversation/CMakeLists.txt 1042888
  trunk/extragear/network/konversation/cmake/modules/FindLibKNotificationItem-1.cmake PRE-CREATION
  trunk/extragear/network/konversation/config-konversation.h.cmake 1042888
  trunk/extragear/network/konversation/src/CMakeLists.txt 1042888
  trunk/extragear/network/konversation/src/mainwindow.cpp 1042888
  trunk/extragear/network/konversation/src/notificationhandler.cpp 1042888
  trunk/extragear/network/konversation/src/viewer/trayicon.h 1042888
  trunk/extragear/network/konversation/src/viewer/trayicon.cpp 1042888
  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

_______________________________________________
Konversation-devel mailing list
Konversation-devel@...
https://mail.kde.org/mailman/listinfo/konversation-devel