Make WordPress Core

Opened 17 years ago

Closed 17 years ago

Last modified 17 years ago

#4119 closed defect (bug) (fixed)

5px line-height for h2 title in wp-admin.css

Reported by: bertilow's profile bertilow Owned by: rob1n's profile rob1n
Milestone: 2.3 Priority: normal
Severity: normal Version: 2.1.3
Component: Administration Keywords: has-patch commit
Focuses: Cc:

Description

The stylesheet "wp-admin.css" has the following rule:

h2 {
  border-bottom: .5em solid #e5f3ff;
  color: #333;
  font: normal 32px/5px serif;
  margin: 5px 10px;
}

The value "32px/5px" sets the line-height to five pixels.
That gives disastrous text-overlapping if an h2-title does
not fit in one line.

The rule should be changed to:

h2 {
  border-bottom: .5em solid #e5f3ff;
  color: #333;
  font: normal 32px serif;
  margin: 5px 10px;
}

That sets the line-height to "normal".

I attach a screen-shot that shows text overlapping.

(In general it's better not to fiddle with line-height.)

Attachments (7)

wp-admin.png (25.4 KB) - added by bertilow 17 years ago.
wp-admin.css.diff (341 bytes) - added by JeremyVisser 17 years ago.
Patch for removing line-height property for better display.
Before.png (67.9 KB) - added by JeremyVisser 17 years ago.
Before patch is applied
After.png (63.1 KB) - added by JeremyVisser 17 years ago.
After patch is applied; note the side-effect
heading-bg.gif (37 bytes) - added by JeremyVisser 17 years ago.
Background image for the H2.
wp-admin.css.2.diff (828 bytes) - added by JeremyVisser 17 years ago.
Makes the H2 use background-image instead of border. Fixes text wrapping problem.
wp-admin.css.3.diff (1.1 KB) - added by JeremyVisser 17 years ago.
Let's have a respin of that.

Download all attachments as: .zip

Change History (26)

@bertilow
17 years ago

#1 @JeremyVisser
17 years ago

  • Keywords needs-patch added
  • Owner changed from anonymous to JeremyVisser
  • Status changed from new to assigned

+1. Cooking up patch.

@JeremyVisser
17 years ago

Patch for removing line-height property for better display.

#2 @JeremyVisser
17 years ago

  • Keywords has-patch added; needs-patch removed

Actually, this has a problem. The border will no longer show underneath the text with this patch applied.

@JeremyVisser
17 years ago

Before patch is applied

@JeremyVisser
17 years ago

After patch is applied; note the side-effect

#3 @JeremyVisser
17 years ago

  • Keywords bughunt dev-feedback 2nd-opinion added

I think we should discuss this at tomorrow's bug hunt.

#4 follow-up: @matt
17 years ago

That rule is deliberate to put the line behind the text. H2s should not be so long as to wrap.

#5 @JeremyVisser
17 years ago

Yeah, I got that impression already. However, as you can see from bertilow's screenshot, sometimes they do wrap, and when they do, it's bad.

#6 in reply to: ↑ 4 @rob1n
17 years ago

  • Milestone changed from 2.4 to 2.3
  • Owner changed from JeremyVisser to rob1n
  • Status changed from assigned to new

Replying to matt:

That rule is deliberate to put the line behind the text. H2s should not be so long as to wrap.

Usability > aesthetics.

#7 @rob1n
17 years ago

  • Keywords css line-height bughunt dev-feedback removed

#8 @andy
17 years ago

The line-height trick is clever and it looks good. It falls apart when it wraps and this happens too often to be dismissed.

The blue line does not look good at any distance away from the text. It looks good under the bottom half. That's when there's one line of text. When it wraps, that line has to go somewhere. It belongs at the bottom so it should always be partly overlapped by the lowermost line of text.

I suggest that line be kept right where it is: halfway up the bottom line. I also want the line-height rule and the bottom-border rule removed. Compromise? Background-image! Anybody interested in implementing that?

#9 @JeremyVisser
17 years ago

  • Owner changed from rob1n to JeremyVisser
  • Status changed from new to assigned

I'll have a bash.

@JeremyVisser
17 years ago

Background image for the H2.

@JeremyVisser
17 years ago

Makes the H2 use background-image instead of border. Fixes text wrapping problem.

#10 @JeremyVisser
17 years ago

Works like a charm (at least on Fx2/Ubuntu).

#11 @JeremyVisser
17 years ago

  • Keywords needs-testing added

Oh, forgot to mention. Patched against trunk 5375.

@JeremyVisser
17 years ago

Let's have a respin of that.

#12 @JeremyVisser
17 years ago

  • Owner changed from JeremyVisser to rob1n
  • Status changed from assigned to new

#13 @rob1n
17 years ago

  • Keywords commit added; 2nd-opinion needs-testing removed

I like it.

#14 @rob1n
17 years ago

  • Status changed from new to assigned

#15 @rob1n
17 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [5430]) Make H2's in the admin wrap nice, and some CSS cleanup. Props JeremyVisser. fixes #4119

#16 @matt
17 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This image is far too tall, the proportions are off and ugly.

#17 @rob1n
17 years ago

Looks the same to me...

#18 @rob1n
17 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Looks fine for me.

#19 @JeremyVisser
17 years ago

Screenshot, Matt?

Note: See TracTickets for help on using tickets.