Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add print view for travel advice #274

Merged
merged 3 commits into from Mar 8, 2017
Merged

Add print view for travel advice #274

merged 3 commits into from Mar 8, 2017

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Mar 3, 2017

https://trello.com/c/MluRcP1W/138-2-render-travel-advice-print-page-in-government-frontend-m

  • Add route for handling print requests
  • Render a print view showing all parts
  • Use variants to indicate which template to render
  • Render summary and all parts using standard template rather than slimmer’s print version – this applies the print styles we use across GOV.UK rather than the broken bespoke slimmer ones
  • Indicate that the link to the print page is an alternate version
  • Don’t index the print version
  • Add “Print” to title element for unique title

Note that we opted against general line-length improvements to print: alphagov/static#913

When reviewing I recommend comparing the PDF versions. (ProTip: You can drag and drop PDFs to PRs)

Old New
Old print view (PDF) New print view (PDF)
old-print-view new-print-view
fofr added 3 commits Feb 27, 2017
* Put route before travel advice, otherwise `/print` considered to be a
part of the guide
The summary will be re-used by the print view
* Use variants to indicate which template to render
* Render summary and all parts using standard template rather than
slimmer’s print version – this applies the print styles we use across
GOV.UK rather than the broken bespoke slimmer ones
* Indicate that the link to the print page is an alternate version
* Don’t index the print version
* Use custom “Print” title element
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-274 Mar 3, 2017 Inactive
@mcgoooo
mcgoooo approved these changes Mar 3, 2017
Copy link
Contributor

@mcgoooo mcgoooo left a comment

minor non blocking nit pick

content_for :simple_header, true
content_for :extra_head_content do %>
<meta name="robots" content="noindex, nofollow">
<script>window.onload = function() { window.print(); }</script>

This comment has been minimized.

@mcgoooo

mcgoooo Mar 3, 2017
Contributor

should we maybe put this in an external ajavscript for when we do csp?

This comment has been minimized.

@fofr

fofr Mar 3, 2017
Author Contributor

Seems like overkill. I'd expect to use a nonce, etc. Can fix when we use CSP. (This is a direct port)

This comment has been minimized.

@mcgoooo

mcgoooo Mar 3, 2017
Contributor

👍

@nickcolley
Copy link
Contributor

@nickcolley nickcolley commented Mar 6, 2017

I'm not sure we should include the govuk_template layout in the new version?

@fofr
Copy link
Contributor Author

@fofr fofr commented Mar 7, 2017

@nickcolley What's your specific objection to it?

  1. That the print view doesn't look different to other pages, before being printed?
  2. The risk that the header/footer will be accidentally printed?

Or something else?

@nickcolley
Copy link
Contributor

@nickcolley nickcolley commented Mar 7, 2017

I don't have a good objection other than it feels odd to have a 'print' page that could be confused for a normal page.

@mcgoooo
Copy link
Contributor

@mcgoooo mcgoooo commented Mar 8, 2017

Generally i think this is a good idea, it is slightly odd, but the user owudl only see it on accident anyway, so good to give them an option to navigate away

@nickcolley nickcolley merged commit dac8b1a into master Mar 8, 2017
2 checks passed
2 checks passed
continuous-integration/jenkins/branch This commit looks good
Details
security/snyk No new vulnerabilities
Details
@nickcolley nickcolley deleted the travel-advice-print branch Mar 8, 2017
@nickcolley
Copy link
Contributor

@nickcolley nickcolley commented Mar 8, 2017

I've made a card to look into making the print view look more like a print view if you cancel the print dialog etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.