The attachment shows some identified problems in the filter editor dialog. 1. the space to the right of the add/remove buttons is different between the Rules and the Actions lists. I'd try to make it the same and even make it smaller than it is now in the Actions list. 2. even if there is plenty of space, the fields in the Actions list do not expand (flex). I'd try to add same flex values to them. Expanding e.g. the folder picker could help bug 698434. 3. The fields in the Rules list do have flex values. But they all expand uniformly. I'd think the header and the operator fields do not gain much from expanding. The value field should expand much more quickly as it is more important and could contain long values. I try to play with the flex values in http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/searchTermOverlay.dtd and add some new ones for the Actions fields. What do you think? Or is the current state intentional?

I don't think the current state is particularly intentional, and the changes you suggest make sense to me. Thanks, Blake.

Thanks I'll look into it. There is also a spacer right of the filter name, that is just taking space that could be used for the filter name. Maybe it just looks better?

This is some progress. But I can't manage to make the elements flex properly. 1. The intention was to have the SearchTermAttribute flex=1, the Operator flex=1 and Value flex=3. 5 in total, then the action columns like this: ActionType flex=1, ActionValue flex=4, 5 in total. But the SearchTermAttribute is not the same width as ActionType (yes, I considered the mix-width in the theme) when the window is wide. 2. the ActionValue element (e.g. target folder of Copy/move or Forward address) does not span the whole column as I would like it to. Any ideas how to fix these?

Comment on attachment 629596 [details] [diff] [review] WIP patch I'm no expert with the dark art of flex, but Neil is.

Comment on attachment 613566 [details] screenshot of existing problems 1. The filler adjustment has the problem that it only works in the filter editor. Better would be simply to remove the flex from the button column. If you wanted to, you could change the filter actions to have a dummy column too, and remove the filler class entirely. 2. This simply has a minimum width defined in CSS, there's no way to automatically size it although we could arrange to have a percentage of the width of the list (I don't think the full width would look right but that's bwinton's decision). 3.Not sure what the problem is here.

(In reply to aceman) > 3. The fields in the Rules list do have flex values. But they all expand > uniformly. I'd think the header and the operator fields do not gain much > from expanding. The value field should expand much more quickly as it is > more important and could contain long values. The problem here is that the possible values of the search operator changes depending on the search term. This means that if we made the search operator inflexible then its width would change, which could negatively impact layout. In fact it's probably possible to get the width of the search term list to change too, but I guess that's somewhat less likely. I guess we could work around that by specifying a minimum width in the locale. By comparison the filter action column is inflexible because its list doesn't have to change.

Thanks, but could you comment on the patch, comment 3? About the filler, in the patch I have made buttons in both lists (rules and actions) to have the filler, so they have the same padding. Yes, it is there only in filter editor and search dialog does not have the padding (in both lists). But it is uniform inside each dialog (which is the point of this bug). If the same padding is wanted in search dialog, I can add it. Flex on buttons' column is removed in the patch.

Comment on attachment 629596 [details] [diff] [review] WIP patch I'm not sure how to fix the issues you're reporting, but I'll add another one. ;) When I expand the box, and then collapse it, the fields don't shrink, leading to crazy cut-off fields. So, given that, and the problems you're hitting, I guess I'm going to say f-, but I do think that, at least visual-wise, you're heading in the right direction… Thanks, Blake.

If you click on a dropdown in the list the fields will shrink as needed. But that problem is there regardless of my patch. Is there a way to force reflow of the whole dialog on resize?

