<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/127876/">https://git.reviewboard.kde.org/r/127876/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 9th, 2016, 10:22 p.m. UTC, <b>Andreas Kainz</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 can't review the code but this is one of the best features in the past 2 years
+1 from the vdg</p></pre>
</blockquote>
<p>On May 10th, 2016, 11:40 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;">Well, Ship it is a bit premature.
I could not test the patch but: it compiles and code looks sensible.
However, there are probably pieces missing. I could spot on, on non-flat toolbuttons
(should be in bool Style::drawToolButtonLabelControl, round line 4310 or so, for !flat && hasFocus)
Also, did you test if it works for readonly comboboxes ? (possibly this is already covered by pushbuttons).
You can probably test this with oxyge-demo/oxygen-demo5
(with style breeze)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Finally, oxygen would need a similar patch. I can put it in, but without being yet able to test (I'd need the other updates). So if you have the time and motivation, feel free :)</p></pre>
</blockquote>
<p>On May 10th, 2016, 12:13 p.m. UTC, <b>Marco Martin</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;">oxygen-demo5 seems ok for things that depend from the style (the icons in the sidebar would need to be colored as well but i don't think it depends from the style?)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">oxygen doesn't have menu items or selected buttons colored with highlight colors tough, so probably there aren't parts in oxygen that are affected</p></pre>
</blockquote>
</blockquote>
<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;">Ok even for toolbuttons (see http://wstaw.org/m/2016/05/10/plasma-desktopE24540.png) ?
Note that you need to use the Tab key to get the focus on the said buttons. Pressing them will not pass focus to them).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For Oxygen: it does have menu item colored, on option! (damn options):
http://wstaw.org/m/2016/05/10/plasma-desktopy24540.png
but you are right this is not the default, so low priority.
And for pushbuttons, you are right.</p></pre>
<br />
<p>- Hugo</p>
<br />
<p>On May 9th, 2016, 3:32 p.m. UTC, Marco Martin 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 Frameworks and Plasma.</div>
<div>By Marco Martin.</div>
<p style="color: grey;"><i>Updated May 9, 2016, 3:32 p.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;">with the stylesheet based recoloring for icons, it is possible to make selected icons the same color as the selected text from the palette.
with the breeze icons style this makes the selected item more readable and more in line with the style.</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">(403771c)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127876/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/05/09/cfbb4571-ed40-411e-ad89-453d29fa2610__menu.png">menu.png</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>