Review Request: Move from KTabWidget to KTabBar tab switching by mouse wheel events.

[Christoph Feck]

> On 2010-08-31 16:23:08, Christoph Feck wrote:
> > Sorry, that's even worse. Now it might skip tabs because on each wheel event both the wheelDelta() as well as your new code is executed. Users will be annoyed.
> > 
> > I just looked at lxr.kde org, and for example [1] uses wheelDelta() signal to implement the tab switching on wheel.
> > 
> > We have three options:
> > 
> > A) Do not touch it: We respect the initial envisioned API that applications listen to that signal and use it for whatever they see fit. Not that I like that signal-based API, but just breaking it without looking at the consequences is short sighted. It probably was added so that developers don't need to put all that QT_NO_WHEELEVENT etc. stuff into their code.
> > 
> > B) Your initial approach: We silently discard that signal-based API by simply never emitting the signal and force developers to reimplement wheelEvent() instead when they want custom behavior. Only developers who used the signal for purposes other than tab switching will have to change their code.
> > 
> > C) Add a property to KTabBar that switches between A and B, with A being the default (and mention that A is deprecated, i.e. developers should set it to B, because that will be the only option in KDE 5).
> > 
> > Comments please.
> > 
> > [1] http://lxr.kde.org/source/extragear/sdk/kdevplatform-git/sublime/container.cpp#181
> 
> Thomas Lübking wrote:
>     Personally "don't touch it" - iff touch it, maybe also reimplement KTabBar::connectNotify( "wheelDelta" ) & KTabBar::disconnectNotify( "wheelDelta" ), keep a counter and as long as it's not "0" do nothing internal on wheeling? =\
> 
> David Palacio wrote:
>     I am aware there are still other applications that use KTabBar directly and too have to reimplement this basic behaviour. Even KTabWidget does it wrong when switching to disabled tabs. My proposal requires to fix applications, but then all would share the same tab switching code. Sure, certain kdelibs/app versions might misbehave but, this is the ideal solution. With enough time, it will all work fine.
>     
>     A) That, indeed, is a solution. Rekonq and many other apps just have to reimplement this code. It's not a good solution imo, as KTabBar would be less capable than QTabBar is.
>     
>     B) This is the less painful fix, for applications. But then the KTabBar would be lying about emitting the wheelDelta signal. I'm fine with this.
>     
>     C) setWheelScrollingEnabled(bool) would be fine?
>     
>     D) Find affected applications and apply necessary changes. Old versions of certain applications outside the Software Compilation would switch twice when running with newer kdelibs.
>     
>     @Thomas: disconnectNotify implies connectNotify and, when the connection  is made, you can not really infer what the application developer really wants. So, KTabBar::connectNotify( "wheelDelta" ) should imply setWheelScrollingEnabled(false).
> 
> Christoph Feck wrote:
>     E) Implement the "wrapping" in Qt, and remove all "wheel on tabs" related-code from KDE and KDE applications. THAT would be the ideal solution :)
>     
>     Sorry again, but we cannot do D, we just cannot break "Old versions of certain applications outside the Software Compilation [that] would switch twice when running with newer kdelibs."
>     
>     Actually I like Thomas' suggestion, and we should investigate if that works. If there is no connection from wheelDelta(), then handle the scrolling in kdelibs, otherwise assume the application does it. This would avoid adding API for solution C.
>     
>     Thanks for your patience so far.

QObject::receivers() sounds like a deal.


- Christoph


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


On 2010-08-31 15:54:54, David Palacio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/5184/
> -----------------------------------------------------------
> 
> (Updated 2010-08-31 15:54:54)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> KTabWidget implements tab switching as response to mouse wheel events. This makes applications like Rekonq, that use a KTabBar instead, do nothing when the wheel is scrolled.
> 
> The proposed patch moves the behaviour implementation from KTabWidget to
> KTabBar, disables and disconnects the relevant signal in KTabBar/KTabWidget.
> 
> Note that QTabBar in Qt already implements this with a small difference: it
> does not wrap around the extreme tabs. That is, it will not go from the last to
> the first tab and viceversa.
> 
> 
> This addresses bug 248962.
>     https://bugs.kde.org/show_bug.cgi?id=248962
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/apps/dolphin/src/dolphinmainwindow.h 1170268 
>   /trunk/KDE/kdebase/apps/dolphin/src/dolphinmainwindow.cpp 1170268 
>   /trunk/KDE/kdebase/apps/konsole/src/ViewContainer.cpp 1170268 
>   /trunk/KDE/kdelibs/kdeui/widgets/ktabbar.cpp 1170208 
>   /trunk/KDE/kdelibs/kdeui/widgets/ktabwidget.cpp 1170208 
> 
> Diff: http://reviewboard.kde.org/r/5184/diff
> 
> 
> Testing
> -------
> 
> Rekonq works as expected.
> KTabWidget users, such as Konqueror and Konsole work as well.
> 
> 
> Thanks,
> 
> David
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20100831/960f75bd/attachment.htm>