603762 - Vertical margins between header, footer, and content are not consistent

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[LegNeato]

Attachment: Make margins 1em

This has been bugging me even though I know it is trivial. Margins between header, footer, and content are not consistent. Attached is a screenshot showing a before and after.

Comment 1

[LegNeato]

Attachment: Before and after

Comment 2

[Frédéric Buclin]

Why should they be the same?

Comment 3

[LegNeato]

Because it looks lopsided/off if they aren't with the dusk theme.

Comment 4

[Max Kanat-Alexander]

Hmm. I agree that it's nice to have the consistency. However, in Dusk, there's a slight difference in purpose between the side margins and the top/bottom margins:

 * The side margins offset the text of the page and the general content, since we know that having side margins is important for readability. (See: hundreds of years of research and data on book formatting.)

  * The top/bottom margins are there simply to separate the header and footer from the content, and in Bugzilla, vertical space is at a premium (as it is on most web sites). We want as little as possible at the top of Bugzilla, while still keeping it nice-looking and readable. So I actually want the top/bottom margins to be thinner than the side margins.

Comment 5

[LegNeato]

Yeah, I agree. This patch only makes it so the top and bottom margins are equal. I matched the bigger of the two, but it could just as easily be set at 1em (though at 1.5em it looks a little nicer to my eyes). The patch doesn't touch the side margins. It looked like the vertical space difference between header->content and content->footer wasn't a conscious choice, as top/bottom margins & padding were set and reset to multiple values and eventually stacked on each other,

Comment 6

[LegNeato]

(I now realize my screenshot makes it look look like I changed the side margins, but that's merely an unfortunate crop)

Comment 7

[Max Kanat-Alexander]

Ah ha! In that case then yes, I am totally in agreement with you about this patch. :-) But I will let pyrzak review it. :-)

Comment 8

[Guy Pyrzak]

Comment on attachment 482653 [details] [diff] [review]
Make margins 1em

sorry it took so long, must have gotten lost in the mix. Looks good.

Comment 9

[Max Kanat-Alexander]

Christian: So, you have the ability to check in your own patches now, right? Our instructions for that are here:

  https://wiki.mozilla.org/Bugzilla:Committing_Patches

Would be best to do it when myself or another committer is already online, so that you can run any questions by them, and also have them confirm the correctness of whatever command lines you use (since we can't uncommit or change a revision once it's in).

Comment 10

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified skins/contrib/Dusk/global.css
modified skins/standard/global.css
Committed revision 7634.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.0/
modified skins/contrib/Dusk/global.css
modified skins/standard/global.css
Committed revision 7501.