(In reply to :aceman from comment #7) > About the filler, in the patch I have made buttons in both lists (rules and > actions) to have the filler, so they have the same padding. Yes, it is there > only in filter editor and search dialog does not have the padding (in both > lists). But it is uniform inside each dialog (which is the point of this > bug). If the same padding is wanted in search dialog, I can add it. Flex on > buttons' column is removed in the patch. Some padding is needed in the search dialog; because you removed the dummy column, the search dialog has no padding at all so if you have many search criteria then the scrollbar overlaps the buttons.

Comment on attachment 629596 [details] [diff] [review] WIP patch It turns out that I can't comment on the patch because you didn't update the suite locales so I can't actually open the filter editor.

Sorry about that, I'll fix the last 2 comments. Yes I removed the dummy column. So I'll replace it with a properly spaced filler in both dialogs.

OK, try this in SM.

Comment on attachment 631760 [details] [diff] [review] WIP patch v2 This doesn't work very well because the action popup changes in size depending on which action you select. This is because flex applies to the spare space, which varies depending on whether the action has a target. You can work around this by adding a width to the action listcol so that its allocation of space doesn't depend on its content. Normally if a listcell is too narrow then its contents simply overflow, so I added orient="vertical" align="start" pack="center" which enforces a maximum width of the contents to be that of the cell. Unfortunately for you this also disables your attempts to make the contents flex to the width of the cell. Also flex="1" on a listcell makes no sense.

Changing orient to horizontal seems to expand the action value as I want. Short of aligning the rules columns and the action columns (both filter attribute and filter action columns have flex=1 and they still do not align when when the other columns have a total flex of 4), the patch does what I wanted.

Comment on attachment 636167 [details] [diff] [review] patch v3 When I tried to apply this patch I got more rejects than successes :-( >- orient="vertical" align="start" pack="center"/> >+ orient="horizontal" align="start" pack="center"/> Switching the orient also switches the meaning of the align and pack. You had two options here: 1. Because orient="horizontal" pack="start" align="center" is the default for a listcell, you could just remove the orient="vertical" align="start" pack="center" completely 2. You could just change to align="stretch", which would avoid having to add flex="1" everywhere to the children. >+ <xul:hbox align="start" flex="1"> Why? >+ <xul:menupopup flex="1"> Is this to fix bwinton's issue? I can't see what other effect it's supposed to have.

I added flex all the way from listcol (thus listcell) to the lowers element so that the contents stretch. It may be that some of them are superfluous. Maybe that is what you say in point 2. Into which element should I add align="stretch"?

(In reply to neil@parkwaycc.co.uk from comment #16) > >+ <xul:hbox align="start" flex="1"> > Why? Copied from the rules list so that the buttons have the same enclosing box and so same style applied. But I'll remove the flex. > >+ <xul:menupopup flex="1"> > Is this to fix bwinton's issue? I can't see what other effect it's supposed > to have. No, I can't fix bwinton's issue. Any idea why that happens? I'll remove the flex.

Thanks, I used the stretch and could remove many of the flex="1".

(In reply to neil@parkwaycc.co.uk from comment #16) > When I tried to apply this patch I got more rejects than successes :-( Oops, it does help to unapply the previous patch...

(In reply to aceman from comment #18) > (In reply to neil@parkwaycc.co.uk from comment #16) > > >+ <xul:hbox align="start" flex="1"> > > Why? > Copied from the rules list so that the buttons have the same enclosing box > and so same style applied. But I'll remove the flex. I don't see a style that depends on the enclosing box. (The only reason the rules list uses an enclosing box is that the code that constructs the list can only handle a single direct child of each listcell.) > > >+ <xul:menupopup flex="1"> > > Is this to fix bwinton's issue? I can't see what other effect it's supposed > > to have. > No, I can't fix bwinton's issue. Any idea why that happens? No, I thought it was to do with menupopup sizing but that doesn't appear to be the case.

(In reply to neil@parkwaycc.co.uk from comment #21) > (In reply to aceman from comment #18) > > (In reply to neil@parkwaycc.co.uk from comment #16) > > > >+ <xul:hbox align="start" flex="1"> > > > Why? > > Copied from the rules list so that the buttons have the same enclosing box > > and so same style applied. But I'll remove the flex. > I don't see a style that depends on the enclosing box. (The only reason the > rules list uses an enclosing box is that the code that constructs the list > can only handle a single direct child of each listcell.) Maybe no style today (except for differing flex), but one day something may be applied to them. The point of this bug is to cleanup sizes and spacing and make them consistent in those 2 lists. So is it a problem adding that hbox? There is no visible difference for now. Maybe performance?

(In reply to aceman from comment #22) > The point of this bug is to cleanup sizes and spacing > and make them consistent in those 2 lists. Ah yes, I see you have the word "consistently" in the description. [You would lose out slightly on blame though.] (In reply to neil@parkwaycc.co.uk from comment #14) > This doesn't work very well because the action popup changes in size > depending on which action you select. Any advance on this?

(In reply to neil@parkwaycc.co.uk from comment #23) > (In reply to neil@parkwaycc.co.uk from comment #14) > > This doesn't work very well because the action popup changes in size > > depending on which action you select. > Any advance on this? No, it still happens and I don't know why. It is caused by the patch (doesn't happen before). But is it a problem? Too ugly?

(In reply to aceman from comment #24) > (In reply to #23) > > (In reply to comment #14) > > > This doesn't work very well because the action popup changes in size > > > depending on which action you select. > > Any advance on this? > No, it still happens and I don't know why. It is caused by the patch > (doesn't happen before). But is it a problem? Too ugly? Well, I think it is. I'd ask bwinton but he's away :-(

Comment on attachment 636469 [details] [diff] [review] patch v4 No, it's not too ugly. ui-r=me. :) Thanks, Blake.

Comment on attachment 636469 [details] [diff] [review] patch v4 Well, I guess we'll see if anyone else complains ;-)

Comment on attachment 636469 [details] [diff] [review] patch v4 Review of attachment 636469 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! Thanks aceman!