EL-986 make pdf titles single blocks #795

willc-work · 2023-07-07T09:39:06Z

Jira ticket
as covered in the comments, the problem is related to ligatures (@exonian I believe you called this as a potential problem early on, bit I evidently missed that). The css fix to the print page prevents the split on fi character pairs

removed the partial being used to display the service name as a H1 and added a H2 directly to the print page.

as per:
https://usability.yale.edu/web-accessibility/articles/headings
https://www.w3.org/WAI/tutorials/page-structure/headings/#main-heading-after-navigation
there is no problem with a H2 appearing before a H1 and the H1 should be the most important part, which for us is the result.

The issue with the H2 being split (1.4.1) cannot be exactly replicated. The fix for 1.3.1 also prevents splits on this H2 but the PDF splits anything that crosses more than 1 line. For spans this isn't really an issue but for headings this will result in each line being announced as a heading. The fix for this in this case is to prevent wrapping. It doesnt cause any issues with the rest of the display, although the heading looks slightly smaller on the page.

Guidance to review

Checklist

Before you ask people to review this PR:

Tests and rubocop should be passing
Branch is generally up to date with main Github - definitely no conflicts
No unnecessary whitespace changes. These make diffs harder to read and conflicts more likely.
PR description says what changed and why, with a link to the JIRA story.
Diff has been checked for unexpected changes being included.
Commit messages say why the change was made.

patrick-laa · 2023-07-07T10:07:54Z

@willc-work have you verified that this fixes the accessibility issue? It looked from the ticket like the splits being reported in the accessibility audit didn't necessarily line up with visible line breaks, so I wasn't sure if fixing the line breaks would fix the a11y problem.

app/views/estimates/print.html.slim

app/services/pdf_service.rb

willc-work · 2023-07-07T10:38:15Z

@patrick-laa the ticket indicates that the issue is that the title is split into 3 sections, each of which are given a H1, so there is a single heading but the screenreader splits it into 3 and announces each one e.g
"Heading 1 check if your client quali"
"Heading 1 fies for" ....
I dont have access to NVDA and Voiceover doesnt seem to like PDFs. So my assumption is that this fixes the issue with the first title because now the PDF has a single span containing the title, so should be read out as "Heading 1 check if your client qualifies for legal aid"

patrick-laa · 2023-07-07T11:02:17Z

@willc-work the screenshot suggests that the split is "Check if your client quali" "fi" "es for legal aid". If this split was due to line breaks I'd be surprised, simply because I don't think any linebreak algorithm would break "qualifies" into 3 like that. So I'm not sure that line breaks are the source of the problem.

patrick-laa · 2023-07-07T11:08:21Z

@MazOneTwoOne do you now have working Adobe? If so, can you download a PDF from https://el-986-pdf-a11y-check-client-qualifies-legal-aid-uat.cloud-platform.service.justice.gov.uk/ and confirm that the reported bizarre "fi" split etc is fixed?

willc-work · 2023-07-07T11:22:52Z

@patrick-laa if I only have the line:
media_features: [{ name: "prefers-color-scheme", value: "dark" }]
and remove scale , this fixes the weird line break, so that line breaks are applied at the end of the line. But I am not sure if this will fix the screenreader issue as it might still read out Heading 1 before each line.

MazOneTwoOne · 2023-07-07T11:48:23Z

@MazOneTwoOne do you now have working Adobe? If so, can you download a PDF from https://el-986-pdf-a11y-check-client-qualifies-legal-aid-uat.cloud-platform.service.justice.gov.uk/ and confirm that the reported bizarre "fi" split etc is fixed?

Hey Both,

Still don't have Adobe Pro (for accessibility checks/tree) or NVDA (Windows screen reader) to verify this.

What I did notice, is that these changes have also fixed iOS screen readers, being able to announce numbers but added in another issue with iOS screen readers.

If the table goes over the the page then the iOS screen reader can't announce the rest of the table on the other page when opening with Adobe.

willc-work · 2023-07-07T13:03:27Z

@patrick-laa ok I have signed up for a 14 day free trial at https://assistivlabs.com/ and done some more testing. Even though I am able to get the title into a single span locally, when I try it on UAT it splits it again into 3 sections it also does this on the second heading mentioned on the ticket.

As noted the split is really weird, but in both sections fi is set on its own. Is it possible that whatever is splitting the words is reading fi as an end statement as in LINUX if statement?
https://www.thegeekstuff.com/2010/06/bash-if-statement-examples/

Confirmed that it is this combination or characters that is causing the strange behaviour, I think this is a puppeteer (as opposed to grover) issue.

patrick-laa · 2023-07-07T16:32:34Z

@willc-work looks like someone else experienced 'fi' weirdness - puppeteer/puppeteer#4125 (comment), and suggested font-variant-ligatures: none; in CSS would fix. If this is our issue, props to @exonian who suggested it was a ligature issue ages ago.

