79249 - UI problems with the per-server LDAP override UI

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Account Configuration, defect, P1)

Description

[(not reading, please use [e-mail] instead)]

UI problems with the per-server LDAP override UI

1)  it's tacked on at the end of the "server" options for an account.  this
make the "server" options page too long so the LDAP override UI gets cut off.  
perhaps it needs to be hidden in some advanced dialog?  (cc jglick, for a link 
to the spec)

2)  when resizing, the LDAP override UI doesn't behave right.  it should be 
"attached" to the element above it, not the bottom of the window.  here comes a 
screen shot.

Comment 1

[(not reading, please use [e-mail] instead)]

Attachment: the account mananger window streched, notice the gap

Comment 2

[Håkan Waara]

This does not look good. I talked with dmose and mpt about this for a few days
ago, cc'ing.

Comment 3

[scottputterman]

nominating for nsbeta1

Comment 4

[Jussi-Pekka Mantere]

Priority P2, TFV 0.9.1.

Comment 5

[(not reading, please use [e-mail] instead)]

the per server LDAP override UI seems advanced to me.  since space is at a
minimun, any objections to adding a "Advanced..." button and moving this (and
other advanced server settings) to it?

jglick / mpt?

Comment 6

[Matthew T (active 1999-2002)]

I suggest the following changes to this panel, some of which are just to make 
it more understandable, but some of which will also help it fit in 640*480:

1.  Put the `Port:' field to the right of the `Server Name:' field. These two
    are highly related, and it makes no sense for them to be separated by the
    `User Name:' field.

2.  Rename the `Server Type:' field to `Type:' (to remove redundancy), and
    place it below the server name, not above it.

3.  Remove the `Server Settings' group box, which is redundant with the name of
    the panel itself.

4.  Rename `Limit message download to [  ] kB per message' (which sounds like
    it might truncate the message!) to `Only download messages which are [  ] K
    or smaller'.

5.  Change `Empty Trash on Exit' to `Empty the Trash folder when I exit
    Mozilla', and remove the stray padding above it.

6.  Change the `Local directory:' field to `Local copies are stored in:', and
    put the field to the right of the label instead of below it.

