<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/115408/">https://git.reviewboard.kde.org/r/115408/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 30th, 2014, 9:20 p.m. UTC, <b>Thomas Lübking</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;">tried to center align it?</pre>
</blockquote>
<p>On January 30th, 2014, 9:23 p.m. UTC, <b>kdeuser56 kdeuser56</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;">Center what? The widget or the iconlabel? I think in case of the widget it makes no sense.
In case of the iconlabel the icon itself would be on the exact same position as the icon of the icon button.
I tried both, but I find right aligned prettier and more logical considering the text under it. </pre>
</blockquote>
<p>On January 30th, 2014, 9:49 p.m. UTC, <b>Thomas Lübking</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;">Either. Reason is that due to the "arbitrary" shape (shadow, padding), rightaligning the icon/button does not really line up with the text - thus looks buggy (failed approach)
Otoh, left alingning looks unbalanced mostly due to the vertical tab border (ie. it's less an issue in eg. bespin, which has no vertical tabborder)</pre>
</blockquote>
<p>On January 30th, 2014, 10:02 p.m. UTC, <b>kdeuser56 kdeuser56</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;">I am not sure I understand your point. Could explain me the "arbitrary" shape problem?
I do not think the visual problem is caused by the vertical tab border. Even if I remove that, it still looks bad and the bigger the font gets, the uglier it looks. (Some words are longer in other languages, so this could also be triggered by different language settings).
So in my opinion it looks bad, regardless of the theme.
Do you have another idea?
</pre>
</blockquote>
<p>On January 30th, 2014, 10:14 p.m. UTC, <b>kdeuser56 kdeuser56</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;">I would be okay with centering the iconlabel. Centering the widget too would be still better than left aligning, but I would prefer to keep it right.
Should I change the diff to center the iconlabel, or should I provide screenshots first? </pre>
</blockquote>
<p>On January 30th, 2014, 10:17 p.m. UTC, <b>Thomas Lübking</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;">> Could explain me the "arbitrary" shape problem?
The icons do not have a defined border as the text. They'll visually end some pixels before the logical end (would even get worse for icons that do not contain a sheet, but even that is visually 2px off)
"Ugly" is subjective.
Fact is, that the vertical border creates a distinct border that the left aligned version is closer to than to the implicit right border, thus it's unbalanced (left overweight)
If you remove that border, the label visually moves more to the center between window frame and implicit right border (the lineedit), what implies local symmetry, thus stability. (Actually, you might even get a golden mean positioning)
Granted: for long texts and huge fonts the part left of the tab certainly becomes neglectable and that effect is lost.
-> I'd simply center align the icon.</pre>
</blockquote>
<p>On January 30th, 2014, 10:18 p.m. UTC, <b>Thomas Lübking</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;">I'd suggest to have screenshots and wait for some more opinions then.</pre>
</blockquote>
<p>On January 31st, 2014, 8:47 p.m. UTC, <b>kdeuser56 kdeuser56</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;">Okay I have added screenshots (widget-<position>-icon<position>.png). Please provide feedback and I will change the review request according to that feedback. </pre>
</blockquote>
<p>On February 7th, 2014, 6:43 p.m. UTC, <b>kdeuser56 kdeuser56</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;">Thomas: Will you ship it, if we agree to center align it? I think we can agree, that center is better than the current situation.</pre>
</blockquote>
<p>On February 7th, 2014, 6:54 p.m. UTC, <b>Thomas Lübking</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;">Yes, of course - but you're seeking approval of Peter Penz.</pre>
</blockquote>
<p>On February 7th, 2014, 6:55 p.m. UTC, <b>kdeuser56 kdeuser56</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;">Is he still active? (http://ppenz.blogspot.co.at/2012/06/dolphin-21.html)</pre>
</blockquote>
<p>On February 7th, 2014, 7:05 p.m. UTC, <b>Thomas Lübking</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;">Ah, no - sorry (just git blame'd and didn't think =)
Please attach Frank Reininghaus instead (since dolphin is still likely the most important client code)</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;">Dolphin may be one of the major users of the Properties dialog, but still, design and placement of stuff on the screen are not really my areas of expertise.
I agree that the current state is not optimal, and I appreciate that work is being done to improve it. However, I don't feel qualified to judge what's the best alternative. Sorry about that.</pre>
<br />
<p>- Frank</p>
<br />
<p>On February 8th, 2014, 10:02 a.m. UTC, kdeuser56 kdeuser56 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 kdelibs and Frank Reininghaus.</div>
<div>By kdeuser56 kdeuser56.</div>
<p style="color: grey;"><i>Updated Feb. 8, 2014, 10:02 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</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;">The iconbutton and the iconlabel were clearly aligned using the old style, when everything was left aligned.
In my interpretation of the KDE HIG guidelines, the iconbutton/label should also be right aligned.
Especially with bigger font sizes, the visual issue becomes obvious.
Idea: see kproperties-dolphin-1.png
Before: see before-1.png and before-2.png
After: see after-1.png and after-2.png
Diff for kio (frameworks) can be found here: http://pastebin.kde.org/p4ojv6a1w</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;">Compiled and installed. Works as expected. </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>kio/kfile/kpropertiesdialog.cpp <span style="color: grey">(6611ee7)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115408/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/01/30/91648ead-a248-4c42-b45c-8741d1291955__kproperties-dolphin-1.png">idea</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/01/30/f9b5bba2-f810-4de5-b292-da66e0cf60ac__before-1.png">before1</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/01/30/516dbfec-597f-4f95-bb83-797d10ddebfc__before-2.png">before2</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/01/30/03fdb43f-6f67-407f-be27-e6afad906340__after-1.png">after1</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/01/30/06455bef-a229-4a1a-b9c0-cb1de61f7fa0__after-2.png">after2</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/01/31/ab93b637-e914-4521-a9c5-025515c97790__widget-center-icon-center.png">center-center</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/01/31/38cd56fb-c411-4876-bebe-bc9923855751__widget-left-icon-leftunpatched.png">left-left</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/01/31/80672290-b6fb-4fe3-b2ab-5ea5f0c6ed53__widget-right-icon-center.png">right-center</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/01/31/8dec5429-021a-49a0-a34f-1a2e77d7aeef__widget-right-icon-right.png">right-right</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>