Re: Review Request: Proxy overhaul Part 6: Proxy Configuration dialog rewrite
On Oct. 14, 2011, 12:05 a.m., Christoph Feck wrote: 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? Dawit Alemayehu wrote: 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. Jonathan Marten wrote: 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? 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. - Dawit --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/#review7304 --- On Oct. 14, 2011, 8:38 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/ --- (Updated Oct. 14, 2011, 8:38 p.m.) Review request for KDE Base Apps. Description --- 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. This addresses bugs 82352, 139789, 142562, 145006, 147340, 164460, 189019, 190149, 190901, 205594, 258196, and 283226. http://bugs.kde.org/show_bug.cgi?id=82352 http://bugs.kde.org/show_bug.cgi?id=139789 http://bugs.kde.org/show_bug.cgi?id=142562 http://bugs.kde.org/show_bug.cgi?id=145006 http://bugs.kde.org/show_bug.cgi?id=147340 http://bugs.kde.org/show_bug.cgi?id=164460 http://bugs.kde.org/show_bug.cgi?id=189019 http://bugs.kde.org/show_bug.cgi?id=190149 http://bugs.kde.org/show_bug.cgi?id=190901 http://bugs.kde.org/show_bug.cgi?id=205594 http://bugs.kde.org/show_bug.cgi?id=258196 http://bugs.kde.org/show_bug.cgi?id=283226 Diffs - konqueror/settings/kio/CMakeLists.txt e5476c9 konqueror/settings/kio/envvarproxy.ui a09450d konqueror/settings/kio/kenvvarproxydlg.h c7d2f5f konqueror/settings/kio/kenvvarproxydlg.cpp ef5c86c konqueror/settings/kio/kmanualproxydlg.h 4efa63c konqueror/settings/kio/kmanualproxydlg.cpp 6760957 konqueror/settings/kio/kproxydlg.h b2cafb8 konqueror/settings/kio/kproxydlg.cpp 2f512ce konqueror/settings/kio/kproxydlg.ui 96ff996 konqueror/settings/kio/kproxydlgbase.h 2047b90 konqueror/settings/kio/kproxydlgbase.cpp 9933698 konqueror/settings/kio/ksaveioconfig.h e895c2c konqueror/settings/kio/ksaveioconfig.cpp 62852fa konqueror/settings/kio/manualproxy.ui c69047f Diff: http://git.reviewboard.kde.org/r/102802/diff/diff Testing --- - 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. Screenshots --- No Proxy http://git.reviewboard.kde.org/r/102802/s/296/ PAC Proxy http://git.reviewboard.kde.org/r/102802/s/297/ System Proxy http://git.reviewboard.kde.org/r/102802/s/298/ Manual Proxy http://git.reviewboard.kde.org/r/102802/s/301/ Alternate dialog with all options visible http://git.reviewboard.kde.org/r/102802/s/303/ Thanks, Dawit Alemayehu
Re: Review Request: Proxy overhaul Part 6: Proxy Configuration dialog rewrite
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/ --- (Updated Oct. 18, 2011, 6:53 a.m.) Review request for KDE Base Apps. Changes --- Removed a closed and unrelated bug report from the list. Description --- 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. This addresses bugs 82352, 139789, 147340, 164460, 189019, 190149, 190901, 205594, 258196, and 283226. http://bugs.kde.org/show_bug.cgi?id=82352 http://bugs.kde.org/show_bug.cgi?id=139789 http://bugs.kde.org/show_bug.cgi?id=147340 http://bugs.kde.org/show_bug.cgi?id=164460 http://bugs.kde.org/show_bug.cgi?id=189019 http://bugs.kde.org/show_bug.cgi?id=190149 http://bugs.kde.org/show_bug.cgi?id=190901 http://bugs.kde.org/show_bug.cgi?id=205594 http://bugs.kde.org/show_bug.cgi?id=258196 http://bugs.kde.org/show_bug.cgi?id=283226 Diffs - konqueror/settings/kio/CMakeLists.txt e5476c9 konqueror/settings/kio/envvarproxy.ui a09450d konqueror/settings/kio/kenvvarproxydlg.h c7d2f5f konqueror/settings/kio/kenvvarproxydlg.cpp ef5c86c konqueror/settings/kio/kmanualproxydlg.h 4efa63c konqueror/settings/kio/kmanualproxydlg.cpp 6760957 konqueror/settings/kio/kproxydlg.h b2cafb8 konqueror/settings/kio/kproxydlg.cpp 2f512ce konqueror/settings/kio/kproxydlg.ui 96ff996 konqueror/settings/kio/kproxydlgbase.h 2047b90 konqueror/settings/kio/kproxydlgbase.cpp 9933698 konqueror/settings/kio/ksaveioconfig.h e895c2c konqueror/settings/kio/ksaveioconfig.cpp 62852fa konqueror/settings/kio/manualproxy.ui c69047f Diff: http://git.reviewboard.kde.org/r/102802/diff/diff Testing --- - 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. Screenshots --- No Proxy http://git.reviewboard.kde.org/r/102802/s/296/ PAC Proxy http://git.reviewboard.kde.org/r/102802/s/297/ System Proxy http://git.reviewboard.kde.org/r/102802/s/298/ Manual Proxy http://git.reviewboard.kde.org/r/102802/s/301/ Alternate dialog with all options visible http://git.reviewboard.kde.org/r/102802/s/303/ Thanks, Dawit Alemayehu
Re: Review Request: Proxy overhaul Part 6: Proxy Configuration dialog rewrite
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/ --- (Updated Oct. 14, 2011, 8:38 p.m.) Review request for KDE Base Apps. Description --- 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. This addresses bugs 82352, 139789, 142562, 145006, 147340, 164460, 189019, 190149, 190901, 205594, 258196, and 283226. http://bugs.kde.org/show_bug.cgi?id=82352 http://bugs.kde.org/show_bug.cgi?id=139789 http://bugs.kde.org/show_bug.cgi?id=142562 http://bugs.kde.org/show_bug.cgi?id=145006 http://bugs.kde.org/show_bug.cgi?id=147340 http://bugs.kde.org/show_bug.cgi?id=164460 http://bugs.kde.org/show_bug.cgi?id=189019 http://bugs.kde.org/show_bug.cgi?id=190149 http://bugs.kde.org/show_bug.cgi?id=190901 http://bugs.kde.org/show_bug.cgi?id=205594 http://bugs.kde.org/show_bug.cgi?id=258196 http://bugs.kde.org/show_bug.cgi?id=283226 Diffs - konqueror/settings/kio/CMakeLists.txt e5476c9 konqueror/settings/kio/envvarproxy.ui a09450d konqueror/settings/kio/kenvvarproxydlg.h c7d2f5f konqueror/settings/kio/kenvvarproxydlg.cpp ef5c86c konqueror/settings/kio/kmanualproxydlg.h 4efa63c konqueror/settings/kio/kmanualproxydlg.cpp 6760957 konqueror/settings/kio/kproxydlg.h b2cafb8 konqueror/settings/kio/kproxydlg.cpp 2f512ce konqueror/settings/kio/kproxydlg.ui 96ff996 konqueror/settings/kio/kproxydlgbase.h 2047b90 konqueror/settings/kio/kproxydlgbase.cpp 9933698 konqueror/settings/kio/ksaveioconfig.h e895c2c konqueror/settings/kio/ksaveioconfig.cpp 62852fa konqueror/settings/kio/manualproxy.ui c69047f Diff: http://git.reviewboard.kde.org/r/102802/diff/diff Testing --- - 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. Screenshots (updated) --- No Proxy http://git.reviewboard.kde.org/r/102802/s/296/ PAC Proxy http://git.reviewboard.kde.org/r/102802/s/297/ System Proxy http://git.reviewboard.kde.org/r/102802/s/298/ Manual Proxy http://git.reviewboard.kde.org/r/102802/s/301/ Alternate dialog with all options visible http://git.reviewboard.kde.org/r/102802/s/303/ Thanks, Dawit Alemayehu
Review Request: Proxy overhaul Part 6: Proxy Configuration dialog rewrite
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/ --- Review request for KDE Base Apps. Description --- 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. This addresses bugs 82352, 139789, 142562, 145006, 147340, 164460, 189019, 190149, 190901, 205594, 258196, and 283226. http://bugs.kde.org/show_bug.cgi?id=82352 http://bugs.kde.org/show_bug.cgi?id=139789 http://bugs.kde.org/show_bug.cgi?id=142562 http://bugs.kde.org/show_bug.cgi?id=145006 http://bugs.kde.org/show_bug.cgi?id=147340 http://bugs.kde.org/show_bug.cgi?id=164460 http://bugs.kde.org/show_bug.cgi?id=189019 http://bugs.kde.org/show_bug.cgi?id=190149 http://bugs.kde.org/show_bug.cgi?id=190901 http://bugs.kde.org/show_bug.cgi?id=205594 http://bugs.kde.org/show_bug.cgi?id=258196 http://bugs.kde.org/show_bug.cgi?id=283226 Diffs - konqueror/settings/kio/CMakeLists.txt e5476c9 konqueror/settings/kio/envvarproxy.ui a09450d konqueror/settings/kio/kenvvarproxydlg.h c7d2f5f konqueror/settings/kio/kenvvarproxydlg.cpp ef5c86c konqueror/settings/kio/kmanualproxydlg.h 4efa63c konqueror/settings/kio/kmanualproxydlg.cpp 6760957 konqueror/settings/kio/kproxydlg.h b2cafb8 konqueror/settings/kio/kproxydlg.cpp 2f512ce konqueror/settings/kio/kproxydlg.ui 96ff996 konqueror/settings/kio/kproxydlgbase.h 2047b90 konqueror/settings/kio/kproxydlgbase.cpp 9933698 konqueror/settings/kio/ksaveioconfig.h e895c2c konqueror/settings/kio/ksaveioconfig.cpp 62852fa konqueror/settings/kio/manualproxy.ui c69047f Diff: http://git.reviewboard.kde.org/r/102802/diff/diff Testing --- - 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. Screenshots --- No Proxy http://git.reviewboard.kde.org/r/102802/s/296/ PAC Proxy http://git.reviewboard.kde.org/r/102802/s/297/ System Proxy http://git.reviewboard.kde.org/r/102802/s/298/ Manual Proxy http://git.reviewboard.kde.org/r/102802/s/299/ Thanks, Dawit Alemayehu
Re: Review Request: Proxy overhaul Part 6: Proxy Configuration dialog rewrite
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/#review7304 --- 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? - Christoph Feck On Oct. 13, 2011, 9:13 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/ --- (Updated Oct. 13, 2011, 9:13 p.m.) Review request for KDE Base Apps. Description --- 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. This addresses bugs 82352, 139789, 142562, 145006, 147340, 164460, 189019, 190149, 190901, 205594, 258196, and 283226. http://bugs.kde.org/show_bug.cgi?id=82352 http://bugs.kde.org/show_bug.cgi?id=139789 http://bugs.kde.org/show_bug.cgi?id=142562 http://bugs.kde.org/show_bug.cgi?id=145006 http://bugs.kde.org/show_bug.cgi?id=147340 http://bugs.kde.org/show_bug.cgi?id=164460 http://bugs.kde.org/show_bug.cgi?id=189019 http://bugs.kde.org/show_bug.cgi?id=190149 http://bugs.kde.org/show_bug.cgi?id=190901 http://bugs.kde.org/show_bug.cgi?id=205594 http://bugs.kde.org/show_bug.cgi?id=258196 http://bugs.kde.org/show_bug.cgi?id=283226 Diffs - konqueror/settings/kio/CMakeLists.txt e5476c9 konqueror/settings/kio/envvarproxy.ui a09450d konqueror/settings/kio/kenvvarproxydlg.h c7d2f5f konqueror/settings/kio/kenvvarproxydlg.cpp ef5c86c konqueror/settings/kio/kmanualproxydlg.h 4efa63c konqueror/settings/kio/kmanualproxydlg.cpp 6760957 konqueror/settings/kio/kproxydlg.h b2cafb8 konqueror/settings/kio/kproxydlg.cpp 2f512ce konqueror/settings/kio/kproxydlg.ui 96ff996 konqueror/settings/kio/kproxydlgbase.h 2047b90 konqueror/settings/kio/kproxydlgbase.cpp 9933698 konqueror/settings/kio/ksaveioconfig.h e895c2c konqueror/settings/kio/ksaveioconfig.cpp 62852fa konqueror/settings/kio/manualproxy.ui c69047f Diff: http://git.reviewboard.kde.org/r/102802/diff/diff Testing --- - 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. Screenshots --- No Proxy http://git.reviewboard.kde.org/r/102802/s/296/ PAC Proxy http://git.reviewboard.kde.org/r/102802/s/297/ System Proxy http://git.reviewboard.kde.org/r/102802/s/298/ Manual Proxy http://git.reviewboard.kde.org/r/102802/s/299/ Thanks, Dawit Alemayehu
Re: Review Request: Proxy overhaul Part 6: Proxy Configuration dialog rewrite
On Oct. 14, 2011, 12:05 a.m., Christoph Feck wrote: 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? 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. - Dawit --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/#review7304 --- On Oct. 13, 2011, 9:13 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/ --- (Updated Oct. 13, 2011, 9:13 p.m.) Review request for KDE Base Apps. Description --- 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. This addresses bugs 82352, 139789, 142562, 145006, 147340, 164460, 189019, 190149, 190901, 205594, 258196, and 283226. http://bugs.kde.org/show_bug.cgi?id=82352 http://bugs.kde.org/show_bug.cgi?id=139789 http://bugs.kde.org/show_bug.cgi?id=142562 http://bugs.kde.org/show_bug.cgi?id=145006 http://bugs.kde.org/show_bug.cgi?id=147340 http://bugs.kde.org/show_bug.cgi?id=164460 http://bugs.kde.org/show_bug.cgi?id=189019 http://bugs.kde.org/show_bug.cgi?id=190149 http://bugs.kde.org/show_bug.cgi?id=190901 http://bugs.kde.org/show_bug.cgi?id=205594 http://bugs.kde.org/show_bug.cgi?id=258196 http://bugs.kde.org/show_bug.cgi?id=283226 Diffs - konqueror/settings/kio/CMakeLists.txt e5476c9 konqueror/settings/kio/envvarproxy.ui a09450d konqueror/settings/kio/kenvvarproxydlg.h c7d2f5f konqueror/settings/kio/kenvvarproxydlg.cpp ef5c86c konqueror/settings/kio/kmanualproxydlg.h 4efa63c konqueror/settings/kio/kmanualproxydlg.cpp 6760957 konqueror/settings/kio/kproxydlg.h b2cafb8 konqueror/settings/kio/kproxydlg.cpp 2f512ce konqueror/settings/kio/kproxydlg.ui 96ff996 konqueror/settings/kio/kproxydlgbase.h 2047b90 konqueror/settings/kio/kproxydlgbase.cpp 9933698 konqueror/settings/kio/ksaveioconfig.h e895c2c konqueror/settings/kio/ksaveioconfig.cpp 62852fa konqueror/settings/kio/manualproxy.ui c69047f Diff: http://git.reviewboard.kde.org/r/102802/diff/diff Testing --- - 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. Screenshots --- No Proxy http://git.reviewboard.kde.org/r/102802/s/296/ PAC Proxy http://git.reviewboard.kde.org/r/102802/s/297/ System Proxy http://git.reviewboard.kde.org/r/102802/s/298/ Manual Proxy http://git.reviewboard.kde.org/r/102802/s/299/ Thanks, Dawit Alemayehu
Re: Review Request: Proxy overhaul Part 6: Proxy Configuration dialog rewrite
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102802/ --- (Updated Oct. 14, 2011, 5:32 a.m.) Review request for KDE Base Apps. Changes --- * Added Port: label to GUI. * Fixed bug in the code the loads proxy settings from file. Description --- 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. This addresses bugs 82352, 139789, 142562, 145006, 147340, 164460, 189019, 190149, 190901, 205594, 258196, and 283226. http://bugs.kde.org/show_bug.cgi?id=82352 http://bugs.kde.org/show_bug.cgi?id=139789 http://bugs.kde.org/show_bug.cgi?id=142562 http://bugs.kde.org/show_bug.cgi?id=145006 http://bugs.kde.org/show_bug.cgi?id=147340 http://bugs.kde.org/show_bug.cgi?id=164460 http://bugs.kde.org/show_bug.cgi?id=189019 http://bugs.kde.org/show_bug.cgi?id=190149 http://bugs.kde.org/show_bug.cgi?id=190901 http://bugs.kde.org/show_bug.cgi?id=205594 http://bugs.kde.org/show_bug.cgi?id=258196 http://bugs.kde.org/show_bug.cgi?id=283226 Diffs (updated) - konqueror/settings/kio/CMakeLists.txt e5476c9 konqueror/settings/kio/envvarproxy.ui a09450d konqueror/settings/kio/kenvvarproxydlg.h c7d2f5f konqueror/settings/kio/kenvvarproxydlg.cpp ef5c86c konqueror/settings/kio/kmanualproxydlg.h 4efa63c konqueror/settings/kio/kmanualproxydlg.cpp 6760957 konqueror/settings/kio/kproxydlg.h b2cafb8 konqueror/settings/kio/kproxydlg.cpp 2f512ce konqueror/settings/kio/kproxydlg.ui 96ff996 konqueror/settings/kio/kproxydlgbase.h 2047b90 konqueror/settings/kio/kproxydlgbase.cpp 9933698 konqueror/settings/kio/ksaveioconfig.h e895c2c konqueror/settings/kio/ksaveioconfig.cpp 62852fa konqueror/settings/kio/manualproxy.ui c69047f Diff: http://git.reviewboard.kde.org/r/102802/diff/diff Testing --- - 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. Screenshots (updated) --- No Proxy http://git.reviewboard.kde.org/r/102802/s/296/ PAC Proxy http://git.reviewboard.kde.org/r/102802/s/297/ System Proxy http://git.reviewboard.kde.org/r/102802/s/298/ Manual Proxy http://git.reviewboard.kde.org/r/102802/s/301/ Thanks, Dawit Alemayehu
