-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3368/#review4626
-----------------------------------------------------------



/trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp
<http://reviewboard.kde.org/r/3368/#comment4130>

    Why was this needed? If we'll filter all child events then it will be 
possible to flick the scrollbars. The check should probably either stay as it 
is or be changed to
    if (!d->widget.data() || i == d->verticalScrollBar || i == 
d->horizontalScrollBar)
        return false;



/trunk/KDE/kdelibs/plasma/widgets/webview.cpp
<http://reviewboard.kde.org/r/3368/#comment4131>

    You're the biggest event filter fan in the world :) Why do we need that?



/trunk/KDE/kdelibs/plasma/widgets/webview.cpp
<http://reviewboard.kde.org/r/3368/#comment4132>

    I'd probably just create a vertical or horizontal layout, put the 
d->webView in it and set the layout on the d->scrollingWidget, this way you 
don't have to worry about setting d->webView geometry. But I'm not 100% sure if 
we need the scrollWidget at all, wouldn't it be easier to just set 
ScrollWidget::setWidget(d->webView) directly?


I think ideally we shouldn't need a dummy scrollingWidget or any event filters. 
If it doesn't work then, unless I'm missing something here, there's probably a 
bug/missing feature in the Plasma::ScrollWidget. I could look at it tomorrow 
night.
Either way we should probably test it with the new QtWebKit with the 
AcceleratedCompositing option on to make sure it works at all.

- Zack


On 2010-03-23 14:54:52, Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3368/
> -----------------------------------------------------------
> 
> (Updated 2010-03-23 14:54:52)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> after the last modifications by Zack on Scrollwidget, now flicking is managed 
> by ScrollWidget itself, because it's the only way it can safely filter child 
> events..
> and since html elements will be done by qraphicsitems, it will probably be 
> necessary to do the same thing in WebView.
> this is an early attempt, but doesn't seem to work very well. there is a 
> qgraphicswidget with geometry syncronized with the page contents so flicking 
> it makes the page scroll around.
> 
> it's not always correct since is not possible to watch for a change in the 
> zoom level.
> things still missing: disabling everything with setDragToScroll and support 
> for multitouch events.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1106404 
>   /trunk/KDE/kdelibs/plasma/widgets/webview.h 1106402 
>   /trunk/KDE/kdelibs/plasma/widgets/webview.cpp 1106402 
> 
> Diff: http://reviewboard.kde.org/r/3368/diff
> 
> 
> Testing
> -------
> 
> it seems significantly slower and quite buggy, but i don't see much more ways 
> to do it.
> 
> another way would be generalizing scrollwidget to make it possible to set 
> properties in the webview instead of moving the clipped widget into the 
> parent, but it would make it even more complex.
> 
> another way could be moving everything back in kineticscroll again, make it 
> visible to the classes that are using it (so won't be possible anymore to use 
> it outside of libplasma) and forward by hand to it sceneeventfilter.
> 
> the latter is at the moment the one i'm leaning towards.
> 
> 
> Thanks,
> 
> Marco
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to