Closed Bug 623660 Opened 9 years ago Closed 9 years ago

Change FilterEditor dimensions (and move them to relevant .dtd files)

Categories

(MailNews Core :: Filters, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 5.0b1

People

(Reporter: stefanh, Assigned: stefanh)

Details

Attachments

(1 file, 3 obsolete files)

The FilterEditor dialog needs a bit more height on mac, especially after I re-styled it in bug 614382. Dimensions are hardcoded in the xul file, so I also propose we move them to the mail & suite .dtd files.

For seamonkey, I've came to the conclusion that (on mac) it looks good with a width of 80ch and a height of 40em. Both width and height can be increased a bit if it's a problem on other OS. I don't expect anyone to want to increase the height, though.

For thunderbird, I'd say that my proposed dimensions should work too. Note that you have a hardcoded width of 560px set on the dialog in filterDialog.css, which you obviously copied from the seamonkey files for 100 years ago (it's not there anymore in the seamonkey files).

philor: maybe you want the mac styling (you'll cry less when looking at the dialog)?
Attached patch Move & change dimensions (obsolete) — Splinter Review
See comment #0 for background etc.
Assignee: nobody → stefanh
Status: NEW → ASSIGNED
Attachment #501726 - Flags: superreview?(neil)
Attachment #501726 - Flags: review?(philringnalda)
Attachment #501726 - Flags: review?(mnyromyr)
Actually, I could remove the hardcoded dimensions in the css files too... give me a sec...
Should be easier to test dimensions in thunderbird now ;-)
Attachment #501726 - Attachment is obsolete: true
Attachment #501735 - Flags: superreview?(neil)
Attachment #501735 - Flags: review?(philringnalda)
Attachment #501735 - Flags: review?(mnyromyr)
Attachment #501726 - Flags: superreview?(neil)
Attachment #501726 - Flags: review?(philringnalda)
Attachment #501726 - Flags: review?(mnyromyr)
Attachment #501735 - Attachment is patch: true
Attachment #501735 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 501735 [details] [diff] [review]
Now with removed dimensions in mail theme files

I think this is a little too tall and narrow, I'd prefer 35em by 90ch.
Comment on attachment 501735 [details] [diff] [review]
Now with removed dimensions in mail theme files

r=me with Neil's sizes.
Attachment #501735 - Flags: review?(mnyromyr) → review+
Comment on attachment 501735 [details] [diff] [review]
Now with removed dimensions in mail theme files

Bah. I was going to get my Linux VM working again so I could look there, and instead I just let it slip away. I'm fine with it in terms of code, so I'll delegate my review to Bryan, since it's really more of a ui-r anyway.
Attachment #501735 - Flags: review?(philringnalda) → review?(clarkbw)
Comment on attachment 501735 [details] [diff] [review]
Now with removed dimensions in mail theme files

Both sizes worked and I don't have a strong feeling for which is better so neil's size works fine by me.  switching this over to ui-review only, standard8 might have a peak at the code.
Attachment #501735 - Flags: review?(clarkbw) → ui-review+
On IRC a long time ago now, I mentioned that it might be a good idea to edit searchTermOverlay.xul to add rows="4" to its listbox, because then we can lose the height on FilterEditor.xul completely, as it will automatically size.

As for the width, I actually had to bump it to 95ch in Classic and a massive 105ch in Modern before I could see the whole of "From, To, CC or Bcc" [V] "isn't in my address book" [V] "Personal Address Book" [V].
Attached patch Now with no height specified (obsolete) — Splinter Review
(In reply to comment #8)
> On IRC a long time ago now, I mentioned that it might be a good idea to edit
> searchTermOverlay.xul to add rows="4" to its listbox, because then we can lose
> the height on FilterEditor.xul completely, as it will automatically size.
Done. I had to remove one hbox since it somehow made the the listbox to not stretch across the window. I also killed a few flex attributes.

> As for the width, I actually had to bump it to 95ch in Classic and a massive
> 105ch in Modern before I could see the whole of "From, To, CC or Bcc" [V]
> "isn't in my address book" [V] "Personal Address Book" [V].
It's huge, but I guess we have to bump it to 105ch.

(I'll let thunderbird peers look at this once things are settled with Neil)
Attachment #501735 - Attachment is obsolete: true
Attachment #520540 - Flags: superreview?(neil)
Attachment #520540 - Flags: review?(neil)
Attachment #501735 - Flags: superreview?(neil)
This implies string changes, so if we want this for seamonkey it has to block b3.
blocking-seamonkey2.1: --- → ?
Neil pointed out that the flex attributes are needed (in case someone resize the dialog).
Attachment #520540 - Attachment is obsolete: true
Attachment #520547 - Flags: superreview?(neil)
Attachment #520547 - Flags: review?(neil)
Attachment #520540 - Flags: superreview?(neil)
Attachment #520540 - Flags: review?(neil)
Attachment #520547 - Flags: superreview?(neil)
Attachment #520547 - Flags: superreview+
Attachment #520547 - Flags: review?(neil)
Attachment #520547 - Flags: review+
Comment on attachment 520547 [details] [diff] [review]
Keep flex attributes

clarkbw, I discovered that some labels in the menulists where cropped with 95ch, so I increased it to 100ch for Thunderbird (I don't think you want 105ch).

Asking Mark for review per comment #7.
Attachment #520547 - Flags: ui-review?(clarkbw)
Attachment #520547 - Flags: review?(bugzilla)
(In reply to comment #12)
> clarkbw, I discovered that some labels in the menulists where cropped with
> 95ch, so I increased it to 100ch for Thunderbird (I don't think you want
> 105ch).

Forgot to say that we also now have 4 rows height on both listboxes.
Comment on attachment 520547 [details] [diff] [review]
Keep flex attributes

Tested this out in Seamonkey on the Mac and it looks good. I like the improved +/- buttons.
Attachment #520547 - Flags: ui-review?(clarkbw) → ui-review+
(In reply to comment #14)
> Comment on attachment 520547 [details] [diff] [review]
> Keep flex attributes
> 
> Tested this out in Seamonkey on the Mac and it looks good. I like the improved
> +/- buttons.

In case anyone wonders, we sorted this out on irc:
<stefanh>	clarkbw: oh, so the r+ is actually for the patch in bug 623660
<clarkbw>	the dimensions look fine
<stefanh>	ah cool
<clarkbw>	yeah
Attachment #520547 - Flags: review?(bugzilla) → review+
http://hg.mozilla.org/comm-central/rev/cec7e6a809ad
Status: ASSIGNED → RESOLVED
blocking-seamonkey2.1: ? → ---
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a4
You need to log in before you can comment on or make changes to this bug.