<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/105283/">http://git.reviewboard.kde.org/r/105283/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 18th, 2012, 3:37 p.m., <b>Viranch Mehta</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;">The button size and the hover appearance is different from the original one. The IconButton component was made to keep the look of the buttons consistent with the original version of the applet. Do we want to change this?</pre>
</blockquote>
<p>On June 20th, 2012, 3:15 p.m., <b>David Edmundson</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;">Valid argument for now, won't be valid when everything moves to QML/Plasma Components.
You're maintainer, you have final say.
If you want me to wait till 4.10 when more applets are QML based I will do.</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;">Well after a second thought, I think its a better idea to use plasma components for consistency over plasma rather than maintaining consistency with previous versions. but the original button for some reason looks *really* better in visual terms to me (in fact, the button is also used in some other plasmoids including the network manager). so...
to plasma components dev: can we have an option in the button of what background svg is used? may be a switch between the current one and the one in this plasmoid (widgets/viewitem)?
if that may take time to come up, or is not desired, we can have this patch shipped right in!</pre>
<br />
<p>- Viranch</p>
<br />
<p>On June 17th, 2012, 7:52 p.m., David Edmundson 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.</div>
<div>By David Edmundson.</div>
<p style="color: grey;"><i>Updated June 17, 2012, 7:52 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;">Current battery monitor implements it's own Button class, this previously broke styles with theme text and overloads icon sizes and such.
It's bad for applets to implement their own version of common classes as it prevents consistency.
(will fix the whitespace addition before commit)</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;">Checked applet looked ok.</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/generic/applets/batterymonitor/contents/ui/IconButton.qml <span style="color: grey">(d4454c6)</span></li>
<li>plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml <span style="color: grey">(a2ab72a)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105283/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>