Review Request: Minor cleanups and improvements in the KineticScrolling

[Marco Martin]

On Sun, Mar 14, 2010 at 7:37 PM, Zack Rusin <zack at kde.org> wrote:
> 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.

yes, in this case including instead of inheriting is the only thing
that cqn be done, i'm just thinking that maybe ScrollView is not
generic enough? we could use the inner scrolling widget invisible that
filters the input events and forwards them to the kgraphicswebview, i
wonder if it isn't a bit gross?

> 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

on a more broad scope, what i would like it is a way for all our
widget to figure out where they are (being based on a config value or
hardware autodetection i still don't know)
so if the main input device is a touchscreen most of them would behave
a bit differently: the scrollview would have passive scrollmarkers
that maybe appear only when scrolling, not interactive (while if there
is a mouse a traditional scrollbar makes sense)
other widgets, like buttons would refuse hover events, since the
concept of mousehover becomes a bit meaningless on a touch.

> 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.

yes, would make perfectly sense.

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

to me sounds good, as i said, if using scrollview to filter and
forward events to the webview is a clean way enough, i'm happy with
that.

Cheers,
Marco Martin