Plasma::ToolTipManager API review

[Kevin Ottens]

Hello,

I took some time tonight to take a look at Plasma::ToolTipManager API since it 
was reasonably self contained, and I could work on it without having to wait 
for some on-going effort.

Surprisingly (or not) my conclusions on this one will be radical. I think 
quite a few methods can be removed (when I say "removed" in this mail, one has 
to understand "removed from public API", might still make sense to use 
privately).

* ToolTipManager (QObject *parent=0)
* ~ToolTipManager ()

ToolTipManager being a singleton, I'd say they probably shouldn't be public. 
Even better, to get closer from QToolTip, we should probably have no ctor, 
dtor in this class and only static methods.

* void   showToolTip (QGraphicsWidget *widget)

Never used, remove until really needed?

* bool   isWidgetToolTipDisplayed (QGraphicsWidget *widget)

Used only once (in digital-clock). Probably should be 
isToolTipVisible(widget)?

* void   delayedHideToolTip ()

Never used, remove until really needed?

* void   hideToolTip (QGraphicsWidget *widget)

Never used, remove until really needed?

* void   registerWidget (QGraphicsWidget *widget)

Used several times, but always followed by a setToolTipContent() call... first 
line of setToolTipContent() being registerWidget(). Very good candidate for 
removal IMO.

* void   unregisterWidget (QGraphicsWidget *widget)

Used only once by a good citizen. Otherwise widgets are never unregistered, 
even if the tooltip content gets empty. Just like registerWidget() is called 
by setToolTipContent(), this method should probably call unregisterWidget() 
for empty content. And then candidate for removal.

* void   setToolTipContent (QGraphicsWidget *widget, const ToolTipContent 
&data)

This one is very much used. Should stay. I'm not sure what to do about 
ToolTipContent though. Makes sense internally, but maybe in the public API you 
want to pass the parameters in the method. Not having to construct an object, 
set the members, then make the call.

* void   clearToolTipContent (QGraphicsWidget *widget)

IMO useless, and used only once. Use setToolTipContent() with empty content 
instead. Would allow to be in sync with QToolTip behavior wise.

* bool   widgetHasToolTip (QGraphicsWidget *widget) const 

Never used, remove until really needed?

* void   setToolTipActivated (QGraphicsWidget *widget, bool enable)

Used only in Kickoff. It really looks like an implementation detail which 
creeped out in public interface. I'd expect code to remove the tooltip then 
set it again. This way we get only one mecanism for the task.

  bool   isToolTipActivated (QGraphicsWidget *widget)

Never used, remove until really needed?

With this first round of comments, my ideal ToolTipManager API would become:
--------------------------
class ToolTipManager
{
    static bool isToolTipVisible(QGraphicsWidget *widget);
    static void setToolTipContent(QGraphicsWidget *widget,
                                  const QString &mainText,
                                  const QString &subText = QString(),
                                  const QPixmap &image = QPixmap(),
                                  WId windowToPreview = 0);
    static void setToolTipContent(QGraphicsWidget *widget,
                                  const QString &mainText,
                                  const QPixmap &image,
                                  WId windowToPreview = 0);
};
--------------------------

I'm not fully happy with it yet, but I'd like to get feedback to see if it's 
at least a good start. Improvements missing are in my opinion with removing 
the "ToolTip" part in the method names. Probably also ditching the "Manager" 
part in the class name would be an idea.

Regards.
-- 
Kévin 'ervin' Ottens, http://ervin.ipsquad.net
"Ni le maître sans disciple, Ni le disciple sans maître,
Ne font reculer l'ignorance."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20081021/c4292c08/attachment.sig