79188 - Offline UI: Fix the Select Folders dialog

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Backend, defect, P2)

Description

[laurel]

Using may07 commercial trunk build

Shouldn't duplicate mail/news/special folder icons in the column for the
checkbox associated with each item. It looks funny.

1.  From mail window: Edit|Mail/News Account Settings|<account name>|Offline &
Disk Space panel. 
2.  Click Select button which takes you to the select items dialog.  Note that
an icon appears both in the checkbox column and in the item name/type column. 

Result:  Looks odd and cluttered, no need for duplicate icons.

Comment 1

[jglick]

I agree it doesn't look right. 

1. Can we use the same checkboxes used by the Subscribe dialog? see attachment.
2. Can the order of the columns be reversed (Accounts/Folders first, then 
Selected) to also be consistent with the Subscribe dialog?

Comment 2

[jglick]

Attachment: Use checkboxes and column order similar to Subscribe dialog

Comment 3

[Diane Sun]

Gary,  please verify if this happens on all platform.   As we tried before,  
both Mohan and Myself can not see this on NT. Please double verify your 
configuration, etc.  or try another NT machine.

Jennifer, Offline UI is completely different from Subscribe, the sever is 
different.  Also I think  6.x  should be consistent with 4.x.  Let us not 
change the spec for this.

Comment 4

[laurel]

It happens in mac OS 9.0, I don't have the current linux build up yet to see it
there.

Comment 5

[scottputterman]

This happens on NT and Win 2000 for me as well using 2001050704.  I like it when
it's consistent with subscribe also, but getting rid of the double icons is the
important part of this bug in my opinion.

Comment 6

[laurel]

It happens in the modern skin, but not classic.

Comment 7

[Håkan Waara]

Some of the comments in this bug are quite confused. Let me shred some light in
the dark:

1. The icons are newsgroups, which doesn't make sense. Use the folder icon instead.
2. This is happening in both Modern and Classic, it's not a themes issue but
rather a xul issue. (I have screenshots coming up to prove this fact.)
3. The columns are in the wrong order; it's common sense to have
checkboxes/radiobuttons and widgets in general on the right side. Just imagine
if the buttons in a dialog was left-aligned, or the checkboxes in a prefs dialog
was to the left of the text describing it... you get the point.

I would like to know who it was that approved the checkin of this UI in the
first place?

Anyways, I'll take this bug and (attempt to) fix it since the owner is obviously
not willing to.

Comment 8

[Håkan Waara]

Attachment: How it looks using the Classic theme

Comment 9

[Håkan Waara]

Attachment: How it looks using the Modern theme

Comment 10

[scottputterman]

I do prefer jglick's screenshot from 5/7. Having the folder tree on the left
makes it look a lot better. I think we should still use the newsgroup icon for
newsgroups and all of the special folder icons for special mail folders.

Comment 11

[Håkan Waara]

Putterman: indeed we should. the newsgroup icon I am referring to is next to 
the checkbox in Modern though. That one is redundant and a bug.

(BTW: that redundant newsgroup icon is not next to the checkbox in Classic - as 
laurel truthfully states. Sorry...)

Comment 12

[scottputterman]

Yeah, we have to get rid of those!

Comment 13

[scottputterman]

Håkan, do you think you'll be getting to this in 0.9.1?

Comment 14

[Håkan Waara]

Laurel, can you attach a screenshot if this happens to you still?

I might be able to fix this for 0.9.1, putterman.

Comment 15

[laurel]

Attachment: screenshot win98 may22 build

Comment 16

[chris hofmann]

let's land this in 0.9.2. let drivers know if this creates problems. thanks.

Comment 17

[scottputterman]

Håkan, if you have time to fix this for 0.9.1, or you don't and someone else
does, I'd be more than happy to argue for getting this in.

Comment 18

[Håkan Waara]

Moving back to 0.9.1, I have a fix and some general UI cleanup for this
amazingly ugly looking dialog for you all to enjoy.

cc hewitt, he will review when I attach my patch. putterman, please argue away.

Comment 19

[Håkan Waara]

oops, forgot to cc hewitt. :)

Comment 20

[Håkan Waara]

Alright. Now I have a patch ready and a screenshot for the UI people.

Coming up next...

Comment 21

[Håkan Waara]

Attachment: Screenshot after my modifications

Comment 22

[Håkan Waara]

Attachment: patch

Comment 23

[Håkan Waara]

