> On March 22, 2011, 6:23 p.m., John Layt wrote: > > My thoughts are if the anonymous tickbox is first focus it should be above > > the username/password fields and the username/password fields should be > > disabled if anonymous is selected. But that may look a little weird with > > the text to the left and the title below it. > > > > Or does entering a username/password automatically de-select the anonymous? > > In which case this is fine so long as the username field has the cursor > > focus and pressing enter immediately activates the ok button. > > > > Or perhaps pre-populate username with 'anonymous' and have the word > > pre-selected so it can be overtyped and do away with the tickbox entirely? > > > > Basically what I'm looking for is the easiest shortest most obvious path > > for the main use cases. > > > > </bikeshedding> > > Thomas Lübking wrote: > > the anonymous tickbox is first focus it should be above the > username/password > +1 > > > username/password fields should be disabled if anonymous is selected > the problem is about checking the box will disable the UI. > > can we maybe drop the checkbox entirely and just > QLinedit::setPlaceholderText("anonymous") and > QLinedit::setPlaceholderText("no password required for anonymous login") or > sth. like this? > > The tab order can btw be controlled explicitly and unrelated to the > actual layout. > > Dawit Alemayehu wrote: > > Or does entering a username/password automatically de-select the > anonymous? In which case this is fine so long as the username field has the > cursor focus and pressing enter > > immediately activates the ok button. > > Nope. You have to manually deselect the checkbox to enter the > username/password. Otherwise, those input widgets will be set to read only. > > > Basically what I'm looking for is the easiest shortest most obvious > path for the main use cases. > > Unfortunately, that is the problem. This feature does not solve any main > use case. Rather it seems to have been added to deal with a very corner case > in specific ioslaves, ftp being the main or only?? one. > > > can we maybe drop the checkbox entirely and just > QLinedit::setPlaceholderText("anonymous") and > QLinedit::setPlaceholderText("no password required for anonymous > > login") or sth. like this? > > Well, it seems that the anonymous checkbox was specifically added to > distinguish between the default "anonymous" login and a user entered > "anonymous" user name for the ftp ioslave. At least that is what I get > reading the commit message in the ftp ioslave that seems to be related to > this checkbox [1]. As such, I fail to see how the placeholder text approach > would satisfy what the checkbox is attempting to achieve. However, I > personally do not think that checkbox should exist at all. It is there to > handle a very corner case for very specific ioslaves (mainly ftp ioslave). > Even then it seems to be used incorrectly right now. Anyhow, I like the > placeholder idea but I will serve the same purpose as the checkbox. > > > The tab order can btw be controlled explicitly and unrelated to the > actual layout. > > Right, but the checkbox will still look out of place where it is right > now. > > > > > [1] > http://quickgit.kde.org/?p=kdelibs.git&a=commit&h=2e48eecc2fda83f08fb5f55519fe51793f536734 > > John Layt wrote: > Looking at the api and code, the Anonymous tickbox is only displayed if > the ShowAnonymousLoginCheckBox flag is set, and the default value of the > tickbox is set by calling setAnonymousMode(), so it is a corner case that is > enabled by the client. Most of the time the tickbox will not be displayed. > When it is displayed I think it needs to be more prominent as you are > effectively choosing a mode, so if you keep the tickbox it does need to be > _above_ the username field. If the anonymous mode is enabled, then the > tickbox needs first focus, if it is not enabled and no username is preset > then the username needs first focus, if the username is set or if it is > protected then the password needs first focus (most of this already happens). > > I like the placeholder idea, but I don't think it can be made to work > with the existing api, e.g. if both setAnonymousMode(true) and > setUsername("Bob") are called by the client then it won't work. > > So we just need to figure out a non-ugly way for the tickbox to be above > the Username. The ugliest part at the moment is having the tickbox in the > label column which is not HIG compliant, it needs to be in the same column as > the text entry boxes. To look even neater I'd be tempted to put a text label > to the left of the tickbox, either "Login:", or "Username:" and hide the > label on the actual Username box. Have a play :-) > > Aurélien Gâteau wrote: > Maybe using radio buttons instead of a checkbox would help? I am thinking > about something like this: > > ( ) Anonymous Login > (*) Authenticated Login > > Login: [_______________] > Password: [_______________] > > Thomas Lübking wrote: > Since we cannot avoid the checkbox, what about attaching it to the > "Username" label. > The anon-enabled login would then only differ by the preset, empty line > handling and this checkbox. > > ui dummy (lacks a little smartness regarding the pw placeholder and the > login content) > http://pastebin.com/NikPB1PJ > > Dawit Alemayehu wrote: > Hmm... I think we are now over thinking this and trying to be too smart > about it when it really should not be the case. The placement of the "Login > anonymously" should be equivalent to where the "Remember password" checkbox > is for a number of very valid reasons: > > #1. Both of them are options, not requirements, and their effects apply > equally to both inputs, i.e. the username and the password. "Remember > password" is not limited to just remembering the password. > > #2. In the normal use case, both are final acts the user would likely > perform before dismissing the dialog box. That is click on the checkboxes and > press Ok or Cancel. > > Thomas, I can tell you that your approach is a non-starter because the > login in anonymously right now is a corner case for a single ioslave (ftp) > right now and the password dialog is more generic. It is used by many > applications for different purposes. Hence, optimizing for this minor use > case does not make sense. > > Perhaps if I explain under what circumstance this checkbox will be shown > in the immediate future, it will clarify the use case for all. The only time > this checkbox will be visible to the end user is when {s}he types a username > as part of the url, i.e. ftp://<user>@<host>. Otherwise, the anonymous login > will have been automatically be tried by the ftp ioslave before the user is > prompted for credentials. In which case of course it simply means that the > anonymous login was rejected by the server. As such, there is absolutely no > point in giving the user the option to retry the same thing that failed just > now. This means the "login anonymously" checkbox will only be available to > the user under very limited circumstances ; no to mention only one ioslave at > the moment. This is why I argued it should not have been implemented in the > first place. However, whomever implemented it, saw the need/use-case for it. > > Anyhow, I just want someone to explain to me why where I placed the > checkbox in the first place is inappropriate based on what I just described > above. Anonymous login was never intended to have a prominent role as seems > to be assumed by most of you. IMO, it should by no means interfere with the > normal workings of the password dialog. That would be true if there were many > more ioslaves requiring this functionality. It is even more so when only one > needs and uses it right now. > > Thomas Lübking wrote: > From a GUI design pov the box is wrong because it adds a horizontal > structure to an otherwise pure vertical one (no, the buttonbar doesn't count > ;-) > > However, > > Perhaps if I explain under what circumstance this checkbox will be shown > On this background the entire thing is -as you suspected- flawed. > > a) Placeholders don't work since there's content (attempted login) or > (iff, in doubt) anon is for sure wrong. > b) The checkbox does NOT belong next to the "store PW" box, since that is > a persistent setting while the former one is (hopefully, the option is likely > wrong ;-) a temporary option - also it's mutually exclusive to all other UI > elements. > c) The dialog should stress that the attempted login has /failed/ and > therefore one has to > c1) provide better login data and click ok (which would ideally read > "login", problem solution #1) or > c2) suggest "Try anon login" in a pushbutton (problem solution #2) - > NOT in a checkbox. It's no longer an option but a completely other action to > resolve the initial problem. (from a user POV - that it uses the same > codepath with different parameters doesn't matter) > > Updated dummy (notice that the buttonbar is no real buttonbar, since i > don#t know how to change the text w/ designer only ;-) > http://pastebin.com/qvisH1Vv > > John Layt wrote: > I think you're missing that KPasswordDialog is supposed to be a generic > but configurable dialog for use by any application to obtain a users > credentials for whatever purposes they need, not just for use by kio slaves > logging into ftp servers. The anonymous feature was added in 2008 long > before the patch you point to tweaking its behaviour. You can't design the > dialog based solely on the single special case you know about in kio-ftp > where it is displayed in response to a failure to login as anonymous. You > have to think about what happens when an application deliberately wants to > ask the user to choose between anonymous and username login to ANYTHING and > so chooses to enable the option. The first thing needed in that case is for > the user to choose between anonymous and username, not the last. This should > be the default behaviour if anonymous is enabled, with the app able to choose > if it is selected by default or not. If it isn't selected then focus moves > to the username. > > If the kio-ftp is a special use case of the anonymous feature then it > needs to be catered for in a special way consistent with the generic use case > of the dialog. Perhaps look at using the showErrorMessage() api call to add > the message about anonymous failing, default the anonymous tickbox to enabled > but not selected, and so the username takes first focus so the user can enter > the details they need, completely bypassing the anonymous selection box while > leaving it there should they want to try again (as futile as that may seem). > This solves your use case cleanly while preserving the general case. > > Using radio button may be a solution, but I don't think it's much better > visually, you still get the mismatch between widgets with text on the left > and right sides, and it's an extra thing to tab through to firstly turn > anonymous off and then to get to the username. I like the look of the > Anonymous and Login buttons provided they fit the various sizes needed, it > solves the text alignment issue and reduces clutter, but I'd need to do > walk-through's of the scenarios to make sure it would be consistent with > existing behaviour and api options. > > You asked what was wrong with your design? It does look tidy visually > but functionally it is the wrong position. Besides it really being the first > decision not the last as I explained above, the position suggests it's a > minor option related to passwords and saving passwords. Think also of the > tab order if it's enabled and selected by default, you have to have a funny > tab order where you start at the tickbox then jump back up to the username > then down to password and then the OK, it just doesn't flow following the > natural visual path of top-to-bottom and left-to-right (or rtl) that people > expect. > > John Layt wrote: > http://i.imgur.com/THJfp.png
Please give an example other than FTP which allows anonymous login. Anonymous login is an anachronism. It's a workaround for a protocol which does not work without login. Also, kdelibs is the wrong group for reviewing this. Please take it to kde-usability. - Ingo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100920/#review2106 ----------------------------------------------------------- On March 22, 2011, 6:04 p.m., Dawit Alemayehu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100920/ > ----------------------------------------------------------- > > (Updated March 22, 2011, 6:04 p.m.) > > > Review request for kdelibs. > > > Summary > ------- > > The attached patch renames and moves the "Anonymous" box shown in the first > screenshot below the password input widgets. This change is done to > > #1. Remove the checkbox from screwing tab order when you enter > username/password. > #2. Make the purpose of the checkbox less cryptic to the end user. > > Any objections ? > > > Diffs > ----- > > kdeui/dialogs/kpassworddialog.ui 2649870 > > Diff: http://git.reviewboard.kde.org/r/100920/diff > > > Testing > ------- > > > Screenshots > ----------- > > Current Password Dialog > http://git.reviewboard.kde.org/r/100920/s/106/ > New Password Dialog > http://git.reviewboard.kde.org/r/100920/s/107/ > > > Thanks, > > Dawit > >