Re: Patch to add libnotify support to nm-applet

[Rodney Dawes <dobey novell com>]

On Wed, 2006-02-15 at 17:46 +0100, Michael Biebl wrote:
> Rodney Dawes wrote:
> > Ah. Looks like g-p-m could use a little updating then. If you pass in
> > the last argument to notify_notification_new (), then the code will just
> > automatically get the info needed for the x/y hints from that widget.
> > 
> > Here's an updated patch that does this, rather than adding all the extra
> > duplicate code into nm itself.
> 
> Yes, this looks much better now! One last issue: Could you store the
> reference to the NotifyNotification somewhere so that you have to create
> it only once and not on each event. I would also suggest to move
> notify_init somewhere into the constructor of NMWirelessApplet so that
> it has to be called only on the initial setup.

As far as I know, each notification needs to be a separate
NotifyNotification object. We need to create it newly every time,
because the title, message, and icon change. And the icon may actually
move as well, as other items in the system tray appear and disappear.
Is there any particular reason you'd want to keep a singleton of it
around? There aren't any performance or memory issues with the current
patch, are there? It just seems unnecessary to me to do that.

I did have notify_init being called from the nmwa_init method, but this
was causing nm-applet to lock up waiting for a response from dbus. Is
there a better construction method to put it in?

Also, regarding the complaint about the icon scaling, I committed a
change to icon-naming-utils yesterday, to create symlinks for some of
the nm-* icon names, so that device icons in the theme will just get
used. This means that for Tango and gnome-icon-theme HEAD, things should
mostly work ok. I'll also look into fixing the notification-daemon to
center the icon in a fixed size icon widget, rather than scaling it up
by default.

Thanks.

-- dobey