Although that would only explain the h1 issue, not the h2 issue (and no idea why the h2, which also has a "fi", doesn't also have a split there)

app/assets/stylesheets/print.scss

patrick-laa

OK cool, so if this solves the fi split mentioned in 1.3.1 of the report that's great. Do we think that the h2 issue they mentioned 1.4.1, which is a split without a 'fi', is completely separate?

Also, they say in the 1.3.1 bit that it would be better if there was only one h1 element. With this fix we've got it down from 4 to 2. But there actually are 2 h1 elements on the page - both "Check if your client qualifies...." and "Your client is likely to qualify for...". Would it be a good idea, as a bonus thing, to fix it so that the first one is an h1 and the second is an h2?

willc-work · 2023-07-11T14:37:57Z

The h2 issue is the same issue isn't it? The report says it is split in 3 parts but I am actually seeing it split into 4 parts, with the 2 instances of fi that occur in finacially and certificated causing the splits. This fix works on that sentence too.

re: the the multiple h1's I will go back and have a look at that.

patrick-laa · 2023-07-11T14:42:03Z

@willc-work I'm not sure it is the same issue. What we see on their report (not on our own replication in adobe) is that with the h1, the "fi" bit is split into its own line:

But on the h2 the split is in other places:

patrick-laa · 2023-07-13T08:45:57Z

@willc-work the problem with a no-wrap is that this CSS also controls the "print" view, and it means on the print preview screen, and if you try to print, the text runs off the side of the page:

willc-work · 2023-07-13T13:43:09Z

@patrick-laa the issue with the no-wrap appears to be browser specific. I have added in a scale: attribute to grover_options so that the page is scaled in browsers that dont automatically do this so that all of the wording fits on the line and is printed as exopected

patrick-laa · 2023-07-14T08:24:05Z

app/services/pdf_service.rb

@@ -10,6 +10,7 @@ class PdfService
    emulate_media: "screen",
    launch_args: ["--font-render-hinting=medium", "--no-sandbox"],
    execute_script: "document.querySelectorAll('button').forEach(el => el.style.display = 'none')",
+    scale: 0.75 / 1.0,


This doesn't fix the issue with trying to print the page on Chrome that I screenshotted before (because printing doesn't involve PdfService at all).

I've got another theory. The line breaks were occurring because when grover/puppeteer "looks at" the page, it's using a small viewport, narrow enough that the h2 line was being broken up. But it didn't actually insert visible linebreaks at those points, they only appeared in the underlying structure exposed by the accessibility tool. So we can infer that: the size of the viewport affects accessibility data without affecting the look of the page.

So how about you try removing the "no-wrap" CSS (to fix the print view in Chrome), but then setting the grover viewport to be super wide (the default is only 640px - https://github.com/Studiosity/grover#configuration) so that when it's viewing the page the header doesn't have a linebreak in it?

Happy to give this a try, but the scale solution does fix the issue from your screenshot.

I'm afraid it doesn't fix the issue - your screenshot above is of something different.. I'm talking about printing the results using the 'Print' button on the results page, NOT printing the PDF. As I say, this is completely unaffected by how you scale the PDF, because there is no PDF involved.

New issue identified with print screen.

willc-work · 2023-07-18T08:37:12Z

@patrick-laa apologies, you are correct, for some reason i cant use the print from page button on firefox or chrome either locally or on UAT.

app/views/estimates/print.html.slim

patrick-laa · 2023-07-19T08:08:12Z

app/services/pdf_service.rb

@@ -7,6 +7,10 @@ class PdfService
      left: "1cm",
      right: "1cm",
    },
