Page MenuHomePhabricator

Make PHUITwoColumnView a little more printable
ClosedPublic

Authored by avivey on Sep 9 2017, 3:53 AM.

Details

Summary

Hide navbar, and make curtain behave like on a phone, when printing.

Test Plan

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

avivey created this revision.Sep 9 2017, 3:53 AM
avivey edited the test plan for this revision. (Show Details)Sep 9 2017, 3:54 AM
avivey added a comment.Sep 9 2017, 4:07 AM

The build failure looks like some setup issue - it failed to start.

I think we had a task somewhere for "make it easier to print tasks", and this will make it so, but mostly I want this for printing special pages - e.g., https://secure.phabricator.com/phortune/ would look much better when printed.

webroot/rsrc/css/phui/phui-curtain-view.css
24–35

I couldn't use !print here, because it would have to be .printable.device-desktop (no space), and also !print doesn't support !print a, !print b { syntax, so I had to write it twice anyway.

I think the build failure is because of this (e.g., if you click "Land Revision"):

When the diff was generated, the client was not able to determine which repository it belonged to, so the change was not pushed to staging. Changes must be pushed to staging before they can be landed from the web.

I'd guess your origin is a GitHub fork or something? Or arc which should be more illuminating. Slightly unclear because the revision now has the correct repository.

But the failure mode for "nothing got pushed to staging" is currently the muddy mess in Harbormaster.

I think it's probably better to write the rule itself twice using !print than to use @media print. If there's a mistake with this way of doing things (say, a font-color: blinking red explosion that gets typo'd in the @media print rule) it won't be visible on ?__print__=true, and ?__print__=true and the actual printed output will differ.

If you write the rules twice instead, __print__ and actual printing will always use the same rules. Since the two approaches seem similar otherwise, that seems like an advantage for !print.

I'd guess your origin is a GitHub fork or something?

Yes, something like that; that explains it.


I think it's probably better to write the rule itself twice using !print...

I'll try it again, but I think I had some strange problem with that.

avivey updated this revision to Diff 44760.Sep 25 2017, 6:55 PM

Check if I'm set up currectly re:staging...

avivey planned changes to this revision.Sep 25 2017, 6:55 PM
avivey updated this revision to Diff 44761.Sep 25 2017, 7:23 PM
  • Use !print more
  • Also remove background when printing
avivey edited the test plan for this revision. (Show Details)Sep 25 2017, 7:24 PM
avivey added inline comments.Sep 25 2017, 7:27 PM
webroot/rsrc/css/core/core.css
64

.printable goes on body, so it doesn't answer to .printable body...

avivey updated this revision to Diff 44762.Sep 25 2017, 7:30 PM

remove border-top

avivey edited the test plan for this revision. (Show Details)Sep 25 2017, 7:31 PM

There are 4 copies of the important rules because:

  1. !print transformation doesn't handle selector, selector rules, which I guess could be fixed in some way.
  2. At least in my tests, .device-desktop .phabricator-action-list-view + .phui-curtain-panel {...} beats @media print { .phabricator-action-list-view + .phui-curtain-panel { ... }}, so it needs the !print .device-desktop variant.
  3. In my Chrome, the print layout depends on the width of the window that prints - if the window is narrow enough for .device, the print css gets the .device tag. I'm not sure if this is javascript or media calculations? That makes the !print w/o device-desktop needed (Although I guess since it's mostly the same rules as the .device variants, we could get rid of those?)
chad removed a reviewer: chad.Sep 25 2017, 7:39 PM
epriestley accepted this revision.Sep 25 2017, 7:43 PM
This revision is now accepted and ready to land.Sep 25 2017, 7:43 PM
This revision was automatically updated to reflect the committed changes.