Review Request: Some improments to the system tray notifications.

[Rob Scheepmaker]

On Wednesday 05 November 2008 15:18:02 Jason Stubbs wrote:
> Rob Scheepmaker wrote:
> > I noticed a couple of issues in the way system tray notifications are
> > currently displayed:
>
> A little bit cheeky seeing as most of these issues were comments posted
> by others on your blog. ;)

you caught me ;)

> > * The icon us quite large and takes up a lot of space.
> > Solution: remove the icon from the notification itself and use it as the
> > icon of the extenderitem.
>
> I guess you could call this a KIcon issue, but saving it to a config
> file does not work all the time. As notifications are currently being
> constructed from a QIcon (and thus has no associated name) notifications
> will have an "unknown type" icon after plasma is restarted. This is an
> issue with extenders themselves rather than this patch though.

I have changed extenders so that the setIcon(QString) call will also save the 
icon to the config. But you're right that this doesn't seem to work in all 
cases. I'll investigate. (I currently don't have the source nearby, but I 
wonder why notifications are being constructed from a QIcon? The notification 
spec provides a name right?)

> > * The fixed size of the notifications make small messages take too much
> > space, but also isn't enough to display larger messages. Solution: make
> > the size hint depend on the size of the message. Make sure each
> > notification is always displayed entirely.
>
> After dragging a notification away, the resulting applet appears to have
> a zero size. The extender item then seems to be drawing out of its
> bounds. Not sure exactly what's going on.

Yeah, that is an issue in extenders that is there for quite some time now, and 
of which I have got no idea on how to solve. The applet doesn't honor the 
sizehints of the layout it  contains. Naughty layouts.....

> > While making these changes the current split in two classes became a bit
> > awkward to work with, so I also merged these two classes into one QGW,
> > and cleaned up a bit. I think that with this patch notifications are more
> > usable, and the code is a bit cleaner, but I would very much like to hear
> > your feedback.
>
> This is much better. I never understood why they were separate classes
> in the first place...
>
> There was also a change in behaviour that I'm not sure was intentional.
> When a notification that has buttons has been detached and then a button
> is clicked, an empty applet is left with the message "no notifications"
> or some such. Previously, the buttons would have just disappeared so as
> to respect the stance that an application can't close a detached
> notification.

Ah, this is probably an easy to solve bug, which I will fix later today.

> You might want to check the behaviour of for each of the various
> combinations of with/without buttons, with/without a timeout and
> with/without being detached.

Agreed.