Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#22797 closed defect (bug) (fixed)

Twenty Twelve Print Stylesheet Needs More Contrast

Reported by: miqrogroove's profile miqrogroove Owned by: lancewillett's profile lancewillett
Milestone: 3.5 Priority: normal
Severity: minor Version: 3.5
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

I just tried printing a page in Twenty Twelve for the first time and saw some deal breakers in there:

  • The site title is not black and not even close to black.
  • The content text appears to be the same shade of gray and is bordering on difficult to read.
  • The site subtitle is not legible because it is almost white on white.

According to http://core.trac.wordpress.org/browser/trunk/wp-content/themes/twentytwelve/style.css#L1616 there are actually no colors specified in the print stylesheet. That means it's using #515151 for the site title and #444 for content.

Attachments (8)

miqro-22797.diff (751 bytes) - added by miqrogroove 12 years ago.
Fixes color in print stylesheet.
miqro-22797.2.diff (810 bytes) - added by miqrogroove 12 years ago.
Also fixes title font in Chrome.
printed Theme Busters R Us -1.pdf (491.3 KB) - added by lancewillett 12 years ago.
Sample PDF printed from OS X Lion Firefox "Aurora" stable
twenty-twelve-no-patch.png (77.8 KB) - added by miqrogroove 12 years ago.
22797.3.diff (406 bytes) - added by lancewillett 12 years ago.
Suggested change to color only
22797.3.diff.png (72.4 KB) - added by miqrogroove 12 years ago.
miqro-22797.2.diff.png (72.9 KB) - added by miqrogroove 12 years ago.
miqro-22797.3.diff (675 bytes) - added by miqrogroove 12 years ago.
Alternative version, in case you guys insist on full coverage ink.

Download all attachments as: .zip

Change History (28)

@miqrogroove
12 years ago

Fixes color in print stylesheet.

#1 @miqrogroove
12 years ago

Attached patch is working for colors but breaks the H1 font in Chrome.

@miqrogroove
12 years ago

Also fixes title font in Chrome.

#2 @lancewillett
12 years ago

  • Milestone changed from Awaiting Review to 3.5

Can you share your exact steps to repeat? And browser / OS varieties you tested in.

I tried in FF latest stable on OS X Lion, and the result was as expected (dark text, good contrast). Will upload a sample PDF.

@lancewillett
12 years ago

Sample PDF printed from OS X Lion Firefox "Aurora" stable

#3 @miqrogroove
12 years ago

  • Keywords has-patch added

Chrome WinXP.

miqro-22797.2.diff​ tested and working on my end.

#4 follow-up: @miqrogroove
12 years ago

lancewillett, are you able to try my patch and let us know if it works well on OS X?

#5 in reply to: ↑ 4 @lancewillett
12 years ago

Replying to miqrogroove:

lancewillett, are you able to try my patch and let us know if it works well on OS X?

Testing now. I'm all for the idea of setting the base color, the font-family is less important for print.

#6 follow-up: @lancewillett
12 years ago

miqrogroove — I can't repeat the unreadable text with Windows XP and, could you attach a screenshot of what you're seeing? Can you test it out with http://sanbeiji.com/tests/textcolor.php ?

Looks like color: black; is the best choice.

#7 in reply to: ↑ 6 @miqrogroove
12 years ago

Replying to lancewillett:

miqrogroove — I can't repeat the unreadable text with Windows XP and, could you attach a screenshot of what you're seeing?

fwiw, attached

@lancewillett
12 years ago

Suggested change to color only

#8 @lancewillett
12 years ago

Thanks, a screenshot is worth 1,000 words, you know. :)

Could you take another test + screenshot with 22797.3.diff applied, please?

#9 @lancewillett
12 years ago

  • Keywords needs-testing added
  • Severity changed from major to minor

#10 @miqrogroove
12 years ago

I made screen shots of both patches so you can see the difference. Only setting the body color misses the other problems I mentioned.

#11 follow-up: @georgestephanis
12 years ago

As per the CSS Style Guide ( http://make.wordpress.org/core/handbook/coding-standards/css/#properties ) let's make it do color: #000; instead of color: black; -- it's one character shorter, and internally consistent.

#12 @georgestephanis
12 years ago

Also, I don't *think* it needs a !important on the color -- unless you mean to prevent child themes from accidentally overriding it.

#13 @miqrogroove
12 years ago

So we all agree on miqro-22797.2.diff ?

#14 @miqrogroove
12 years ago

Actually, I got miqro-22797.diff to work after a cache dump. +1 that patch.

@miqrogroove
12 years ago

Alternative version, in case you guys insist on full coverage ink.

#15 in reply to: ↑ 11 ; follow-up: @lancewillett
12 years ago

Replying to georgestephanis:

As per the CSS Style Guide ( http://make.wordpress.org/core/handbook/coding-standards/css/#properties ) let's make it do color: #000; instead of color: black; -- it's one character shorter, and internally consistent.

Based on research here: http://www.sanbeiji.com/archives/953black prints faster, but I'm not tied to it.

Agreed on not needing the !important.

#16 @lancewillett
12 years ago

  • Keywords needs-testing removed

miqro-22797.3.diff looks good, agreed on extra coverage to ensure good contrast.

#17 @miqrogroove
12 years ago

I think #222 or even #111 is more representative of the header color from screen, but any of them will work for me.

#18 in reply to: ↑ 15 @lancewillett
12 years ago

Replying to lancewillett:

Based on research here: http://www.sanbeiji.com/archives/953black prints faster, but I'm not tied to it.

Scratch that, no argument there about black as a keyword, nor speed in printing, there. The link was in the HTML5 Boilerplate source which I took to mean "black keyword is better". We're OK with hex values. :)

#19 @lancewillett
12 years ago

  • Owner set to lancewillett
  • Resolution set to fixed
  • Status changed from new to closed

In 23116:

Twenty Twelve: set color values to black to ensure full text color contrast in print styles. Props miqrogroove, fixes #22797.

#20 @miqrogroove
12 years ago

Thank you Lance :) Good work all.

Note: See TracTickets for help on using tickets.