<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/102802/">http://git.reviewboard.kde.org/r/102802/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 14th, 2011, 12:05 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;">Nice, work, as always :) Only one question: Do the port spin boxes need the "Port:" labels, as seen in the ASCII drawings from bug 147340? Or would it clutter the interface? Maybe a "Hostname" and "Port" headline for the two columns?</pre>
</blockquote>
<p>On October 14th, 2011, 4:58 a.m., <b>Dawit Alemayehu</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 guess "Port:" can be added. I simply copied what was in the old configuration dialog with very minor modifications. I do not have a preference one way or the other. I will add the port labels to clarify the interface and make sure users understand the purpose of those inputs.</pre>
</blockquote>
<p>On October 14th, 2011, 9:39 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;">Looks good, but one thing that IMHO doesn't look right is the option-specific entry fields appearing and disappearing within the radio button list as they are selected. Might it look better if the 5 options were in a (full width) combo box at the top, with the area below for the variable fields?
</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;">Using a combobox and a stacked widget for each option specific input is much worse than the current solution. The alternative is to show every like Firefox does, but disable/enable the option specific fields when a particular proxy settings is selected [1]. Of course, this would make the dialog a bit longer, but I guess it is not that bad. Either way, I do not have a preference. Either one will do just fine AFAIC.
[1] See the last screenshot.</pre>
<br />
<p>- Dawit</p>
<br />
<p>On October 14th, 2011, 8:38 p.m., Dawit Alemayehu 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 KDE Base Apps.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated Oct. 14, 2011, 8:38 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 patch completely overhauls the proxy configuration dialog so that it is much simpler to use and maintain. The major difference between it and the old configuration dialog is the merging of the configuration dialogs for the different proxy types into the main proxy dialog and the addition of a new input for specifying SOCKS proxy settings. See the attached screenshots to see how the new dialog looks.</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;">- Configure proxy through dialog and validate the results by checking the settings saved in $KDEHOME/share/config/kioslaverc.
- Reloading the proxy dialog to see the saved information is properly loaded back again.
</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=82352">82352</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=139789">139789</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=142562">142562</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=145006">145006</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=147340">147340</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=164460">164460</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=189019">189019</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=190149">190149</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=190901">190901</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=205594">205594</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=258196">258196</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=283226">283226</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>konqueror/settings/kio/CMakeLists.txt <span style="color: grey">(e5476c9)</span></li>
<li>konqueror/settings/kio/envvarproxy.ui <span style="color: grey">(a09450d)</span></li>
<li>konqueror/settings/kio/kenvvarproxydlg.h <span style="color: grey">(c7d2f5f)</span></li>
<li>konqueror/settings/kio/kenvvarproxydlg.cpp <span style="color: grey">(ef5c86c)</span></li>
<li>konqueror/settings/kio/kmanualproxydlg.h <span style="color: grey">(4efa63c)</span></li>
<li>konqueror/settings/kio/kmanualproxydlg.cpp <span style="color: grey">(6760957)</span></li>
<li>konqueror/settings/kio/kproxydlg.h <span style="color: grey">(b2cafb8)</span></li>
<li>konqueror/settings/kio/kproxydlg.cpp <span style="color: grey">(2f512ce)</span></li>
<li>konqueror/settings/kio/kproxydlg.ui <span style="color: grey">(96ff996)</span></li>
<li>konqueror/settings/kio/kproxydlgbase.h <span style="color: grey">(2047b90)</span></li>
<li>konqueror/settings/kio/kproxydlgbase.cpp <span style="color: grey">(9933698)</span></li>
<li>konqueror/settings/kio/ksaveioconfig.h <span style="color: grey">(e895c2c)</span></li>
<li>konqueror/settings/kio/ksaveioconfig.cpp <span style="color: grey">(62852fa)</span></li>
<li>konqueror/settings/kio/manualproxy.ui <span style="color: grey">(c69047f)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102802/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/102802/s/296/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/10/13/noproxy_400x100.png" style="border: 1px black solid;" alt="No Proxy" /></a>
<a href="http://git.reviewboard.kde.org/r/102802/s/297/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/10/13/automaticscriptproxy_400x100.png" style="border: 1px black solid;" alt="PAC Proxy" /></a>
<a href="http://git.reviewboard.kde.org/r/102802/s/298/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/10/13/systemproxy_400x100.png" style="border: 1px black solid;" alt="System Proxy" /></a>
<a href="http://git.reviewboard.kde.org/r/102802/s/301/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/10/14/manualproxy_400x100.png" style="border: 1px black solid;" alt="Manual Proxy" /></a>
<a href="http://git.reviewboard.kde.org/r/102802/s/303/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/10/14/new_proxy_dialog_with_show_all_options_400x100.png" style="border: 1px black solid;" alt="Alternate dialog with all options visible" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>