<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/108400/">http://git.reviewboard.kde.org/r/108400/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On January 14th, 2013, 8:59 a.m. CET, <b>Thomas Lübking</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;">my 2c only:

if air has an unnotable *highlight* frame this should be fixed in the theme.
if it is not fixable in the theme because the focus is on a mostly light and "airy" appearance i would frankly raise the question of usability in some contexts and esp. for qml fall back to an osd look (unstyled effect frames) for the default implementations (which can easily be replaced by the plasma components one if the user wants to)

hardcoding a certain look in a theme using qml to fix a specific (even the default) theme is imo definitively wrong.
consider a theme with edgy corners or a dark one with a contrasting highlight color (not tested but there's some ghost theme or so i'd inspect before adding such patch) where the hardcoded workaround might then miserably fail :-(</pre>
 </blockquote>




 <p>On January 14th, 2013, 9:07 a.m. CET, <b>Martin Gräßlin</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;">I have to agree with Thomas here. In additon I have a few questions:
* how does it looks like with other themes? Is the highlight color a required element? Are the themes prepared for it?
* what about the other switchers? Are they suffering the same problem or is the highlight there better?</pre>
 </blockquote>





 <p>On January 14th, 2013, 3:11 p.m. CET, <b>Sebastian Kügler</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;">I agree in principle as well. The switcher does use an unfortunate combination, however, with its grey solid background. "hover" on top of this is almost indistinguishable from the background though. It works a lot better with translucent framesvgs behind and a darker wallpaper (in the taskbar, in krunner for example).

Note that I've also tried using PlasmaComponents.Highlight (which semantically seems even more correct), but that's using the same theme elements.

I've used a few other themes, Oxygen, Slim Glow especially. theme.highlightColor is defined in all of them (and it has been a mandatory color from the beginning, so we can safely assume it's defined and sensible -- it's used in other places as well, so would quickly stick out). The only issue I've seen with some themes is that the rounding of the corners of the theme is not reflected when using my patch, one could maybe use the margins as hint here. It's a fairly minor thing, anyway. In all themes I've tried, the highlighted window was much easier to spot, using the switcher was much faster and needed less mental attention.

I've gone through the other switchers in my kwin install. Some show the same problems, others not to such a degree due to other circumstances:
- informative: slightly better, icon highlighting helps, also easier to scan vertical list
- {large,small} icons: slightly better, frame contrasts with "more uniform icons", could definitely be improved
- windowstick: similar problems
- compact view: slightly better since text is made bold for highlighted items
- grid: same problem
- text icon: same problem
- flipswitch, coverswitch: no problem

In the end, the lack of contrast for me is only a problem in the window switcher, other UI bits where this problem could arise do not suffer from it to that degree. I'd recomment to not just look at this patch, but try it, since it makes quite a difference in usage.

Would it be possible to use a translucent framesvg in the background, so we get some extra contrast from underlying "stuff"?</pre>
 </blockquote>





 <p>On January 14th, 2013, 3:22 p.m. CET, <b>Martin Gräßlin</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;">there shouldn't be a difference in contrast compared to e.g. KRunner. That the TabBox is using an opaque theme is a bug Thomas is working on (see review http://git.reviewboard.kde.org/r/107983/ ).</pre>
 </blockquote>





 <p>On January 14th, 2013, 3:43 p.m. CET, <b>Thomas Lübking</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;">I've checked some themes[1] and except "spoons" (very old version, no idea whether ivan ever updated it), "opaquity" and the "new air" (even the old air) they all sufficiently highlight the selected window.

Reg. the translucency:
i'm not a big tabbox user, but there seems to be two stacked frames here (forming the background plate), the lower one seems completely transparent but blurred when blurring is enabled and the upper one does not seem to be ARGB.
Also i'm no way sure that the mentione fix will help for kwin (kwindowsystem signals to kwin ... i only got kselectionwatcher instantiated after the eventloop is up working so far, but didn't test the patched kwindowsystem from inside kwin -> gonna)
All in all and from a non tabbox expert, it's *looks* somehow wrong in general :-\


[1] Androbit, Ghost, T-remix-black, Tibanna, OpenSuSE, Oxygen, Product, SlimGlow (pretty old version on my HDD, no idea about updates)</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;">I will investigate the TabBox opacity on Wednesday. Wanted to do a TabBox bugfix day anyway.</pre>
<br />










<p>- Martin</p>


<br />
<p>On January 14th, 2013, 9:08 a.m. CET, Sebastian Kügler wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for kwin and Plasma.</div>
<div>By Sebastian Kügler.</div>


<p style="color: grey;"><i>Updated Jan. 14, 2013, 9:08 a.m.</i></p>






<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;">Improve highlighted contrast in thumbnail tabbox

Air's hovered svg provides only a thin frame around the borders, too
little to quickly spot the highlighted item when the whole widget is
only around for a few milliseconds (as is usual with the tabbox).

This patch paints a rectangle with rounded corners behind the
highlighted item instead, making it much easier to spot the currently
highlighted item.

Aimed for master and KDE/4.10.</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;">Tested with Air, Oxygen and Slim Glow, all work as expected, all show contrast improvements, while still looking beautiful. :)</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>kwin/tabbox/qml/clients/thumbnails/contents/ui/main.qml <span style="color: grey">(4c33703d54c84fd54da94f821234e4cbd9c1c001)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/108400/diff/" style="margin-left: 3em;">View Diff</a></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>

<ul>

 <li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/14/kwin-tabbox-contrast-before.png">before, try spotting the highlighted item</a></li>

 <li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/01/14/kwin-tabbox-contrast-after.png"> after, much easier to find</a></li>

</ul>





  </td>
 </tr>
</table>








  </div>
 </body>
</html>