Page MenuHomePhabricator

Design changes to desktop print styles
Closed, ResolvedPublic5 Story Points

Description

Problem

For better readability, we need to make default print styles for desktop more printer friendly and reader friendly.

This work involves changes to typography and layout.
This task is for us to make the design changes to the styles so we can move towards resolving the EPIC at T154965

These changes also speak to T142207 which involves community consultation done by WMDE

Patches to review

Design changes

  • Omit certain elements

Various elements should be hidden in the display.

  • Typography
  • Justify ** is captured in T171330

Better headings
Right now, headings get diluted in the body content and are difficult to spot. They also have uneven spacing between its own content block vs last content <p>

Spacing
Spacing between headings and <p> is a big issue. with new styles, we will have clear ownership of <p>s with its heading

Hyperlink underline
This is a community requested feature. We do not use color in print styles because it's printer friendly. Because of this, it is not possible to indicate blue links in articles. If a user wants to know if this article exists on wikipedia, it's not possible given solution. We would like to underline the <a> tags in print styles. it's better for accessibility as well.

Body typography
We would like to match Latex style body typography with single column. The new styles will reduce the number of pages by around 20-25%

That means, an article which takes 100 pages to print will take around 80 pages to print.
~~https://gerrit.wikimedia.org/r/366173~~

  • TOC

Right now, table of contents looks broken in print styles. It looks unstyled and has no sense of hierarchy
~~https://gerrit.wikimedia.org/r/366618~~

  • Hiding unwanted elements**

We would also like to hide unwanted elements on print styles, such as protect icons, hatnotes, buttons without content etc. the list is in CSS.

  • editor styles: (references and infoboxes)

http://reading-web-staging.wmflabs.org/w/index.php?title=MediaWiki:Print.css&action=edit
(to be ported to production once we've worked them out)

Design spec

All of the above changes have been incorporated in this print.css file.
https://gist.github.com/jdlrobson/111b3fbf1e0e41fd5468ab5a5be9e969
(do not copy and paste verbatim - make sure you use LESS variables, reuse existing rules where it makes sense and do not add redundant CSS rules)

Zeplin spec for printing
First page + body text - https://zpl.io/UFmH3

H2-h3-h4 - https://zpl.io/1tywav

TOC - https://zpl.io/ZCn2Yv

Implementation notes

  • Nirzar's original stylesheet added overflow: hidden to h2s but it's not clear why. Check if there is a reason this is needed. We worked out why (see patches to review)
  • Nirzar's original stylesheet both hid and added styling for #siteSub

https://gist.github.com/jdlrobson/712e6860ddf8a5dddcb55d7c164a2d1a - which is the prefered solution? Checked with Nirzar.. it's hiding it.

Acceptance criteria

  • This should be feature flagged, defaulting to false
  • We will enable the print styles on the beta cluster and reading web staging

Sign off steps

  • Please get a developer to return reading web staging to normal. In particular the config $wgMFRemovableClasses in LocalSettings.php should be commented out.

Related Objects

StatusAssignedTask
OpenJKatzWMF
OpenNone
DuplicateNone
OpenJKatzWMF
OpenNone
OpenNone
Resolvedpmiazga
InvalidNone
OpenNone
Resolvedovasileva
InvalidNone
Resolvedphuedx
OpenNone
InvalidNone
InvalidNone
OpenNone
DuplicateNone
ResolvedNirzar
ResolvedNirzar
OpenNone
Declinedmobrovac
StalledJdlrobson
DeclinedNone
StalledNirzar
ResolvedNirzar
OpenNirzar
ResolvedNirzar
Resolvedovasileva

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 366971 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Adjustments to floating elements in print styles

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

Jdlrobson updated the task description. (Show Details)Jul 21 2017, 9:49 PM
matmarex removed a subscriber: matmarex.Jul 22 2017, 7:43 PM

Change 366967 had a related patch set uploaded (by Bartosz Dziewoński; owner: Jdlrobson):
[mediawiki/core@master] Fix font size and backgrounds in gallery

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

After chatting with @Nirzar and @ovasileva they've requested that I get these new print styles enabled on reading web staging. Heads up to @pmiazga I'll be pushing a patch to MobileFrontend to allow us to use MwContentProvider on desktop...

Jdlrobson updated the task description. (Show Details)Jul 24 2017, 11:07 PM

Change 367627 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Fix stylesheet

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

Jdlrobson updated the task description. (Show Details)Jul 24 2017, 11:55 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)Jul 25 2017, 12:04 AM

