<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/105542/">http://git.reviewboard.kde.org/r/105542/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 13th, 2012, 7:20 a.m., <b>Lasath Fernando</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;">Why does no-one like that button :-/
The part I'm not sure is a good idea is putting a ToolBar in a popout (I haven't seen anything else do that). Did it look really horrible with all those buttons above a separator?</pre>
</blockquote>
<p>On July 14th, 2012, 1:22 a.m., <b>Aleix Pol Gonzalez</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;">From my point of view, if we have components, we should use them. If they don't apply then the component should be fixed. Otherwise we won't ever make all plasmoids look alike...
If you prefer the former implementation, I can remove the change to ToolBar, no problem. I don't really have a say on the looks :).</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;">@Aleix - screenshots on visual changes in future please.
my POV:
Separate button for minimize is a massive +1. Annoying button was annoying, or at least ambiguous in what it's function was.
However, frame inside a frame is bad. Particularly with the ToolButtons, which are a frame inside a frame inside a frame. Very bad!
Padding on toolbar to the items is completely missing. Frame of the button should not touch the frame of the toolbar (on mouseover) and the fact the close button does on the top and bottom but not the right is so wrong.
(Sorry, I whinge about padding a lot.. not that it's currently in a good state)
I'm not sure the plasma component is designed for this use case of being a toolbar in a popup applet, no-one else does. It's more for the apps for Active, so whilst I agree with the "use the components!" mantra, I don't think it applies in this case.
@Lasath - you're component maintainer you have to make hard decisions, accept something you don't like, or rejecting someones patch (or say how to change it)
</pre>
<br />
<p>- David</p>
<br />
<p>On July 14th, 2012, 10:05 p.m., Aleix Pol Gonzalez 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 Telepathy and David Edmundson.</div>
<div>By Aleix Pol Gonzalez.</div>
<p style="color: grey;"><i>Updated July 14, 2012, 10:05 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;">Make the minimize button a separate button.
Put all the top buttons in a toolbar.</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;">I talked to myself wisely.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=293488">293488</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ChatWidget.qml <span style="color: grey">(90cc07d)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105542/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/105542/s/636/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/07/14/tmp_1_400x100.png" style="border: 1px black solid;" alt="After" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>