WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 17 months ago

Last modified 17 months ago

#22797 closed defect (bug) (fixed)

Twenty Twelve Print Stylesheet Needs More Contrast

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

Download all attachments as: .zip

Change History (28)

miqrogroove17 months ago

Fixes color in print stylesheet.

comment:1 miqrogroove17 months ago

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

miqrogroove17 months ago

Also fixes title font in Chrome.

comment:2 lancewillett17 months 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.

lancewillett17 months ago

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

comment:3 miqrogroove17 months ago

  • Keywords has-patch added

Chrome WinXP.

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

comment:4 follow-up: miqrogroove17 months ago

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

comment:5 in reply to: ↑ 4 lancewillett17 months 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.

comment:6 follow-up: lancewillett17 months 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.

comment:7 in reply to: ↑ 6 miqrogroove17 months 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

lancewillett17 months ago

Suggested change to color only

comment:8 lancewillett17 months ago

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

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

comment:9 lancewillett17 months ago

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

miqrogroove17 months ago

comment:10 miqrogroove17 months 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.

comment:11 follow-up: georgestephanis17 months 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.

comment:12 georgestephanis17 months ago

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

comment:13 miqrogroove17 months ago

So we all agree on miqro-22797.2.diff ?

comment:14 miqrogroove17 months ago

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

miqrogroove17 months ago

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

comment:15 in reply to: ↑ 11 ; follow-up: lancewillett17 months 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.

comment:16 lancewillett17 months ago

  • Keywords needs-testing removed

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

comment:17 miqrogroove17 months ago

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

comment:18 in reply to: ↑ 15 lancewillett17 months 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. :)

comment:19 lancewillett17 months 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.

comment:20 miqrogroove17 months ago

Thank you Lance :) Good work all.

Note: See TracTickets for help on using tickets.