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."
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
