<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/128880/">https://git.reviewboard.kde.org/r/128880/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 12th, 2016, 1:23 a.m. CEST, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/128880/diff/2/?file=476709#file476709line65" style="color: black; font-weight: bold; text-decoration: underline;">sublime/container.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">65</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setStyle</span><span class="p">(</span><span class="n">qTabBarStyle</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm not super fond of using fusion randomly here. Why does it work for Kate and not for us?</p></pre>
</blockquote>
<p>On September 12th, 2016, 9:23 a.m. CEST, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On September 12th, 2016, 1:25 p.m. CEST, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Doesn't Qt offer any native OS X component that works as tabs? Switching style should be a last resource.</p></pre>
</blockquote>
<p>On September 12th, 2016, 2:42 p.m. CEST, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">No, nothing other than the QTabBar widget, the look of which I've documented here and in the linked bug report. I'm not even aware if OS X itself has another tab widget; tab style isn't exactly identical in applications that provide a tabbed document interface (i.e. they all seem to roll their own to some extent).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That said, whether Cocoa has a native tabbed document thingy is a moot point: Qt uses native drawing routines but not the native widgets (bar a few select exceptions). They achieve the native look by using standard Qt drawing primitives that map to native drawing calls but that could just as well map to X11 drawing commands (and they do if you use the xcb QPA).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">They only thing that we can do with the native QTabBar, apparently, is to configure it to behave as much as possible like QTabBar does with basically all other widget styles. The 3 lines after the style switch take care of that. I'll presume you agree that looks awful. I certainly doesn't look like anything I've ever seen on OS X before.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'll see if I have other truly native apps (other than Safari) that put up a tabbed document interface on my Mac, and post screenshot(s).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What I think we should asks ourselves here is if using the Fusion style jumps out as something odd or anomalous when we try to look at it without remembering that we're seeing a different style rather than a bottom-up implementation of the interface? Would you have noticed if you hadn't known beforehand that this was a cheap alternative to implementing a new tabbar widget from scratch?</p></pre>
</blockquote>
<p>On September 12th, 2016, 9:04 p.m. CEST, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Maybe it would make sense to open a bug report on Qt requesting this display accessible? https://git.reviewboard.kde.org/r/128880/file/3353/</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Then mention the bug report and implement the setStyle workaround for the moment.</p></pre>
</blockquote>
<p>On September 12th, 2016, 10:52 p.m. CEST, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">https://bugreports.qt.io/browse/QTBUG-44473</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">How is that bug report related at all?</p></pre>
<br />
<p>- Aleix</p>
<br />
<p>On September 12th, 2016, 10:47 p.m. CEST, René J.V. Bertin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Software on Mac OS X and KDevelop.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Sept. 12, 2016, 10:47 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=363473">363473</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevplatform
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is a potential fix for an issue raised on BKO: https://bugs.kde.org/show_bug.cgi?id=363473</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Sublime::ContainerTabBar</code> to use that style</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In my testing, the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">ContainerTabBar</code> 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 <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">qTabBarStyle</code> 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 <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">#ifdef Q_OS_OSX</code> 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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There is still some weirdness behind the tabs which looks like a misaligned well or frame. I'd love to get that right too.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>sublime/container.cpp <span style="color: grey">(b04f6c3)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128880/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/09/12/3fbb5571-8fb7-4af7-b6d5-eb1d9b32cea1__tabbar-breeze-with-fusion.png">background: pure Breeze theme. Foreground: Breeze with the QTabBar set to Fusion</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/09/12/b1bc9406-4f98-4920-832b-2728aa9d2ee2__tabbar-breeze-with-qtcurve.png">Background: pure QtCurve<OS X Graphite>. Foreground: QtCurve<OS X Graphite> with QTabBar set to Fusion</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/09/12/5790c412-566c-42bc-83b5-efbec458d711__MacNativeTabs.png">Tabbed document interface in native OS X applications</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/09/12/0a98cc35-dad4-439c-af91-51ffc14e21b0__MacNative-QTabBarFusion.png">pure native Macintosh style with Fusion QTabBar</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/09/12/56f92d7e-8f71-401d-b874-55165d0f701a__MacNative-QTabBarFusion.png">pure native Macintosh style, Fusion QTabBar, no misaligned underlining</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>