+    viewport: {
+      width: 800,


Has this had any effect, or am I barking up the wrong tree? If it works the way I hoped it works, changing the width from 640 to 800 wouldn't get rid of all breakpoints in the accessibility metadata, but we'd maybe see them shift along a bit (without affecting how the PDF actually looked). Then it would just be a case of finding a width that was enough to remove them entirely.

dont use partial for service name add scale for chrome print to PDF

willc-work requested a review from a team as a code owner July 7, 2023 09:39

willc-work force-pushed the el-986-pdf-a11y branch 2 times, most recently from ab37ef4 to 6653ff3 Compare July 7, 2023 09:42

patrick-laa reviewed Jul 7, 2023

View reviewed changes

app/views/estimates/print.html.slim Outdated Show resolved Hide resolved

app/services/pdf_service.rb Outdated Show resolved Hide resolved

willc-work force-pushed the el-986-pdf-a11y branch 2 times, most recently from 5b693ca to ea85a2f Compare July 7, 2023 10:53

MazOneTwoOne closed this Jul 7, 2023

patrick-laa reopened this Jul 7, 2023

willc-work force-pushed the el-986-pdf-a11y branch 2 times, most recently from 55a270d to ea85a2f Compare July 7, 2023 13:33

willc-work force-pushed the el-986-pdf-a11y branch 2 times, most recently from ecebea9 to c6ac04f Compare July 7, 2023 19:29

patrick-laa reviewed Jul 10, 2023

View reviewed changes

app/assets/stylesheets/print.scss Outdated Show resolved Hide resolved

willc-work force-pushed the el-986-pdf-a11y branch 2 times, most recently from ed11623 to f2d26a7 Compare July 11, 2023 11:09

willc-work added the Ready For Review label Jul 11, 2023

patrick-laa previously approved these changes Jul 11, 2023

View reviewed changes

willc-work force-pushed the el-986-pdf-a11y branch 3 times, most recently from 4d5a054 to 2af8f5b Compare July 11, 2023 16:45

willc-work force-pushed the el-986-pdf-a11y branch 4 times, most recently from db9087e to ba09d31 Compare July 13, 2023 13:35

patrick-laa reviewed Jul 14, 2023

View reviewed changes

patrick-laa added question Further information is requested and removed Ready For Review labels Jul 17, 2023

willc-work force-pushed the el-986-pdf-a11y branch 2 times, most recently from 683b022 to 6afe33e Compare July 18, 2023 08:35

willc-work added Ready For Review and removed question Further information is requested labels Jul 18, 2023

patrick-laa reviewed Jul 19, 2023

View reviewed changes

willc-work force-pushed the el-986-pdf-a11y branch 4 times, most recently from b98bba6 to 32523d2 Compare July 19, 2023 19:25

patrick-laa approved these changes Jul 20, 2023

View reviewed changes

patrick-laa added Approved and removed Ready For Review labels Jul 20, 2023

willc-work force-pushed the el-986-pdf-a11y branch 2 times, most recently from 7ed9bb8 to f56c85f Compare July 25, 2023 10:43

css fix for puppetteer bug

beff761

dont use partial for service name add scale for chrome print to PDF

willc-work force-pushed the el-986-pdf-a11y branch from f56c85f to beff761 Compare July 25, 2023 10:44

willc-work merged commit ea03b27 into main Jul 25, 2023
3 checks passed

willc-work deleted the el-986-pdf-a11y branch July 25, 2023 10:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EL-986 make pdf titles single blocks #795

EL-986 make pdf titles single blocks #795

willc-work commented Jul 7, 2023 •

edited

Loading

patrick-laa commented Jul 7, 2023

willc-work commented Jul 7, 2023

patrick-laa commented Jul 7, 2023

patrick-laa commented Jul 7, 2023

willc-work commented Jul 7, 2023

MazOneTwoOne commented Jul 7, 2023 •

edited

Loading

willc-work commented Jul 7, 2023 •

edited

Loading

patrick-laa commented Jul 7, 2023

patrick-laa left a comment

willc-work commented Jul 11, 2023 •

edited

Loading

patrick-laa commented Jul 11, 2023

patrick-laa commented Jul 13, 2023

willc-work commented Jul 13, 2023

patrick-laa Jul 14, 2023

willc-work Jul 14, 2023

patrick-laa Jul 17, 2023 •

edited

Loading

willc-work commented Jul 18, 2023

patrick-laa Jul 19, 2023

EL-986 make pdf titles single blocks #795

EL-986 make pdf titles single blocks #795

Conversation

willc-work commented Jul 7, 2023 • edited Loading

Guidance to review

Checklist

patrick-laa commented Jul 7, 2023

willc-work commented Jul 7, 2023

patrick-laa commented Jul 7, 2023

patrick-laa commented Jul 7, 2023

willc-work commented Jul 7, 2023

MazOneTwoOne commented Jul 7, 2023 • edited Loading

willc-work commented Jul 7, 2023 • edited Loading

patrick-laa commented Jul 7, 2023

patrick-laa left a comment

Choose a reason for hiding this comment

willc-work commented Jul 11, 2023 • edited Loading

patrick-laa commented Jul 11, 2023

patrick-laa commented Jul 13, 2023

willc-work commented Jul 13, 2023

patrick-laa Jul 14, 2023

Choose a reason for hiding this comment

willc-work Jul 14, 2023

Choose a reason for hiding this comment

patrick-laa Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

willc-work commented Jul 18, 2023

patrick-laa Jul 19, 2023

Choose a reason for hiding this comment

willc-work commented Jul 7, 2023 •

edited

Loading

MazOneTwoOne commented Jul 7, 2023 •

edited

Loading

willc-work commented Jul 7, 2023 •

edited

Loading

willc-work commented Jul 11, 2023 •

edited

Loading

patrick-laa Jul 17, 2023 •

edited

Loading