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

Hide header nav on print (to fix print layout issues) #1001

Merged
merged 4 commits into from
Sep 17, 2024

Conversation

anandamaryon1
Copy link
Contributor

@anandamaryon1 anandamaryon1 commented Aug 27, 2024

Description

When printing pages that contain a header with navigation links, the print layout depends on the viewport width, leading to layout inconsistency (where the page is being 'zoomed out' to try fit the header, resulting in smaller printed text when printed from a wider browser/desktop).

Example:

page printed at desktop width page printed at mobile width
image image

And a live nhs.uk example:

page printed at desktop width page printed at mobile width
image image

I've tested on Chrome, Safari and Firefox and all present the same issue (with slightly different results).

I propose we hide the navigation items when printing.

This PR (hide the nav items):

page printed at desktop width page printed at mobile width
image image

Checklist

frankieroberto
frankieroberto previously approved these changes Aug 28, 2024
Copy link
Contributor

@frankieroberto frankieroberto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 🖨️

@anandamaryon1 anandamaryon1 added 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) header labels Sep 9, 2024
edwardhorsford
edwardhorsford previously approved these changes Sep 10, 2024
@frankieroberto
Copy link
Contributor

@anandamaryon1 @edwardhorsford thinking about it some more, I wonder if we should also hide everything outside the <main> tag when printing? ie also hide the back link and breadcrumbs?

@edwardhorsford
Copy link
Contributor

@frankieroberto that probably makes sense. Do you think there's any risk of teams putting useful content outside of main? (they shouldn't be!)

@frankieroberto
Copy link
Contributor

@frankieroberto that probably makes sense. Do you think there's any risk of teams putting useful content outside of main? (they shouldn't be!)

Feels low risk?

working out how to hide the breadcrumbs and back links from print is slightly complicated though - I think you'd have to hide everything within <div class="nhsuk-width-container"> and then un-hide everything within <main>?

@edwardhorsford
Copy link
Contributor

I think you could probably do a selector that targets siblings to main. But we'd want to test it carefully - I'd suggest doing as a separate pr for consideration.

@anandamaryon1
Copy link
Contributor Author

The breadcrumbs already have styles to hide themselves on print but the back link doesn't seem to (I'm not sure why).

It does feel a little risky to hide everything outside main, eg. perhaps an aside containing related sections could be useful content.

I'm thinking to keep this PR focussed on this bug, other print styling can maybe be done later, but I'd like to get this in now if you approve?

@anandamaryon1 anandamaryon1 merged commit 4a3f3cb into main Sep 17, 2024
2 checks passed
@anandamaryon1 anandamaryon1 deleted the print-hide-header-nav branch September 17, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) header
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants