Page MenuHomePhabricator

Typography refresh fixup: list line-height, h1 margin, h3 font-size
Closed, ResolvedPublic

Description

Following the Typography refresh, two changes I propose:

  • <dd>, <ol> and <ul> have their line-height set to 1.5em in commonElements.css; these must be removed from elements.css.
  • H1 has no top margin in the body, causing it to stick to content directly above it. Set to 1em for H1, and to 0 for #firstHeading.

I would *like* to increase H3 font-size from 1.17em to 1.2em, as it is too close in size to body text. Are there and objections to this?

Details

Reference
bz64653
Related Gerrit Patches:

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 3:19 AM
bzimport added a project: Vector.
bzimport set Reference to bz64653.
bzimport added a subscriber: Unknown Object (MLST).
Edokter created this task.Apr 30 2014, 12:52 PM

Typography folks: Could somebody please provide feedback on h3 size to Erwin?

Hey Erwin thanks for raising these issues - a few clarifications!

  • In terms of line height - why do they need to be inherit? What problem does this cause?
  • In terms of the H1 having no top margin can you submit a screenshot showing where this becomes a problem? I see there is top padding on #content so I'm not sure why this would be a problem and I'm worried increasing this gap with a top margin would be weird.
  • In terms of h3 font size I can always review it with the designers and point out the changes. <aybe submit the patch anyway?

Hi Jon,

  • Currently, line-height is set to 1.5em for <dd>, <ol> and <ul> in commonElements.css. This overrides the 1.6em from the typography refresh, which causes inconsistent line heights on talk pages. They will need to match (not inherit, strike that).

I'm not looking to change the title H1, just add the margin to H1 inside bodyContent to match H2.

  • I'll submit it tomorrow.

Oh I see! Yeh this doesn't seem like a problem provided it doesn't touch the top margin of the firstHeading!

Thanks :)

Added issue: diff lineheight, for which a patch is waiting review: https://gerrit.wikimedia.org/r/133978/

Will submit patch for other issues later.

Change 133978 had a related patch set uploaded by Jdlrobson:
Apply correct line-height to diffs

https://gerrit.wikimedia.org/r/133978

Change 133978 merged by jenkins-bot:
Apply correct line-height to diffs

https://gerrit.wikimedia.org/r/133978

tomasz added a comment.Aug 5 2014, 7:59 PM

Patch was merged, so resetting bug status to NEW.

Krinkle renamed this task from Minor fixes for Typography refresh: list lineheight, H1 margin, H3 fontsize, diff display to Typography refresh fixup: list line-height, h1 margin, h3 font-size, diff display.Jan 5 2015, 2:19 AM
Krinkle updated the task description. (Show Details)
Krinkle set Security to None.
Krinkle removed subscribers: Unknown Object (MLST), Krinkle.

Change 207424 had a related patch set uploaded (by Edokter):
Remove redundant line-height declarations from elements.css

https://gerrit.wikimedia.org/r/207424

Edokter claimed this task.Apr 29 2015, 12:49 PM
Edokter updated the task description. (Show Details)
Edokter renamed this task from Typography refresh fixup: list line-height, h1 margin, h3 font-size, diff display to Typography refresh fixup: list line-height, h1 margin, h3 font-size.Apr 29 2015, 3:40 PM

Change 207424 merged by jenkins-bot:
Remove redundant line-height declarations from elements.css

https://gerrit.wikimedia.org/r/207424

Edokter updated the task description. (Show Details)May 15 2015, 9:58 AM
Edokter removed a project: Patch-For-Review.

Change 212889 had a related patch set uploaded (by Gerrit Patch Uploader):
Minor header fixes for Typography Refresh

https://gerrit.wikimedia.org/r/212889

Final patch pending. Once merged, this can be closed.

Change 212889 merged by jenkins-bot:
Minor header fixes for Typography Refresh

https://gerrit.wikimedia.org/r/212889

Change 212890 had a related patch set uploaded (by Bartosz Dziewoński):
Remove redundant line-height declarations from elements.css

https://gerrit.wikimedia.org/r/212890

Change 212891 had a related patch set uploaded (by Bartosz Dziewoński):
Minor header fixes for Typography Refresh

https://gerrit.wikimedia.org/r/212891

matmarex closed this task as Resolved.May 23 2015, 12:16 PM
matmarex updated the task description. (Show Details)
matmarex edited projects, added MediaWiki-Interface; removed Patch-For-Review.
matmarex removed a subscriber: gerritbot.

Change 212890 merged by jenkins-bot:
Remove redundant line-height declarations from elements.css

https://gerrit.wikimedia.org/r/212890

Change 212891 abandoned by Jforrester:
Minor header fixes for Typography Refresh

Reason:
Not doing this for this release, per Chad.

https://gerrit.wikimedia.org/r/212891