r97788 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r97787‎ | r97788 | r97789 >
Date:03:51, 22 September 2011
Author:santhosh
Status:reverted (Comments)
Tags:
Comment:
Zero padding for #firstHeading makes text cut for scripts like Devanagari , Malayalam etc(Bug 29405 and Bug 30809), especially when the glyphs are stacked or
having lower diacritic marks. Remove padding-top:0 and padding-bottom:0 to allow h1 inherit the padding bottom and top values from commonElements.css fixes this.
Modified paths:
  • /trunk/phase3/skins/common/commonInterface.css (modified) (history)
  • /trunk/phase3/skins/vector/screen.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/commonInterface.css
——@@ -39,7 +39,6 @@
4040 /* These two rules hack around bug 2013 (fix for more limited bug 11325).
4141 When bug 2013 is fixed properly, they should be removed. */
4242 line-height: 1.2em;
43 - padding-bottom: 0;
4443 }
4544
4645 /* Sub-navigation */
Index: trunk/phase3/skins/vector/screen.css
——@@ -689,9 +689,7 @@
690690 font-size: 0.8em;
691691 }
692692 #firstHeading {
693 - padding-top: 0;
694693 margin-top: 0;
695 - padding-top: 0;
696694 font-size: 1.6em;
697695 }
698696 div#content a.external,

Follow-up revisions

RevisionCommit summaryAuthorDate
r98596Revert r97788santhosh01:35, 1 October 2011
r99389Language specific height correction for titles. Ref Bug 29405 and Bug 30809. ...santhosh04:13, 10 October 2011
r108215Prevent #firstHeading overriding the language specific h1 height....santhosh11:24, 6 January 2012

Comments

#Comment by P858snake (talk | contribs)   03:54, 22 September 2011

How does this affect rendering for other languages/scripts?

#Comment by Santhosh.thottingal (talk | contribs)   03:57, 22 September 2011

For other languages/scripts, this will add the following padding from commonElement.css for the top heading ie, #firstHeading padding-top: .5em; padding-bottom: .17em;

#Comment by Trevor Parscal (WMF) (talk | contribs)   01:13, 1 October 2011

This should be reverted because it causes the first heading to have too much space above it. The correct approach would be to locally, or at least specifically, target this language with CSS.

For example, you might want to use something like this... 

html[lang=hi] h1,
html[lang=hi] h2,
html[lang=hi] h3,
html[lang=hi] h4,
html[lang=hi] h5,
html[lang=hi] h6 {
	line-height: 2em;
}
#Comment by Catrope (talk | contribs)   08:23, 1 October 2011

Wouldn't h1:lang(hi) be better? That's what we use for list numerals too, and it means we support mixed-language HTML.

#Comment by Santhosh.thottingal (talk | contribs)   01:37, 1 October 2011

reverted in r98596

Status & tagging log