Review Request: Minor cleanups and improvements in the KineticScrolling

[Zack Rusin]

On Sunday 14 March 2010 14:00:41 Marco Martin wrote:
> On Sun, Mar 14, 2010 at 4:43 PM, Zack Rusin <zack at kde.org> wrote:
> > On Sunday 14 March 2010 08:53:10 Marco Martin wrote:
> >>  :/ About put things on kineticscrolling or scrollwidget, i know putting
> >>  everything in kinetikscrolling makes the code a bit more wonky, but i
> >>  would like it to be there to recycle it for WebView (and possibly other
> >>  wdgets someday too) as much effortless as possible.
> >
> > This approach just won't work very well. The main issue is that it only
> > works for single items with no children. If you put an item that has
> > child items on the thing you won't be able to filter the events making
> > the entire KineticScrolling class useless. From the ScrollWidget one can
> > do setFiltersChildEvents which deals with exactly this scenario.
> > This is going to be /especially/ visible on the webview since from Qt 4.7
> > when dealing with CSS animation QtWebKit uses child items to accelerate
> > it, so starting Qt 4.7 flicking on web pages will be impossible.
> >
> > Also to be honest I don't see the appeal of exporting code that really
> > belongs in a ScrollWidget to another class to make other widgets that
> > don't inherit use ScrollWidget act like ScrollWidgets. They simply should
> > use/be ScrollWidgets. Plus it makes Plasma a lot harder to understand
> > when objects from beyond the hierarchy always filter coming in events.
> 
> ok, i see.
> what i'm concerned now is that ScrollWidget and WebView would have
> copies of nearly the same implementation...
> Probably ScrollWidget should have been something more abstract than
> the "big widget scrolls into the smaller parent" that is now, case
> that would just be a subclass...
> but right now is a bit late for binary compatibility...

We don't have to make the WebView a ScrollWidget, we can just make it use it, 
i.e. instead of inheriting it, just make it a member in WebViewPrivate and 
simply stuff the real KGraphicsWebView inside it. This way we retain BC, we get 
the proper flicking on webviews with children (soon that'd be all of them) and 
we share the code. The same principle, meaning aggregation, should apply to 
every widget that for some reason can't inherit ScrollWidget but wants 
flicking.

Plus speaking long term we'll need to make scrollviews a bit better suited for 
mobile usage by e.g. instead of stuffing the native scrollbars (which look awful 
on mobile) in a gridlayout, make ScrollWidget overlay nicely looking scroll 
markers (addition to Plasma::Theme I guess) on top of the content. It would 
make sense if this was exported to a common widget instead of duplicated in 
every widget that scrolls and ScrollWidget is the obvious place for that, 
while KineticScrolling would never be able to do that.

Does that sound good, or do you see any problems with this approach?

z