extender api review round 3

[Rob Scheepmaker]

On Sunday 03 August 2008 20:10:59 Kevin Ottens wrote:
> setIsPopup => setPopup (as already pointed), and I still don't like the
> "setPopup" just like Aaron (hence why I proposed a "floating" property
> instead, but Aaron didn't like that name either).

I've removed the function entirely, I don't think it's needed.

> >         virtual void saveState();
>
> Shouldn't this be protected?

Yeah, saveState should be protected.

> >     protected:
> >         virtual void itemAddedEvent(ExtenderItem *item, const QPointF
> > &pos);
> >         virtual void itemRemovedEvent(ExtenderItem *item);
> >         virtual void itemHoverMoveEvent(ExtenderItem *item, const QPointF
> > &pos);
> >         virtual void itemHoverLeaveEvent(ExtenderItem *item);
>
> Where is itemHoverEnterEvent()?

In trunk/ ;)

> >
> >             KConfigGroup config();
>
> const missing (IIRC already pointed by Rich).

fixed :)

> >             void setAutoExpireDelay(uint time = 0);
>
> Please no default here.

agreed.
>
> >             bool hasAutoExpireDelay() const;
>
> Why not simply uint autoExpireDelay() const; ? At least that makes it
> symmetric, and you can get more information out of the class.

done in trunk/ :)

> > class PLASMA_EXPORT Applet : public QGraphicsWidget
> > {
> >     public:
> >         virtual void initExtenderItem(ExtenderItem *item);
>
> Again, I'd try to fit this one somewhere else. Maybe Extender?

I really would like to avoid having to subclass Extender. The only situation where you will have to 
implement this class now, is if you wish to change the presentation of the extenderitems from the 
default verticle layout one, to something fancier.

> >         Extender *extender() const;
>
> Again, I wouldn't keep this one.

> The idea here being that Extender is kind of an add-on decorator to Applet,
> so to make that clear API wise we should really avoid any Applet->Extender
> dependency in the public API.

Well, will need this anyway in private implementation, so even when the extender is somewhere outside of 
the applet (say in the topleft quadrant as part of a popup), you can add items to the extender by just 
dropping them on the applet. And actually, an applet will also always need to keep a pointer around to 
do stuff with their extender so while not entirely necesarry, I don't think this really is api bloat.

> Still open question on my side:
> What would be the use of the new provided API? Before extenders, we had to
> inherit Applet to make a new applet. With the provided API, I have this
> feeling that to get anything useful you'd have to inherit from Applet and
> Extender. Am I right? Any chance we could spare the inheritance from
> Extender, at least for the most common cases?

No, in normal cases you'll never need to subclass extender. You'll just instantiate an extender, and add 
extenderitems to it... extender takes care of the layouting and bookkeeping of these items. Only when 
you'd like to do different layouting you'll need to implement a new extender.