82773 - Improve the UI for the "Edit Filter" dialog

Status: VERIFIED FIXED

(MailNews Core :: Filters, defect, P1)

Description

[Håkan Waara]

I hate to say this after assumably all the hard work behind this one dialog, but
the UI in it, from the spec doesn't look so good. IMHO.

If there is any way we could make it look better, I could be the implementor.
All I need is some sort of spec and a agreement between the CCs.

Jennifer, you are free to mark this INVALID. Now you know at least... :(

Comment 1

[Matthew T (active 1999-2002)]

Glorious wide-screen ASCII art coming shortly.

Comment 2

[Matthew T (active 1999-2002)]

Attachment: Preliminary mockup (ignore the nested stuff for now, that's bug 59821)

Comment 3

[Håkan Waara]

Ignore the nested stuff... Can you attach a mockup which doesn't have the nested
stuff?

What does the "[ + ]" things mean?

Comment 4

[Matthew T (active 1999-2002)]

Attachment: Version 2 - simple boolean logic this time, and `+' changed to `More'

Comment 5

[Håkan Waara]

Mpt: I think it makes more sense to have a global "Delete" button that deletes
the currently selected row instead of a delete button ("[ X ]") on each row.

Comment 6

[Alex Bishop]

I thought that [ X ] represented a checkbox (to enable/disable the filter).

Comment 7

[Håkan Waara]

From IRC session:

<hwaara> mpt: what's [ X ]?
<mpt> hwaara: It deletes the current row

Jglick, I am interested in your input here.

Comment 8

[Matthew T (active 1999-2002)]

It's at this point that Timeless is supposed to jump in and point out, as Håkan 
has already pointed out, that having separate command buttons to perform the 
same function for each row is quite clumsy. (See, for example, the discussion 
starting with Brian Stell's 2001-05-10 comment in bug 28899.) You should be 
able to select the row, and then hit an omnipotent `Delete' button to delete 
the criterion.

Trouble is, I can't think of a nice way of doing that, since the row is full of 
UI gadgets so it's not very selectable. About the best thing I can think of is 
to do what spreadsheets do, and have a clickable row header for selecting the 
row at the left edge. As for 4.x, it ignores this problem completely -- you can 
only delete the last criterion in the filter, so if you decide you want to keep 
all criteria except the first one, you're totally stuck.

Anyway. Håkan wants a less major cleanup design, which doesn't involve changing 
the contents of the criteria overlay. Coming right up ...

Comment 9

[Matthew T (active 1999-2002)]

Attachment: Version 3 -- mainly clearing out unnecessary text, with a little bit of forward compatibility for complex boolean filters thrown in

Comment 10

[Håkan Waara]

I have fixed the following:
* Implemented mpt's spec v3.
* Removed unused code

Two images coming up. One post-fix and one pre-fix to see the difference. 

(Except for that the new window is cleaner, note how considerably much smaller
it is!)

Comment 11

[Håkan Waara]

Attachment: Before

Comment 12

[Håkan Waara]

Attachment: After

Comment 13

[Håkan Waara]

Actually, I will reassign this bug to myself since I have a fix and all...
Though I'm still waiting for jglick's comments to the screenshot.

Comment 14

[Håkan Waara]

reassign

Comment 15

[timeless]

Actually, at first I was going to ask how to take multiple actions. Later I was 
going to ask what new... does.  If it's add an additional action I think new... 
both is the wrong word and has the wrong style (no ellipsis should be used).

The only way I can see to implement global buttons is to the right edge, and 
I'm sure no one would like that
<vbox><button label="Add"/><button label="Remove"/><button 
label="Up"/><button label="Down"/><button label="Left"/><button 
label="Right"/></vbox>

Oh I was going to ask in the tree form how the user creates a non leaf node 
(or a leaf node depending...).

[my awful proposal uses left to take the selected rule and make it a sibling to 
its parent, and right to make it either a child of its preceding sibling or a 
child of a new parent where it used to live.  Clearly this isn't ideal but it 
would work -- if we went this way I'd expect some other names for the buttons].

mpt: wrt selectable, the left edge where there are path widgets should be 
selectable.  (keys such as + -, del, insert should probably work)

Comment 16

[Håkan Waara]

"New..." is for new folder. I changed this in my latest fix, to be "New
folder..." (ellipsis because it doesn't create one instantly but opens up a
dialog first.)

Some other few things I'm working on / I have changed:
* The title should be "Edit filter <what the filtername textfield contains>".
I'm working on it, to do as msgcompose does with live updating when you edit the
subject.

* Putterman said that he doesn't like the placement of the "Fewer" and "More"
buttons, they should be above the tree, he said. I don't know what to do here.

Comment 17

[Matthew T (active 1999-2002)]

Timeless, I don't think we currently have the back-end ability to perform
multiple actions on a filtered message, much as that would be cool -- file an
RFE for that, there doesn't appear to be one already.

Håkan, now that I see your screenshots (and how the dialog currently works), I
see that you can already select a criterion as a whole. So, could you replace
`More' with `Insert Row', and replace `Fewer' with `Delete Row'? `Insert Row'
would add a criterion below the selected one. `Delete' would delete the selected
criterion, solving the 4.x problem I described in my previous comment. (Or you
can just leave that for a different bug, if you like.)

Comment 18

[scottputterman]

I was wondering why no one responded to my comments.  I hope they just didn't go
through and that I didn't post them to a different bug!

I'll try this again.

1.  I think it should be New Folder (looks like hwaara changed that).
2.  I liked More and Fewer below the rules more than above. They seem closer to
what they are affecting when they are below.  Though if you have selection
working and can make inserting and deleting work, then mpt's later comments
regarding insert/delete seem pretty interesting to explore.
3.  I think mpt's proposal of removing the group box around the Actions as well
as the reduction in wording makes it look less cluttered.  I haven't thought
about the wording enough to know if the reduction in it makes sense yet. 

Also, I don't think any of this has been looked at by jglick yet so although
it's great to work on making improvements you might want to wait for some
comments from her before proceeding too far.    

Comment 19

[timeless]

Would 'add criterion' and 'remove criterion' be better than the generic 
'insert row' and 'delete row'?

hwaara: for the current proposal i'd suggest removing the whitespace in the 
listbox. it was originally their because we had the text 'and' or 'or' between 
criteria.

mpt: i really can't believe there isn't an RFE for multiple actions.  But if I 
can't find one I'll file.

um, about 'my' [blake loves that word] 'Inbox', are filters per server? and is 
it possible currently to move a filter from one server to another? -- if this 
gets too far afield or if my question isn't clear enough, i'll file a new bug.

Comment 20

[jglick]

Comments on attachment id=36255:

I think removing the group boxes "Conditions" and "Actions" is a mistake. 
Without them, the items on the window just get mixed together and its not really 
clear which widgets are for setting up conditions and which are for defining 
actions.  Past feedback has shown that the Filter Rules dialog from 4.x is one 
of the most difficult for users to understand how to use. Attachment 36255 [details] looks 
very similar to the Windows 4.x Filter Rules dialog: no grouping of related 
items and no descriptive text.  

The location of the More and Fewer buttons look like they are related to the 
drop down menu to the left, not the text field below. By grouping the Conditions 
in a groupbox and having the More/Fewer buttons in the lower left corner, the 
relationship is more clear.

Using a dropdown menu instead of radio buttons for the AND/OR selection isn't 
recommended.  When you only have TWO MUTUALLY exclusive choices, radio buttons 
should be used. A List box is for diplaying a large number of choices that vary 
in number or context. 

If the goal was to make this dialog smaller, I don't think the gained space 
is a benefit over the grouping and description that has been lost.

If the goal was to add the function of being able to add or remove condition 
rows, change More to Add and Fewer to Remove.  "Add" would add an additional row 
(would be added to the end if no row is highlighted, and added below a row if 
one is currently selected).  Remove would only be enabled if a row is selected 
and would removed that particular row.  

Comment 21

[jglick]

Attachment: Change More/Fewer to Add/Remove

Comment 22

[Alex Bishop]

For what it's worth I think the last screenshot is the best. It looks less
confusing and the layout makes it clear what to do.

Comment 23

[Matthew T (active 1999-2002)]

putterman: The initial reason for moving the More/Fewer buttons from the bottom 
to the top was forward compatibility with bug 59821. In my initial (1998) 
design for this UI, every row could potentially have a `More' button 
determining the number of nested child rows it had (the button would appear if 
you chose `all of the following:' or `any of the following:' from the popup 
menu, whereupon the row would gain children). If the `More' button was at the 
bottom of all the child rows, there would be three problems: (1) we'd be taking 
up a whole extra row for each `any of the following:'/`all of the following:' 
parent, (2) in some cases you'd have to scroll (!) to get from a parent row to 
the widget allowing you to insert more children into that row, and (3) the 
shifting-More-button problem (suffered by 4.x, and by Sherlock 1.0) would be 
back with a vengeance.

All of that is solved much more nicely with the rows themselves being 
selectable, since we can have a single `Insert Row' button and a single `Delete 
Selected Row' button. I still think these buttons are still logically connected 
with `the following' in `{all|any} of the following:', and putting them in the 
same row as that popup menu also saves a fair bit of space in the dialog; but I 
could go either way on that.

Timeless: No-one knows what a `criterion' is. :-) You are certainly correct 
that most of the white-space should disappear -- but that's probably a 
different bug which is theme-dependent. (E.g. in Mac Classic, there should be 4 
pixels of horizontal whitespace between the controls on each row, and 6 pixels 
of vertical whitespace between the controls on adjacent rows.) However, there 
should be a decent square of whitespace at the start of the row, for the user 
to click on the row to select it.

jglick: Ok, so what did the 4.x filter editing UI do wrong? Here's the problems 
I can see:
(1) It combined the filter list and the filter editing UI into a single window,
    which was probably a bad idea.
(2) It used a too-small (9px) font throughout, making the UI appear complex and
    cramped.
(3) It had even less padding between the last criterion row and the action row
    (5px, should have been 12px) than between criterion rows (7px, should have
    been 6px), confusing the user about which controls were related.
The first two problems are already fixed. The third is for hewitt and andreww 
to fix, and is outside the scope of this bug.

> Using a dropdown menu instead of radio buttons for the AND/OR selection isn't
> recommended.  When you only have TWO MUTUALLY exclusive choices, radio
> buttons should be used.

That's very true. However, most of the popup menus *will* have more than two 
options:
+-----------------------+
| subject               |
| sender                |
| body                  |
| date                  |
| priority              |
| to                    |
| to or CC              |
| age in days           |
| Other ...             |
|-----------------------|
| all of the following: |
| any of the following: |
+-----------------------+
It will only be the top-level row (currently outside the listbox) which has two 
options, and I don't think it's worth using an inconsistent control for that 
one just because it has fewer options than the others.

Comment 24

[timeless]

mpt: when we switch to tree form we should move the all/one inside the tree so 
you could have a single criterion or opt to select all/one of.

And you're the one who first used criterion... *shrug* the more/fewer / 
add/remove things seem awkward...

do we really prefer having the buttons below instead of say to the side? i'd 
prefer to trade their position for an extra row or two.

Comment 25

[Håkan Waara]

The placing of the More/Fewer buttons is, to me, a trivial problem. I don't
really care about that.

However, the best part of the fix is that it removes clutter! Some people,
including jglick, seems to be very confused and/or unfamiliar with this new
design but I know it's for the best.

Jglick, you say that this dialog is one of the most difficult dialogs to
understand for users. And thus your proposed solution is to add groupboxes and
more descriptive text?

IMHO, that's the completely wrong way to make the dialog less cluttered and
easier to understand. As I pointed out in the bug about the new Offline prefs
panel: the more "descriptive" text you have, the more for the user to read, the
more confusion and even more difficulty (apart from the interface itself) for
the user to understand the UI.

I know that this last design might seem strange to you, but I strongly believe
that the best solution for making difficult dialogs like this easier to
understand is to _remove_ or shorted down descriptive text rather than adding
more of it.

With the last screenshot, the initial look would be less cluttered so the user
wouldn't have to read lots of descriptive text. If the user feels that it
doesn't have to, then it ends up reading those words that are already there.

Comment 26

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

weighing in here, as the module owner for the mailnews UI:

let's continue the discussion.  but don't implement any changes until we have
buy-off from jglick.   she owns the spec and is the designer of the mailnews UI.

implementing it before she buys off will waste your time and lead to patch bit-rot.

We should only proceed to re-implement *if* your suggestions persuade her to
change the spec.

but until then, this should live as a discussion in bugzilla.

Comment 27

[Håkan Waara]

Sspitzer, Putterman: you are repeating yourself over and over about this buy-off
thing. I know that jglick owns the UI. What do you think I am doing? - I am
discussing this with jglick, am I not?  jesus.

Comment 28

[Håkan Waara]

Let's move on. 

Jglick?

Comment 29

[Matthew T (active 1999-2002)]

Now, let's not get over-excited here.

I think we can all agree that where the quality of language used is constant,
the proportion of people who can use a dialog effectively follows a Poisson
distribution against the number of words used in the dialog. When you have zero
words you have zero comprehension, and this rapidly improves as you add more
words. Soon, however, you get to a peak of comprehension, after which adding
even more words makes the proportion of people who can use the dialog fade away
towards zero again -- they can't be bothered reading all the text, so they try
to work it out by reading only some of the words, and eventually the dialog gets
so large and apparently complex that they can't be bothered trying to understand
it at all.

Now, where we disagree is where that peak of comprehension is. On one extreme we
have Håkan, who takes Joel Spolsky's `users can't read anything, and if they
could, they wouldn't want to' maxim to heart, and tries to cut UI wording to the
bone. At the other extreme we have Robin, who likes injecting sentences of help
text wherever they'll fit. In between we have me, Seth, Jennifer, and putterman,
in approximately that order. As usual, the only way to tell who is right is to
do a usability test -- not the cheap kind of test which finds problems with a
small number of users, but the expensive kind of test which compares alternative
solutions with a large number of users of varying abilities.

However, I think it's fairly safe to say that where a dialog uses the same noun
four times (e.g. `conditions' in this bug, or `Proxy' in bug 83104), that's
probably a symptom of redundancy; and if a writer feels the need to use
CAPITALIZED words in a user interface (`ALL of the following', `any ONE of the
following'), that's a symptom of something gone bad in the UI too.

Comment 30

[Matthew T (active 1999-2002)]

[reopening -- no idea how that happened ...]

Comment 31

[Håkan Waara]

Jglick, if you could respond what you agree/disagree with here in my, mpt, Alex
Bishop and timeless' comments that would be great.

Comment 32

[jglick]

ok, some new screenshots to follow.  Attempts to follow the suggestions of less 
words and no groupboxes.  :-)

Comment 33

[jglick]

Attachment: Less words, no groupbox, Add/Remove

Comment 34

[jglick]

Attachment: same as above except More/Fewer in case highlighting lines won't work

Comment 35

[jglick]

Attachment: no list box, dialog would grow as needed

Comment 36

[jglick]

Attachment: similar to above with + and - instead for each line

Comment 37

[Håkan Waara]

Jglick, I like attachment 36879 [details], since that is already implemented at the
moment. And it would look weird with a dialog that auto-resized itself (for
filters with many criteria, to a very large size).

Are you still convinced that:

* Words should be capitalized like this: "Subject Contains", and "Move to Folder"?
* We should have radiobuttons instead of a popupmenu for the all/any option?
* The More and Fewer buttons should be at the right of the listbox, which may
give the user the assumption that they "belong" to the criteria lines?

Thanks for these new mockups!

Comment 38

[Alex Bishop]

My favourite is attachment 36877 [details]. I think, however, that 'Add' and 'Remove'
could be changed to 'Add Condition' and 'Remove Condition' so it's clear exactly
what the user is adding and removing. I also think it would make more sense to
have the buttons at the bottom (can't exactly explain why, it just seems
better). With regard to attachment 36881 [details], I think using + and - would be
probably be too confusing for the average user. I believe the all/any option
should use radio buttons because it allows the user to see both possibilities
straight away and reminds them that they can switch between the two. As for the
capitalisation, I'm not sure. Essentially, the user is creating a sentence for
each condition, so lowercase would be more logical. On the other hand, it might
look strange.

Comment 39

[Håkan Waara]

I agree that having the buttons Add/Remove (or whatever they'll be called) to
the side of the tree is not good. I don't mind whether they are below or above
the tree, but please don't have the at the side.

I think something short, like "Add" instead of "Add condition" is good, because
otherwise the buttons will be very wide which will force the dialog to be even
wider (note that having the radiobuttons instead of popupmenu makes the dialog
quite wide already.)

Comment 40

[Alex Bishop]

I don't see how the buttons 'Add Condition' and 'Remove Condition' could make
the dialogue wider if they were below the listbox. In any case, I think having a
slightly larger dialogue is a worthwhile price to pay for making it simpler to
understand.

Comment 41

[Håkan Waara]

I'm quite eager for a decision here...

Comment 42

[jglick]

So far attachment 36879 [details] and attachment 36877 [details] are the preferred of the four. I 
agree.  I don't know how feasible the highlighting of a row in attachment 36877 [details] 
is though and also not sure how obvious it would be to users how to select a 
row, which makes me inclinded to lean toward 36879.

>Are you still convinced that:
>* Words should be capitalized like this: "Subject Contains", and "Move to 
>Folder"?

You are probably right on this one, that initial phrase caps only is fine. 
Cc'ing Robin's co-worker while she is out on sabatical since grammer is not my 
strong point. :-)  [e-mail], are inital word caps or inital sentence 
caps appropriate here?

>* We should have radiobuttons instead of a popupmenu for the all/any option?

Yes, i still feel strongly about this one. There are only Two mutually exclusive 
options here, which to me says 'radio buttons'. Also for the good reasons Alex 
mentioned.

>* The More and Fewer buttons should be at the right of the listbox, which may
>give the user the assumption that they "belong" to the criteria lines?

The More/Fewer buttons Do belong to the criteria lines. They add more or reduce 
the number of criteria lines visible.  My concern is that without the group box, 
it won't be as obvious which widget the More/Fewer buttons belong to if they are 
 above or below the list box.  But I will attach a screen shot of the buttons 
below the list box for comparison.

Comment 43

[jglick]

Attachment: More/Fewer below list box

Comment 44

[Håkan Waara]

I for one am very satisfied with the latest mockup. This will greatly improve
the Filters UI.

However, I have one nit. I don't like the capping "Move to Folder", "Subject
Contains", "For any incoming message that match -- All of the following" etc.

I think it's much easier to read if you're not captitalizing everywhere, it's
like reading it all in one sentence then.

Comment 45

[Håkan Waara]

I mean, seriously, how does this look: "Age In Days Is Greater Than". It's not
some newspaper heading we're talking about! ;)

Comment 46

[jglick]

I agree.  Initial sentence cap is probably appropriate.  "Move to folder", etc. 
jatin, you agree? 

Comment 47

[Håkan Waara]

That's great. I'll make a new patch with all the suggestions we agreed on since
the last screenshot from me.

Comment 48

[Håkan Waara]

Ok people, I had to rewrite the whole patch because I messed up. I'm close to
having a final patch for review that addresses concensus.

I really want this in for 0.9.2

Comment 49

[Håkan Waara]

Attachment: screenshot

Comment 50

[Håkan Waara]

Attachment: fix

Comment 51

[Håkan Waara]

Ok, a screenshot and an according patch is posted.

This will clean up a lot (code-wise and UI-wise) and will be a good kickstart
for further improvement of the UI. After this I'll hopefullt fix even more bugs
(we have a ton on the filters UI).

Bhuvan, can you please review?

Comment 52

[Håkan Waara]

Argh, bhuvan wasn't CCed!

Bhuvan, can you review this patch as my previous comment suggests?  Thanks!

Comment 53

[jglick]

Håkan, looks nice. A few comments.

Name of dialog should be "Filter Rules".

Is it possible, and what do you think of, the listbox and More/Fewer buttons 
indented like in attachment (id=37559) to make it more obvious that they belong 
to the "For incoming messages..." part.

A touch more space between the More/Fewer buttons and "Perform this action" so 
it is more clear they are separate things.

Comment 54

[Håkan Waara]

Jglick, if you mean the rows in the tree being indented - that's ugly, and I've
tried to fix it with this bug but failed. :(

Can you please log a separate bug about that?

I will increase the space between the More/Fewer buttons and "Perform this
action" text in my final fix.

Comment 55

[jglick]

>if you mean the rows in the tree being indented - that's ugly, and I've
>tried to fix it with this bug but failed. :(

The blank space at the beginning of each line? Yeah, that is ugly, but i was 
actually referring to something else.  What you are talking about has to do with 
the "and" "or" text space. Thought there was a bug on that already?

See attachment below for example of what I am referring to.

Comment 56

[jglick]

Attachment: example

Comment 57

[Joe Hewitt (gone)]

nice work.  sr=hewitt

Comment 58

[Håkan Waara]

Attachment: New screenshot

Comment 59

[Håkan Waara]

Attachment: New fix

Comment 60

[Håkan Waara]

Ok, I fixed jglick's points and made it so the "New Folder..." button was
aligned to the right as it is supposed to.

I also added more vertical space, which makes the dialog larger but I think it
makes it easier on the eyes.

Requesting r= (re-sr=?) and comments.

Comment 61

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

before this lands, I want a chance to review and test it. 

first question:
in content/SearchDialog.js

- HideSearchColumn("totalCol");

won't that make the totalCol show up in the search dialog?  that's not supposed
to show up in the search column (it's a threading only column.)

Comment 62

[Håkan Waara]

Seth, this is from SearchDialog.js:

#207 HideSearchColumn("totalCol"); // since you can't thread search results
...
#213 HideSearchColumn("totalCol");

My fix removes the duplicate function call.  Other questions?

Comment 63

[Håkan Waara]

Ok, got sr=hewitt. Seth promised to take a look at this today and r= it. I
really want it in before the weekend because I'll be away after that...

Comment 64

[jglick]

attachment id=38527, very nice!  Thank you. :-)  

Comment 65

[racham]

I noticed that menulist for action target folder is missing in your latest
screen shot. I haven't looked at your code changes in details yet..but giving
you heads up on missing item.

bhuvan

Comment 66

[Håkan Waara]

Bhuvan, look again - the action is "Mark the message as read". Is there really
any target folder to set there?  :)

Comment 67

[racham]

oops..!! never mind.

Comment 68

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

testing and reviewing now...

Comment 69

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

r/sr=sspitzer

good work, hwaara.

Comment 70

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

a=dbaron for trunk checkin (on behalf of drivers)

Comment 71

[Håkan Waara]

Fix checked in! wohoo!

Comment 72

[Håkan Waara]

er, FIXED.

Comment 73

[Håkan Waara]

Jglick: note that now you need to change the spec (as discussed before) to
acommodate my implementation. I hope you'll proceed. Thanks

Comment 74

[jglick]

Of course :-)

Comment 75

[laurel]

OK using june21 commercial trunk builds: win98, mac OS 9.0, linux rh6.2
I think I covered all items.  Any lingering issues will be logged separately.

Main point lingering is the space in each row to the left of the criteria
dropdowns -- used to be occupied by "And the" "Or the" text, no longer appearing
and the blank space looks odd.  That issue is under bug 82015 (until decided
otherwise).

Marking this one verified.