Review Request: Spinner widget

[Fredrik Höglund]

On Monday 03 November 2008 15:25, Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.vidsolbach.de/r/253/
> -----------------------------------------------------------
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This is the spinner widget we talked about in the last irc meeting, would like to make in 4.2..
> it's api is at the bare minimum (no api at all basically :p) could have things like strt/stop or speed settings, but not sure it's necessary...
> i'm not really sure about the folderview part, it's done rather quickly as a test :)

One thing I don't like about this code is that it rotates the painter before it
draws the FrameSvg. QPainter will apply the transformation to the pixmap
in software, meaning it will convert it to an image, rotate it, convert it back
to a pixmap, and then drawn it. This makes showing the busy animation
expensive in itself. In my opinion the Spinner widget needs to keep a cached
copy of each frame in the animation.

It also looks to me like the code assumes that there is a shadow element,
but I'm not sure it should be a requirement for the theme to provide one.
The offset for the shadow element is also hardcoded.

The nativeWidget() function doesn't make much sense to me, since the spinner
is a native QGraphicsWidget, and will never be based on a QWidget.

I also don't think it makes much sense to have an empty setStyleSheet()
function, since it can be added at any time in the future without breaking
binary compatibility.

I also think Applet should have a setShowBusyIndicator(bool on) that takes
care of showing and hiding the spinner, so the code for it won't have to be
duplicated in each applet that needs to use it.

Other than that the code looks good :)

Fredrik