<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/127494/">https://git.reviewboard.kde.org/r/127494/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 26th, 2016, 10:14 a.m. UTC, <b>Hugo Pereira Da Costa</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/127494/diff/1/?file=454100#file454100line3388" style="color: black; font-weight: bold; text-decoration: underline;">kstyle/breezestyle.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">3388</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> const bool hasFocus( ( enabled && ( state & ( State_HasFocus | State_Sunken ) ) ) && !( widget && widget->focusProxy()));</pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">3388</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> const State focusState ( isQtQuickControl( option, widget ) ? State_HasFocus : State_HasFocus | State_Sunken );</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;">mmm. But is it not rather a qtc bug that states are set in an inconsistent manner between widgets and qtc ?
This is not the only place in breeze where we have to work around these kind of issues (the other being min iconsize in menus to cite one)
This just tend to make the breeze code bloated and hard to maintain. Besides it doesn't fix the "other" styles (there is oxygen, but there are others).
What are the chances to get these fixed upstream (eg: sunken !? checked) ?</p></pre>
</blockquote>
<p>On March 26th, 2016, 10:15 a.m. UTC, <b>Hugo Pereira Da Costa</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;">meant "sunken neq checked"</p></pre>
</blockquote>
<p>On March 26th, 2016, 10:32 a.m. UTC, <b>David Rosca</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;">Well, QtQuickControls is probably to blame, but it works fine with other styles. What I think is incorrect (even for QtWidgets) is to assume focus on button with State_Sunken.
>From doc:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">QStyle::State_Sunken 0x00000004 Used to indicate if the widget is sunken or pressed.
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">so the widget can be sunken, but not pressed.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The State_Sunken flag for the hasFocus check was added in https://git.reviewboard.kde.org/r/120222/ to fix ToolButtons, but this function is not used for drawing ToolButtons. So maybe we can just use only <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">state & State_HasFocus</code> here for both quickcontrols and widgets?</p></pre>
</blockquote>
<p>On March 26th, 2016, 10:35 a.m. UTC, <b>Hugo Pereira Da Costa</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;">worth at try yes. i can double check for the widget parts.
Funny that the <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">other</em> commit also refers to qtc</p></pre>
</blockquote>
<p>On March 26th, 2016, 10:39 a.m. UTC, <b>David Rosca</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;">Yeah, I guess it was "fixed" in the meantime. I was testing with oxygen-demo5 and seems to work as before.
There is still issue with PushButtons where some are not drawn as checked/focused when showing menu (Buttons tab -> pushbuttons -> both in third column).</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;">Oh but third column in the button tabs are actually comboboxes (read-only).
Buttons with menu are the second tab and here it seems (without the patch at least) that button's focus is rendered properly.
In fact even for combobox it is "almost fine" I'd say. Once the menu is closed (because one item has been selected), the combobox does have the focus.</p></pre>
<br />
<p>- Hugo</p>
<br />
<p>On March 26th, 2016, 10:41 a.m. UTC, David Rosca 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 Plasma and Hugo Pereira Da Costa.</div>
<div>By David Rosca.</div>
<p style="color: grey;"><i>Updated March 26, 2016, 10:41 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
breeze
</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;">Button with checked == true would be incorrectly drawn as always having focus.
Checked QtQuickControls Button always have State_Sunken.</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;">Fixed for QQC, no change in QtWidgets.</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>kstyle/breezestyle.cpp <span style="color: grey">(e97ead6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127494/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>