<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/104727/">http://git.reviewboard.kde.org/r/104727/</a>
     </td>
    </tr>
   </table>
   <br />



 <p>Ship it!</p>



 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It makes sense.</pre>
 <br />







<p>- Artur Duque</p>


<br />
<p>On April 25th, 2012, 4:32 p.m., Aurélien Gâteau wrote:</p>






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

<div>Review request for Plasma and David Edmundson.</div>
<div>By Aurélien Gâteau.</div>


<p style="color: grey;"><i>Updated April 25, 2012, 4:32 p.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;">ToolButton does not support keyboard navigation. The attached patch fixes this by:

- Using the "surface" item to indicate focus when the "flat" property is set to true

- Not giving focus on click if keyboard navigation is not defined. This is a bit tricky, but I figured it would be odd to have a focus border around a button which is part of a toolbar. I could not think of a better way to figure out whether giving focus on click made sense or not.</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;">Running attached test script without the patch, "Focusable Button 1" starts focused (this can be seen by pressing "space") but there is no indication that it is. With the patch, an hover frame appears around the focused button. Pressing tab moves the focus to "Focusable Button 2" which gets the hover frame.
Clicking one of the focusable buttons gives them focus as well, but clicking one of the toolbar buttons does not, since keyboard navigation is not defined for them.</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>plasma/declarativeimports/plasmacomponents/qml/ToolButton.qml <span style="color: grey">(9e7e715)</span></li>

</ul>

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




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








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