This is what my patch does (to help reviewers):

* Move columns, and cells
* Make the window smaller (it was huge before on smaller screens)
* Fix the additonal image that was merged with the checkbox before
* Make the columns' width persistant
* Make the columns' default width's better
* Remove commented-out code
* Remove unnecessary styling (talking about the <grid>)

Please r= and sr=

Comment 24

[Blake Ross]

mpt or a technical writer should review the wording (for one, "Selecting more 
items may increase download time" sounds fishy.  More items than what?)

I don't think that should say "Offline Item Name".  They're not necessarily 
offline items (although they potentially are).  And "Selected" doesn't really 
say much about the other column (nor are the items necessarily selected, 
although they potentially are).  Also, I think the checkboxes should be on the 
left of and closer to the items, pursuant to the common layout of a task list, 
and so that people don't have to try to line up the checkboxes with the proper 
items (which would be more of a problem if this dialog was resized).

The titlebar text seems a bit long and nearly duplicates the (probably too 
long) intro text, but again, I'll let a tech writer comment on that...

Comment 25

[Blake Ross]

It looks like you didn't touch the areas I mentioned, so you don't have to fix 
them, but it would be greatly appreciated (in this or another bug).

Comment 26

[Håkan Waara]

Blake, please file a separate bug on me on that issue. We should avoid to edit
wordings during the l10n freeze.

Comment 27

[scottputterman]

I won't comment on the wording, but I like the checkboxes on the right.  jglick,
just want to make sure you're still happy with this.

Comment 28

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

adding myself.   I'll do the test & review.

yes, please open a new bug on the other issues.

Comment 29

[racham]

Changes look OK to me.

But, I don't have cycles to test it right now..Sorry about that.

Please do get Jennifer's OK on reduced sizes and the layout rearrangement.

r=racham

Comment 30

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

sr=sspitzer.  I tested it and it seems to work fine.

excellent clean up work, hwaara.  the dialog looks much, much better.

I'd leave the default height and width alone (450 x 400).  your defaults make 
the window to small for me (and require a resize).

no matter what we go with, we should add persist="width height" to the 
msgSelectOffline.xul so that if the user resizes, it will stick.

we could also make it so this dialog comes up in the center of the screen.

I'll attach a revised patch with these suggestions.

Comment 31

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

Attachment: hwaara fix, plus persistance, 450 x 400, and centering the dialog.

Comment 32

[Håkan Waara]

r=hwaara on the few changes Seth made. r=racham on all my changes. sr=seth on
everything.

Asa, I need approval to check it in.

Comment 33

[Christopher Blizzard (:blizzard)]

a=blizzard for 0.9.1

Comment 34

[Joe Hewitt (gone)]

Hold on a second before landing this...

Two problems:

1. putting style="list-style-image: none" on the checkboxes is a no-no.  Don't
use the style attribute like that.  You should do that in theme CSS.

2. I would propose that the checkbox column have no flex on it at all, and the
right column have flex="1".  This is because the checkbox column should be only
as wide as the checkbox, and the right column should fill in the rest.

Comment 35

[Håkan Waara]

Attachment: adjusted patch with hewitt's nits fixed

Comment 36

[jglick]

Agree this is much nicer looking! Thanks for fixing. :-)

Agree wording needs to be fixed.  Please cc me and [e-mail] on the 
new bug for that.

Comment 37

[Håkan Waara]

Hey, I need a new sr= and r=

Any takers?

Comment 38

[Joe Hewitt (gone)]

Looks good, except for one thing:

You put the flex="1" on the wrong column.  Shouldn't the folder name column
flex, while the checkbox column should not?

Fix that and you have sr=hewitt

Comment 39

[Håkan Waara]

Still waiting for the tree to open...sigh

Comment 40

[Håkan Waara]

er, I mean for the smoketest blocker to get fixed.

Comment 41

[Håkan Waara]

Fix is in. Woo!

Comment 42

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

heads up, the last patch cause a regression.  (see bug #82695)

flex="1" was on the wrong column, making the checkbox column take up the width, 
hiding the folder names.

Comment 43

[grylchan]

testing commercial builds
 2001052904 on Win NT 4.0
 2001052908 on Linux 2.2 Red Hat 7.0
 2001052908 on MAC OS 9.0.4

Tested both Classic and Modern themes.

Verified that the "select folders dialog" no longer
has the double icons. 

Visually every thing looks fine. Good job guys.

marking as verified.