7.  Remove the <spring> (I assume that's the culprit) which is pushing the LDAP
    settings down to the bottom of the panel.

8.  Remove the `LDAP Directory Server' group box, which is redundant with the
    label for the checkbox.

9.  Change the checkbox to two radio buttons, `Use my global LDAP server
    preferences for this account', and `Use a different LDAP server:'.

10. Put the LDAP server popup menu next to the second radio button, with no
    extra label.

Now, if those changes don't make enough room (and I think they should be made 
even if they don't make enough room), then you will indeed need a sub-dialog. I 
would suggest labelling the button for such a dialog as `LDAP Options ...', 
rather than the mysterious-sounding `Advanced ...'.

Comment 7

[timeless]

4. `Do not download messages larger than [   ] K'

What does this have to do w/ the bug summary 'LDAP'?

Comment 8

[jglick]

Attachment: proposed idea

Comment 9

[jglick]

I like some of mpt's cleanup ideas. 
Above is a minimal changes idea.

Comment 10

[(not reading, please use [e-mail] instead)]

now that we have twisties, why not show "LDAP" as a settings child of the server?

Comment 11

[jglick]

We could do that too, but we'd have to distinguish between "Mail Server" and 
"LDAP Server" in the list.

Comment 12

[(not reading, please use [e-mail] instead)]

or call it "Addressing Settings" or whatever.  those aren't the LDAP server
settings, it which LDAP server to use for autocomplete when using that accounts
identity, right?

Comment 13

[Dan Mosedale (:dmosedale, :dmose)]

I especially like the last couple of suggestions by sspitzer and jglick.  This
would help us avoid the "maze of little dialogs" which is currently required.

Comment 14

[jglick]

Attachment: On a separate panel and radio buttons (as suggested by mpt)

Comment 15

[Srilatha Moturi]

I like the idea of having a separate panel for LDAP, but the panel looks pretty 
empty.
Few suggestions:
1) we can name the panel "Addressing" instead of "LDAP Directory"
2) Enclose all the contents in a box.
3) Move the dropdown listbox and Edit Directories button to the next line.

May be we can add some addressing prefs in here!!

Comment 16

[jglick]

Robin, can you suggest a name for the panel?

A group box really isn't necessary since everything on the panel would be in 
that group box.

Comment 17

[Marek Z. Jeziorek]

Setting target milestone to 0.9.2 (check it in anytime, even before, when the
tree is open for). Per PDT triage.

Comment 18

[Srilatha Moturi]

I think enclosing it in a box will make it look better.
Jennifer do we have a name yet?

Comment 19

[jglick]

"Addressing" to match the global pref is probably fine.

Comment 20

[robinf]

Agree.

Comment 21

[Dan Mosedale (:dmosedale, :dmose)]

*** Bug 78979 has been marked as a duplicate of this bug. ***

Comment 22

[Srilatha Moturi]

The override global preference saved fine using the checkbox. But now based on 
the new spec, using the radiogroup the preference is not getting saved right
This is the code I have
<radiogroup orient="vertical" wsm_persist="true" 
id="identity.overrideGlobalPref"  pref="true" preftype="bool" 
prefattribute="value">
 <radio group="identity.overrideGlobalPref" id="useGlobalPref1" value="false" 
label="&useGlobal.label;"/>
 <radio group="identity.overrideGlobalPref" id="useGlobalPref2" value="true" 
label="&directories.label;"/>
</radiogroup>

clicking on the first radio button does not save a value of false.
Any suggestions?

Comment 23

[Srilatha Moturi]

Jennifer, since I cannot get the radio group to work, can we use checkbox 
(like before) instead of radiogroup.

Comment 24

[Srilatha Moturi]

Copying Bhuvan for sugeestions.

Comment 25

[jglick]

If possible, and especially since we have the space now, I think the radio 
buttons make the choices much more understandable for users.

Comment 26

[Srilatha Moturi]

Attachment: this does not save the pref correctly.

Comment 27

[Srilatha Moturi]

Attachment: Implementation based on the spec

Comment 28

[Srilatha Moturi]

Attachment: patch v2

Comment 29

[Srilatha Moturi]

The latest patch has attributes fro pref locking and I made a minor modification 
like indentation for radiogroup.
ccing Mohanb for review.
Bhuvan can you take a look at the patch too.
Seth can you sr

Thanks to serge, edyyk and mohan in helping solve the radiogroup problem.

Comment 30

[Mohan Bhamidipati]

Can you post this patch at "mailnews" level instead of at "mozilla" level? 

Comment 31

[Srilatha Moturi]

Attachment: patch created from mailnews directory

Comment 32

[Mohan Bhamidipati]

r=mohanb; 

Comment 33

[(not reading, please use [e-mail] instead)]

can you attach a screen shot so I can see how it looks with your patch?

Comment 34

[Srilatha Moturi]

Attachment: screen shot of the preference panel

Comment 35

[(not reading, please use [e-mail] instead)]

thanks for the screen shot.  that looks much better than the old way.

sr=sspitzer

cc'ing msanz / nhotta.

I think you'll need L10n approval before checking in, because of the string changes.

Comment 36

[Michele Carlson]

Sorry I can't give approval for any UI changes at this point. If you change this
now it will be in English in all of our localized builds. Please check it in
after BETA.

thanks

Comment 37

[Jussi-Pekka Mantere]

The beta is on 0.9.1 branch. The trunk is "after beta." So, it's OK to check
this in before the RTM UI freeze.

Comment 38

[Håkan Waara]

Mcarlson, note that those rules only apply to the NS tree. One can still fix
this on the mozilla tree, as long as we get approval from drivers.

Comment 39

[Blake Ross]

* This stuff should be in a groupbox, right?
* Server is capitalized in the second label but not the first.
* In other bugs, we seem to have decided second-person ('you', not 'my')
* Not much content for a panel, what else is planned to go here?

Jennifer, Matthew, German, Ben, others, what do you all think?

Comment 40

[Srilatha Moturi]

* This stuff should be in a groupbox, right?
Comments from Jennifer:
  A group box really isn't necessary since everything on the panel would be in 
that group box.

* Server is capitalized in the second label but not the first.
  This is my mistake. Server is not capitalized in the spec . Will attach a new 
patch with the changes.

Comment 41

[jglick]

Groupbox - it doesn't really need the groupbox since its the only 'group' on the 
panel, but I don't have a problem with having a groupbox around the items if 
folks prefer.

Capitalization - fixed by Srilatha.

You vs My - I don't think this is a problem, but then I'm not a tech writer! :-)
cc'ing Robin's co-worker while she's out. [e-mail]

Not much content - Yeah, I know, but it wouldn't fit on existing panels as you 
can see from the examples early in the bug.  As for future stuffs, the LDAP 
folks might know that one.

 

Comment 42

[Jatin Billimoria]

Although there isn't a consistent standard in the preferences, using the
first-person "my" in this pref panel seems appropriate. "Me" and "my" are used
in other places in preferencs as well.

Comment 43

[Srilatha Moturi]

Attachment: Patch with capitalisation fixed.

Comment 44

[Srilatha Moturi]

The stuff that might be added to this panel in the future:
We want to add the ability to select more than one server for autocompletion. So 
the dropdown list box might be changed to a list. May be some more addressing 
preferences.

Comment 45

[Srilatha Moturi]

Attachment: Here's the complete patch.

Comment 46

[Mohan Bhamidipati]

r=mohanb;

Comment 47

[Linda Baliman]

Adding DE Team to cc-list

Comment 48

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 49

[Srilatha Moturi]

Fix checked in.

Comment 50

[Srilatha Moturi]

marking this as fixed

Comment 51

[yulian chang]

2001061304 Wins trunk build
Verified the fix for the per-server LDAP override UI