Feeling a bit overwhelmed and demoralised with all the moving parts here. Code review very much needed y'all.

I've updated staging with all the above patches and have asked @Nirzar to review http://reading-web-staging.wmflabs.org/wiki/Albert_Einstein and other pages - please feel free to do the same

If you find any bugs please list them in this etherpad: https://etherpad.wikimedia.org/p/DudeYourPrintStyles

I'll talk through the problems with @Nirzar sometime tomorrow.

Nirzar added a comment.EditedJul 25 2017, 1:04 AM

Reviewing the reading web staging

Here are things that need fixing

  • Use "Charis SIL", 'Georgia' , 'Times' , serif; as font stack wherever we use serif. this stack is used on Wikipedia.

Note: we will be just replacing Charis with Linux libertine. also this is dependent on T169828 but we would have rest of the stack same including Georgia.

  • All tables, infoboxes, image captions, fig captions, UL, LIs should use Sans-Serif stack
  • Only H1, H2s, p, blockquote would be using serifs
  • No borders for images, thumbs, use mobilefrontend style
  • The list items in footer sections, further reading / references / notes should be the same.
  • I didn't mention the obvious breaking of H2s and infoboxes

I will keep testing and add stuff to this comment. but looks like we are pretty close :)

Change 367627 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Fix CSS selector output in print stylesheet

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

Change 367812 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Print mode: Define when to use serif and when to use sans-serif

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

Jdlrobson updated the task description. (Show Details)Jul 25 2017, 9:50 PM

Change 367814 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Print styles: Apply padding and margins to ol as well as ul

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

Change 367817 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] Print styles: Border bottom of headings should not overlap infoboxes

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

Jdlrobson updated the task description. (Show Details)Jul 25 2017, 10:24 PM
Jdlrobson updated the task description. (Show Details)Jul 25 2017, 11:24 PM
Jdlrobson updated the task description. (Show Details)Jul 25 2017, 11:32 PM

Please help with code review. I have a bunch of follow ups in the description after a 2.5hr session with @Nirzar - he's happy with the styles - so please let me know what needs improving to get these all merged!

Jdlrobson updated the task description. (Show Details)Jul 25 2017, 11:39 PM
Jdlrobson updated the task description. (Show Details)

Change 367814 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Print styles: Apply padding and margins to ol as well as ul

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

Change 367812 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Print mode: Define when to use serif and when to use sans-serif

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

Jdlrobson updated the task description. (Show Details)Jul 26 2017, 4:38 PM
pmiazga updated the task description. (Show Details)Jul 27 2017, 4:06 PM

Change 366967 merged by jenkins-bot:
[mediawiki/core@master] Fix font size and backgrounds in gallery

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

Change 366738 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Print styles: footer

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

TheDJ added a subscriber: TheDJ.Jul 28 2017, 11:56 AM

We would like to underline the <a> tags in print styles. it's better for accessibility as well.

Really ? There's a reason we don't underline to begin with, and that is because we have too many links to begin with. It creates a really 'busy' view, which generally is harder to read.

Why would you care if an article exists when you are reading it ? You can't click it (well in styled PDFs, but then we might as well keep the colors).

I think text should immediately follow a heading (as opposed to heading staying on the previous page):

I struggled with this a lot :( the page break: avoid property doesn't work at all :( :(
and we can't really use page break always because that adds million more pages
It would be amazing if we can actually solve this with css. The existing styles have this issue too

page-break-inside is for content inside elements, not for breaks between elements.

h1, h2, h3, h4, h5, h6 {
  page-break-after: avoid;
  break-after: avoid;
}

And that is actually already part of our print styles i believe But support seems... lacklustre...
Possibly because we also have: "page-break-before: avoid" on those elements, which doesn't really seem to make sense to me. Might be wise to modify that a bit (and to make sure we support the new break-after).

Change 367817 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Print styles: Border bottom of headings should not overlap infoboxes

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

also, staging has visible edit section links for me at this moment... ?

pmiazga updated the task description. (Show Details)Jul 28 2017, 1:01 PM
pmiazga updated the task description. (Show Details)Jul 28 2017, 2:22 PM

also, staging has visible edit section links for me at this moment... ?

Should be fixed by T171519 but I still need to verify :)

Change 366968 merged by jenkins-bot:
[mediawiki/core@master] Adjustments to print table styles

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

Jdlrobson updated the task description. (Show Details)Jul 31 2017, 5:07 PM
Jdlrobson updated the task description. (Show Details)

