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











<div>




<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/118272/diff/1/?file=274298#file274298line156" style="color: black; font-weight: bold; text-decoration: underline;">applets/batterymonitor/contents/ui/BatteryItem.qml</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">156</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="k">leftMargin:</span> <span class="mi">3</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">122</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="k">leftMargin:</span> <span class="nx"><span class="hl">units</span></span><span class="p"><span class="hl">.</span></span><span class="nx"><span class="hl">gridUnit</span></span><span class="hl"> </span><span class="o"><span class="hl">/</span></span><span class="hl"> </span><span class="mi">3</span></pre></td>
  </tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Use .floor/.round here to avoid half-pixels

...and basically everywhere else</pre>
</div>
<br />



<p>- Martin Klapetek</p>


<br />
<p>On May 23rd, 2014, 12:36 a.m. CEST, Sebastian Kügler wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 Plasma and Kai Uwe Broulik.</div>
<div>By Sebastian Kügler.</div>


<p style="color: grey;"><i>Updated May 23, 2014, 12:36 a.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</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;">Clean up Battery Monitor

This patch simplifies the battery's ui, and brings spacing in line with
Plasma 5 standards.

In detail:

- removing the selection item on batteries: it's already visible by the
  text shown that the battery is selected. It also doesn't need the
  selection semantics across the ui, because -- for what?
- remove separator lines, in Plasma 5, we use spacing for this case
- align extra battery info to the slider, improves logical grouping
- Improve HighDPI by removing hard-coded layout hints, use units.gridUnit
  throughout
- Fix batteryitem's status: it would show the wrong icon and time label,
  because AC Adapter can be empty. Checking charging is semantically
  correct here, since it uses the charge state, not the adapter state.
- remove a bunch of SVGs that were used internally to get margins -- use
  gridUnit for layout internal margins instead
- fix slider's right alignment, this would jump based on the percentage
  label's width, which varies per item. We know the rough length of the
  percentage label from the context, and can align the labels to that.

Code is also in kde-workspace[sebas/batterycleanup].</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 plasmoidviewer and in the shell, plugging ac in and out, tried different combinations of values.</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>applets/batterymonitor/contents/ui/BatteryItem.qml <span style="color: grey">(431aa9ad40435e331cfff494cbe6f9f1e409ce35)</span></li>

 <li>applets/batterymonitor/contents/ui/BrightnessItem.qml <span style="color: grey">(a91ccdeebd7136becfb9d78bfade0e6189f6221a)</span></li>

 <li>applets/batterymonitor/contents/ui/PopupDialog.qml <span style="color: grey">(0444199a7d34deac0472bb50961a34b8717c9f5a)</span></li>

 <li>applets/batterymonitor/contents/ui/PowerManagementItem.qml <span style="color: grey">(530fce30e7e81ebbe6e394d7c8927806e3b9d08b)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/118272/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="https://git.reviewboard.kde.org/media/uploaded/files/2014/05/22/abc1d750-6eb7-4728-a5f2-e9f8566b5b44__battery-before-collapsed.png">before (collapsed)</a></li>

 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/05/22/2a0b9ad3-03e8-4f07-a158-ceb0d3d0e23c__battery-before-expanded.png">before (expanded)</a></li>

 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/05/22/4439093c-345c-44af-afa1-a05cb0973990__battery-after-collapsed.png">after (collapsed)</a></li>

 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/05/22/b18cf61f-0ff1-4e91-93c2-ebd6b1dab38d__battery-after-expanded.png">after (expanded)</a></li>

</ul>





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








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