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

Improve Pagination component print styles #2800

Merged
merged 2 commits into from
Sep 14, 2022

Conversation

NickColley
Copy link
Contributor

@NickColley NickColley commented Aug 19, 2022

By default .govuk-link print styles will output the href attribute
in brackets after the link. For example: "Home (/homepage)".

This is done with the ::after pseudo selector.
So when this was styled to increase the touch area of the pagination link
it resulted in the bracketed information overlapping the component in
a difficult to read way.

By only applying this touch area improvement in the screen media context
we avoid overwriting this inherited print style and allow the default
.govuk-link behaviour to apply as usual.

In practice I think many GOV.UK Frontend users might hide this component from their print views so impact on end-users is low.

I'd be down for doing something more holistic as part of #573 if you're interested in that contribution (including thinking about it from a more high level design pov), but I noticed this just at a quick glance.

Screenshots

Before After
Print mode shows overlapping links Print mode has pagination links are shown inline without overlap

By default `.govuk-link` print styles will output the `href` attribute
in brackets after the link. For example: "Home (/homepage)".

This is done with the `::after` pseudo selector.
So when this was styled to increase the touch area of the pagination link
it resulted in the bracketed information overlapping the component in
a difficult to read way.

By only applying this touch area improvement in the `screen` media context
we avoid overwriting this inherited print style and allow the default
`.govuk-link` behaviour to apply as usual.
@NickColley NickColley changed the title Improve pagination print styles Improve Pagination component print styles Aug 19, 2022
@NickColley NickColley marked this pull request as ready for review August 19, 2022 14:53
@owenatgov
Copy link
Contributor

Hi @NickColley! Finally getting into this.

Very much appreciate the bugfix, deffo something we should've spotted during development 😅 However I'm going to be irritating and say this has prompted me to wonder if we should even show pagination on print and I wonder if it makes more sense to amend this PR to just hide it outright. What are your thoughts? If we wanted to just hide the whole thing, do you have capacity to amend this PR?

@NickColley
Copy link
Contributor Author

NickColley commented Sep 1, 2022

@owenatgov my high level thought at this point is this:

  1. Show all components with print friendly styles by default
  2. Allow the user to then decide which components should be hidden themselves.

Either with a CSS utility or sass variables.

$govuk-accordion-print-styles: none;
@import govuk-frontend/all;

I think this sort of thing is what I was getting at with the wider piece of work I'm interested in doing. I'd put together some proposals and we could weigh up the approach.

That's what I was thinking by the extended piece of work. But what I've shared here is sort of how people are doing it in practice if you look at GOV.UK Publishing for example.

@NickColley
Copy link
Contributor Author

So if you agree we should have a more thought out approach for print styles then we could get this in as is and then explore print styles as a whole as a bigger contribution.

@NickColley
Copy link
Contributor Author

NickColley commented Sep 1, 2022

Maybe something like:

  1. All components have functionally working print styles
  2. By default some components are hidden, e.g. pagination.
  3. User can override the defaults at a per-component basis using Sass variables, e.g. $govuk-pagination-print-styles: enabled and (if needed) a global $govuk-default-print-styles: false.
  4. Write some guidance to explain all of this alongside Sass docs

Or

  1. All components have functionally working print styles.
  2. We have a CSS utility and/or Sass flags to hide them, leave this entirely up to users.
  3. Write some guidance to explain all of this.

I prefer the first one because I imagine most users wont think to configure this...

@querkmachine
Copy link
Member

Another bigger picture thing, but we may also have to consider manipulating the page content in preparation for printing too, such as expanding Details/Accordion sections. Conversely, doing that could be undesirable if a user is only interested in the content of one part of the page.

@NickColley
Copy link
Contributor Author

@querkmachine that reminded me of something Frankie said about how they intentionally made accordions keep the state they are on the page when printed with the idea that the user might want to control that themselves.

@owenatgov
Copy link
Contributor

@NickColley I think this makes sense. Let's get this in for now to clear it up and start thinking about a more consistent print strategy soon. I'll review this next.

Copy link
Contributor

@owenatgov owenatgov left a comment

Choose a reason for hiding this comment

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

📄

@owenatgov
Copy link
Contributor

Going to hassle the team for a 2nd review and then we're good to go on this as far as I'm concerned

@owenatgov owenatgov added this to Needs review 🔍 in Design System Sprint Board via automation Sep 7, 2022
@owenatgov owenatgov requested a review from a team September 14, 2022 11:38
Copy link
Contributor

@domoscargin domoscargin left a comment

Choose a reason for hiding this comment

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

Yup, this looks good to me, and agree we should have a think about print styles later.

@NickColley
Copy link
Contributor Author

Thanks for your time reviewing this, I'll leave it with you to merge as I have no power to merge I am a humble contributor :P

@owenatgov owenatgov merged commit 54afba6 into alphagov:main Sep 14, 2022
Design System Sprint Board automation moved this from Needs review 🔍 to Done 🏁 Sep 14, 2022
@owenatgov owenatgov moved this from Done 🏁 to Ready to release 🚀 in Design System Sprint Board Sep 14, 2022
@NickColley NickColley deleted the fix-pagination-print-styles branch September 14, 2022 14:14
@36degrees 36degrees added this to the [NEXT] milestone Sep 15, 2022
@romaricpascal romaricpascal mentioned this pull request Nov 10, 2022
@36degrees 36degrees moved this from Ready to release 🚀 to Done 🏁 in Design System Sprint Board Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants