Review Request 128880: [OS X] use a different tab bar widget for tabbed documents

[René J.V. Bertin]

> On Sept. 12, 2016, 1:23 a.m., Aleix Pol Gonzalez wrote:
> > sublime/container.cpp, line 65
> > <https://git.reviewboard.kde.org/r/128880/diff/2/?file=476709#file476709line65>
> >
> >     I'm not super fond of using fusion randomly here. Why does it work for Kate and not for us?

I asked Christoph: from what I understand they don't use QTabBar at all, but an implementation of their own. You must have noticed that it has a different way of handling lots of open documents (which I'm not sure I prefer). I also got the impression they don't plan to move the class to a framework.

What do you mean with random use? It's quite specific, on the contrary. To be honest I hardly notice the Fusion style, esp. in Breeze but also on OS X. There it looks very much like Kate's tabs. I've always thought Qt did quite the job with Fusion, making it look acceptably at home on all major platforms.


> On Sept. 12, 2016, 1:23 a.m., Aleix Pol Gonzalez wrote:
> > sublime/container.cpp, line 61
> > <https://git.reviewboard.kde.org/r/128880/diff/2/?file=476709#file476709line61>
> >
> >     the static declaration can be placed here. Also it should probably be a QPointer.

A QPointer that's a member variable then, I think? `setStyle` doesn't take ownership according to the docs, so I presume the pointer should better remain valid for as long as the class instance lives.


- René J.V.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128880/#review99117
-----------------------------------------------------------


On Sept. 12, 2016, 1:20 a.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128880/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2016, 1:20 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDevelop.
> 
> 
> Bugs: 363473
>     http://bugs.kde.org/show_bug.cgi?id=363473
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> This is a potential fix for an issue raised on BKO: https://bugs.kde.org/show_bug.cgi?id=363473
> 
> It's also the most complete/implementation:
> - applies only when the Macintosh widget style is being used
> - if so, creates a QStyle object for the Fusion widget style
> - when successful, sets the `Sublime::ContainerTabBar` to use that style
> 
> This solves all issues stemming from Qt's use of a "native" widget that is intended only for use in dialogs and not in tabbed document interfaces.
> 
> In my testing, the `ContainerTabBar` ctor is called only rarely, apparently only when changing views (e.g. code -> patch review and back again, or code -> debug). If that observation is correct, use of a global `qTabBarStyle` variable is justified (but more elegant solutions might exist). This observation also justifies (IMHO) the check for the active application style rather than using an `#ifdef Q_OS_OSX` or even applying the fix across all platforms and application styles. That is certainly a possibility that doesn't lead to any shocking style mismatches in my eyes. It does cause some loss of compactness when using my QtCurve settings, which is why I added the style check; a small cost as a gesture to users of a highly configurable style.
> 
> There is still some weirdness behind the tabs which looks like a misaligned well or frame. I'd love to get that right too.
> 
> 
> Diffs
> -----
> 
>   sublime/container.cpp b04f6c3 
> 
> Diff: https://git.reviewboard.kde.org/r/128880/diff/
> 
> 
> Testing
> -------
> 
> See https://bugsfiles.kde.org/attachment.cgi?id=99160 (unpatched) and the last series of screenshots attached to the ticket on BKO. They show the fix applied to various styles on OS X.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20160912/e16ab881/attachment-0001.html>