<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/101312/">http://git.reviewboard.kde.org/r/101312/</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 9th, 2011, 10:45 p.m., <b>Aurélien Gâteau</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;">Looks good. I would even go a bit further and suggest the following:
- Remove the Clear button, I don't think it serves any purpose
- Change the label of the OK button to "Create Link"</pre>
</blockquote>
<p>On May 10th, 2011, 10:54 a.m., <b>Jonathan Marten</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;">Good idea to remove the pointless Clear button (probably a hangover from the days when line edit's didn't have clear buttons). Not possible to change the OK button text, though, because this is a general dialogue which may be used in other places.</pre>
</blockquote>
<p>On May 10th, 2011, 10:54 a.m., <b>Christoph Feck</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 think the dialog is more generic and is (could?) also be used by programs for adding bookmarks or something else, so labeling the button "Create Link" could be confusing.
The Clear button is really a relict, and it can be removed if the fields have a standard KLineEdit clear button already.</pre>
</blockquote>
<p>On May 10th, 2011, 10:55 a.m., <b>Christoph Feck</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;">Oh, two idiots adding the same comment at the same time ;)</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;">No no, one idiot did not realize it was a generic method, and two smart developers corrected him :)</pre>
<br />
<p>- Aurélien</p>
<br />
<p>On May 8th, 2011, 3:42 p.m., Jonathan Marten 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 kdelibs.</div>
<div>By Jonathan Marten.</div>
<p style="color: grey;"><i>Updated May 8, 2011, 3:42 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;">This dialogue - used, for example, for the desktop's "Create New - Link to Location" or "Create New - Basic link to" actions, has a very ugly layout where the labels are squashed up to the entry fields and the two lines are not vertically aligned with each other. This change uses a form layout instead, which automatically adopts the standard KDE style and spacing.
In addition to the layout, the "OK" button is enabled when the dialogue is first shown; it should not be because the two entry fields are empty. This is checked at the end of the constructor.</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;">Built kdelibs with these changes, checked operation and appearance of dialogue via Plasma desktop.</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>kfile/knameandurlinputdialog.cpp <span style="color: grey">(fd02af6)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101312/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/101312/s/158/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/05/08/newfiledialog-fixlayout_before_400x100.png" style="border: 1px black solid;" alt="Before" /></a>
<a href="http://git.reviewboard.kde.org/r/101312/s/159/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/05/08/newfiledialog-fixlayout_after_400x100.png" style="border: 1px black solid;" alt="After" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>