Feedback on KNotificationItem

[Marco Martin]

On Tuesday 15 September 2009, Michael Pyne wrote:
> Hi all,
>
> So I took the opportunity to try porting JuK's system tray icon to
> KNotificationItem.  Here's some feedback:
>
> * First off, I like the idea as I've always thought that embedded widgets
> was probably the wrong way to be going about notification item entries in
> the first place, and this has been working fine for me so far, which is
> good. * The API documentation could use improvement.  I've fixed up some of
> the errors that I saw but here's some specifics:
> - I had to read the source to be sure that I could remove an overlay icon
> by doing setOverlayIconByName(QString()) for instance.
> - What is "bool active" in activateRequested for?
> - There's a lot of mixing of QIcon and "pixmap" (more on that in a bit)
> - What does eventFilter do for an icon with no widget?  It's protected, not
> private, so it should still be documented a bit I think.
> - It may be prudent to give an example of a "secondary" activate action, as
> mentioned in the source code (middle-click)
> - setAttentionIconByName mentions a pixmap paramete
> - The detailed description mentioned the Dbus spec.  Is the spec available
> online to be linked to?
> - Several signals are referenced in the APIDOX but not defined (presumably
> they're private?)  For instance newToolTip() and newStatus().
> - What are the title and subtitle for in tool tips?  Can they be HTML/rich
> text? (It's easy enough to test, but it would be nice to be clear in the
> documentation).
> - showMessage() is completely undocumented from what I can tell.
> * "These two will die..." in knotificationitem.h.  If it's true that the

ah, was almost forgetting...
i thnk those two methods could stay.
...opinions? :)

> setAttentionMovie overloads for the QVectors will not make it I'd recommend
> removing it now (or BIC Monday) before the release so we don't forget.
> * Is it not possible to use const QMovie * instead of QMovie * in
> setAttentionMovie?  I see no reason a read-only movie can't be shown in the
> system tray.
> * setToolTip(const QPixmap&, const QString &, const QString &) is declared
> in the .h but never implemented in the .cpp :(
>
> Of course that's all minor stuff that didn't interfere in the port (except
> for the tooltips, but I'll make it work).  I was pleasantly surprised to
> see that both scroll wheel and middle-mouse click detection were present
> for instance.
>
> One thing that is hampering the port is that JuK has a "track announcement
> popup" that pops up next to the system tray icon.  Without the embeddable
> widget, JuK doesn't know where the icon is.
>
> So, can I use KNotify for this (the popup would need to support graphics,
> rich text, and at least two buttons)?  Would KNotification show up in the
> right spot?
>
> If this isn't the case, what would be the best way to get access to that
> information?  Preferably we wouldn't have to re-tool the spec at this late
> stage.
>
> Thanks for your time,
>
> Regards,
>  - Michael Pyne


-- 
Marco Martin