<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/123546/">https://git.reviewboard.kde.org/r/123546/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 26th, 2015, 6:34 p.m. CEST, <b>Martin Klapetek</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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">but we should not use QCommandLinkButton as it is activated on press (and there is no way to easily change it).</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QCommandLinkButton derives from QPushButton, it has exactly the same signals including clicked(); there's really no reason to change that. I'll be changing it back to QCommandLinkButton.</p></pre>
</blockquote>
<p>On May 26th, 2015, 7:19 p.m. CEST, <b>David Rosca</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That's correct, sorry. When I tested it, for some reason I incorrectly thought the clicked signal is emitted on button press.
But there actually is now a reason to use QPushButton instead of QCommandLinkButton: QCommandLinkButton don't have vertically aligned icon and text in Breeze style ( http://wstaw.org/m/2015/05/26/qcommandlinkbutton-breeze.png this is with this commit reverted).
It should be fixed in Breeze, but still...</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Yes, that's a bug in different product and should be fixed there, we shouldn't really be working around it, especially if we have an (relatively easy) way of fixing the cause of the bug. Plus, QPushButton cannot horizontally align things, which imo is a bigger (visual) issue ;)</p></pre>
<br />
<p>- Martin</p>
<br />
<p>On May 11th, 2015, 12:10 a.m. CEST, David Rosca wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Telepathy and Martin Klapetek.</div>
<div>By David Rosca.</div>
<p style="color: grey;"><i>Updated May 11, 2015, 12:10 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kaccounts-integration
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Connect to QPushButton::clicked signal instead of QPushButton::pressed.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This makes it consistent with other buttons, eg. the button action is activated only when releasing the mouse inside the button rect, not immediately on mouse press.
It is also needed to change QCommandLinkButton to QPushButton, where it changes the text alignment from left-aligned to center-aligned. I don't know how to replicate the look with
QPushButton, but we should not use QCommandLinkButton as it is activated on press (and there is no way to easily change it).</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Buttons are now activated on mouse release.</p></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>src/accountwidget.cpp <span style="color: grey">(a56cf04)</span></li>
<li>src/create.cpp <span style="color: grey">(5e4d46e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123546/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>