@Nirzar a question on the code review about this rule:

div.thumbinner { text-align: left; }

"sure about this ? This might mess with things like Template:Mutltiple_image which try to replicate thumbnails.. There isn't any text in this block normally. The only text should be thumb caption, which enforces it's own text-align specifically for this reason."

Can I safely remove?

Change 366971 merged by jenkins-bot:
[mediawiki/core@master] Adjustments to floating elements in print styles

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

Jdlrobson updated the task description. (Show Details)Jul 31 2017, 7:09 PM

Only https://gerrit.wikimedia.org/r/#/c/366970/ remains for review for which there's an open question for @Nirzar: https://phabricator.wikimedia.org/T169823#3487065

Once this is done we can then start reviewing these styles and working out what follows up are needed along with T172144.

@Nirzar a question on the code review about this rule:

can we add that rule to the thumbcaption then? I wanted captions for images, single or multiple to be left aligned with the image itself. right now the center align breaks the layout a bit.

can we add that rule to the thumbcaption then? I wanted captions for images, single or multiple to be left aligned with the image itself. right now the center align breaks the layout a bit.

It's already there. So yup I've retained it. Looks the same to me.

Anyone want to pull the trigger on https://gerrit.wikimedia.org/r/366971 so we can begin some in depth testing :)

Change 366970 merged by jenkins-bot:
[mediawiki/core@master] Adjust print styles for thumb

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

@Nirzar @ovasileva over to you. All changes are merged and on staging. On staging we're still using production data and the feature flag is enabled.
You can test with these on: http://reading-web-staging.wmflabs.org/wiki/Albert%20Einstein
Note: there are 2 issues with testing on staging - table of contents will not display here and footer will display incorrectly.

If you want to see print styles with the feature flag displayed you can test on the beta cluster e.g. https://en.wikipedia.beta.wmflabs.org/wiki/Albert_Einstein - this will help you understand what's feature flagged and A/B testable and what's not.

Hope that makes sense?

Please raise any tasks for follow up work as a subtask of T154965 and we'll make sure those get worked on together.

I'm now going to see if I can get table of contents to show up in staging...

Jdlrobson updated the task description. (Show Details)Jul 31 2017, 11:01 PM

Okay.. table of contents is now showing. In case you developers watching at home were curious

switching

$wgMFMwApiContentProviderBaseUri = "https://en.m.wikipedia.org/w/api.php";

to

$wgMFMwApiContentProviderBaseUri = "https://en.wikipedia.org/w/api.php";
$wgMFRemovableClasses = [
        "base"=>[".navbox",".nomobile"],
        "beta"=>[]
];

did the trick.
Table of contents will be showing up in mobile view of staging so let me know when you are done testing so I can revert this and avoid confusing people.

Jdlrobson updated the task description. (Show Details)Jul 31 2017, 11:01 PM
phuedx added a comment.Aug 1 2017, 7:30 AM

If you want to see print styles with the feature flag displayed you can test on the beta cluster e.g. https://en.wikipedia.beta.wmflabs.org/wiki/Albert_Einstein - this will help you understand what's feature flagged and A/B testable and what's not.

Can you document what is and what isn't testable on T169731: Set up print styles a/b test on all projects, please? Knowing the limits up front will better inform the design of the test.

phuedx added a comment.Aug 1 2017, 7:32 AM

This skipped the Needs QA column. Is @Nirzar responsible for QA here?

Jdlrobson reassigned this task from Jdlrobson to Nirzar.Aug 7 2017, 5:03 PM

Are we and if so how are we dealing with overly long strings (especially URLs):

e.g. if you want to print https://de.wikipedia.org/wiki/Benutzer:TMg/advancedSearch which includes some overly long strings, the line is not breaking and thus text is cut off. This should be easy to avoid by improving the stylesheet.

Two remarks:

  1. It seems not to be easy. One could try with overflow-wrap: break-word; word-wrap: break-word; but they could also break in cases in which it is not essential.
  2. It is sadly a common problem that also exists in LaTex etc.
Nirzar closed this task as Resolved.Aug 23 2017, 4:52 PM

Macro votecat: looks  good

QA for this is captured in a different card. Deployment is captured in a different card.

Signing off as the product matches the spec.

one last remaining subtask around Webfonts will be a followup....

Thank you all!

Nirzar changed the status of subtask T171114: Show important indicators in print styles from Open to Stalled.Aug 28 2017, 7:49 PM