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

Update Marked and switch to Github Flavoured Markdown #2427

Merged
merged 6 commits into from
Nov 29, 2022
Merged

Conversation

domoscargin
Copy link
Contributor

@domoscargin domoscargin commented Nov 18, 2022

What

We use @metalsmith/in-place to transform our source markdown and nunjucks files.

Under the hood, @metalsmith/in-place uses jstransformer, which in turn relies on jstransformer-nunjucks and jstransformer-marked to handle our Nunjucks and markdown code respectively.

Previously, we've been using jstransformer-marked v1.0.3, not realising that it was using an outdated version of marked (0.3.9), and not the direct dependency that we've specified. This means that if we try to update, we generate a lot of rendering errors (at one point I had a 100,000+ line diff) because of various bugs being ironed out in more recent versions of marked.

Stick with pedantic rules?

Previously, we've used pedantic rules, but some of our usage is incorrect, flying under the radar because of marked bugs/changes. If we stick with pedantic rules and update marked, we generate some errors, including:

This last point is not easily fixable with pedantic rules in place: we could mess around with indentation, but it'd be tricky to specify code languages for highlighting, and difficult to enforce.

Switch to Github Flavoured Markdown!

Switching to GFM solves the 3 problems listed above, and is arguably more recognisable to our team as we're using the Github ecosystem.

However, it introduces a new problem:

  • Our .md.njk share Nunjucks and markdown code
  • @metalsmith/in-place renders the Nunjucks, then does another pass to render the markdown
  • The Nunjucks is often rendered with empty blank lines. In some cases, I think this enhances readability a bit. In other cases, it's just due to not controlling white space within our code.
  • These empty lines (despite being in a <pre> block!) run afoul of GFM's HTML Block rules, specifically rule 6 and 7. I've done a lot of messing about with the <pre> blocks in the _example.njk template to no avail.
  • As a result, code in <pre> blocks can be wrapped in <p> tags, and code outside of <pre> blocks can be treated as raw preformatted code.

How

The bulk of this PR is fixing these spacing errors so that the new output differs from the old output only in the following ways:

  • The single email we were mangling is no longer mangled (we don't mangle any others)
  • Autogenerated heading ids concatenate contractions
    • GOV.UK becomes govuk rather than gov-uk
    • You're become youre rather than you-re
  • Headings ending in quotes don't generate an extra dash at the end of the id (eg: <h4 id="users-that-navigate-using-elements-lists-">Users that navigate using ‘elements lists’</h4>)
  • Repeated headings get numbers appended after the duplication (there's only one example of this, so we should decide whether to fix it!)
  • Non-breaking spaces in text content remain in place rather than being replaced by a normal space
  • highlight.js now uses language-[language] in class names instead of lang-[language]
  • blank line removal
  • whitespace differences to prevent blank lines

Method

To test these changes, I ran an npm run build command using the latest main branch, copied the resulting deploy/public folder, then diffed subsequent builds against that folder, ignoring whitespace and blank line changes (i.e: by running diff -r -w -B deploy/mainpublic deploy/public)

I'll attach this diff in a comment on this PR, as one of the issues with this work is there was no built-in way to do this diffing.

Concerns/questions

Am I missing something?

I've spent a lot of time playing around with the <pre> tags around the sample code in views/partials/_example.njk in order to get the sample code rendering fine without having to mess around with spacing (there's a possible issue where a <pre> tag needs a blank line before it to register as a new block of HTML), but I haven't been able to jig it into shape.

Are we concerned about the loss of blank lines in code examples?

  • This largely feels like a positive change with HTML code, which is now nicely beautified all the time rather than occasionally having random whitespace/blank lines
  • But less clear cut with Nunjucks

Character Count HTML Before

Before: Character count HTML code

Character Count HTML After

After: Character count HTML code

Character Count Nunjucks Before

Before: Character count Nunjucks code

Character Count Nunjucks After

After: Character count Nunjucks code

Fragility

We only noticed this issue because a user spotted an error when we tried bumping this before. We have multiple content editors - it's inevitable small issues will crop up again the future unless we guard against it.

Possible future work

Beautify and validate all compiled HTML

I've got a rough idea of what this might look like here: https://github.com/alphagov/govuk-design-system/pull/2408/files, though we could make the validation part of testing only.

Validating the compiled HTML would give us one layer of assurance that we're not deploying anything weird.

Diff the public folder on PRs?

It'd be great if we could see what changes our work would create in compiled code. Don't think we're going to add deploy/public to version control, so it makes sense to diff it to make sure we haven't done anything weird.

Split out sample code rendering

A lot of the issues stemmed from rendering problems in our example component's sample code blocks. It'd be great if we could somehow render page nunjucks source code, then markdown, then examples. This would give us full control over what code samples looked like without having to worry about markdown issues.

Rename all .md.njk files to .md files

It's far easier to edit .md files in Github (which non-devs are largely encouraged to do), and we can generate previews for them.

To make this work with the current set up, we could use metalsmith-renamer to rename the relevant files to .md.njk before passing them to @metalsmith/in-place. This would happen under the hood, so users would only have to deal with the .md files.

I've tested this locally and it works fine and doesn't seem to add much build time at all.

Switch to another site generator

Investigating this is on our radar, and at a glance, I think 11ty is set up to handle our "nunjucks in markdown" set up with minimal fuss or config, so it'd definitely make sense to spike that.

To those of you who've made it this far, thank you for reading! Hope that all made sense - feel free to shoot me any questions.

@netlify
Copy link

netlify bot commented Nov 18, 2022

You can preview this change here:

Name Link
🔨 Latest commit 1a615e7
🔍 Latest deploy log https://app.netlify.com/sites/govuk-design-system-preview/deploys/6385478fc5777d000848b08c
😎 Deploy Preview https://deploy-preview-2427--govuk-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@domoscargin
Copy link
Contributor Author

diff -r -w -B deploy/currentpublic/accessibility/index.html deploy/public/accessibility/index.html
974c974
< <h2 id="accessibility-statement-for-the-gov-uk-design-system-website">Accessibility statement for the GOV.UK Design System website</h2>
---
> <h2 id="accessibility-statement-for-the-govuk-design-system-website">Accessibility statement for the GOV.UK Design System website</h2>
1009c1009
< <h3 id="how-the-gov-uk-design-system-team-makes-this-website-accessible">How the GOV.UK Design System team makes this website accessible</h3>
---
> <h3 id="how-the-govuk-design-system-team-makes-this-website-accessible">How the GOV.UK Design System team makes this website accessible</h3>
diff -r -w -B deploy/currentpublic/community/design-system-working-group/index.html deploy/public/community/design-system-working-group/index.html
1096c1096
< <p>In the meantime, if you are interested in joining the working group email the Design System team at <a href="mailto:&#x67;&#111;&#x76;&#x75;&#x6b;&#45;&#x64;&#101;&#115;&#x69;&#x67;&#110;&#45;&#115;&#x79;&#115;&#116;&#101;&#109;&#45;&#115;&#x75;&#112;&#x70;&#111;&#114;&#116;&#64;&#x64;&#105;&#103;&#105;&#x74;&#97;&#108;&#46;&#99;&#x61;&#98;&#x69;&#110;&#x65;&#x74;&#x2d;&#111;&#x66;&#102;&#105;&#x63;&#101;&#x2e;&#103;&#111;&#118;&#46;&#x75;&#107;">&#x67;&#111;&#x76;&#x75;&#x6b;&#45;&#x64;&#101;&#115;&#x69;&#x67;&#110;&#45;&#115;&#x79;&#115;&#116;&#101;&#109;&#45;&#115;&#x75;&#112;&#x70;&#111;&#114;&#116;&#64;&#x64;&#105;&#103;&#105;&#x74;&#97;&#108;&#46;&#99;&#x61;&#98;&#x69;&#110;&#x65;&#x74;&#x2d;&#111;&#x66;&#102;&#105;&#x63;&#101;&#x2e;&#103;&#111;&#118;&#46;&#x75;&#107;</a>.</p>
---
> <p>In the meantime, if you are interested in joining the working group email the Design System team at <a href="mailto:govuk-design-system-support@digital.cabinet-office.gov.uk">govuk-design-system-support@digital.cabinet-office.gov.uk</a>.</p>
diff -r -w -B deploy/currentpublic/community/resources-and-tools/index.html deploy/public/community/resources-and-tools/index.html
1108c1108
< <h3 id="asp-net-mvc">ASP.NET MVC</h3>
---
> <h3 id="aspnet-mvc">ASP.NET MVC</h3>
1111c1111
< <h3 id="node-js">Node.js</h3>
---
> <h3 id="nodejs">Node.js</h3>
diff -r -w -B deploy/currentpublic/components/accordion/index.html deploy/public/components/accordion/index.html
2303c2276
< <h4 id="users-that-navigate-using-elements-lists-">Users that navigate using ‘elements lists’</h4>
---
> <h4 id="users-that-navigate-using-elements-lists">Users that navigate using ‘elements lists’</h4>
diff -r -w -B deploy/currentpublic/components/character-count/index.html deploy/public/components/character-count/index.html
2012c1978
< <h3 id="if-you-re-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
---
> <h3 id="if-youre-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
diff -r -w -B deploy/currentpublic/components/checkboxes/index.html deploy/public/components/checkboxes/index.html
1641c1623
< <h3 id="if-you-re-asking-one-question-on-the-page">If you’re asking one question on the page</h3>
---
> <h3 id="if-youre-asking-one-question-on-the-page">If you’re asking one question on the page</h3>
2108c2072
< <h3 id="if-you-re-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
---
> <h3 id="if-youre-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
3040c2968
< <h3 id="add-an-option-for-none-">Add an option for ‘none’</h3>
---
> <h3 id="add-an-option-for-none">Add an option for ‘none’</h3>
diff -r -w -B deploy/currentpublic/components/cookie-banner/index.html deploy/public/components/cookie-banner/index.html
1170,1171c1170
< 
<           <span class="govuk-visually-hidden">default – cookie banner</span>
---
>           <span class="govuk-visually-hidden">default – cookie banner</span>
1175c1174
<       <iframe title="Default – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/default/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Default – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/default/index.html" frameBorder="0" loading="lazy"></iframe>
1513c1499
< <h3 id="option-1-if-you-re-only-using-essential-cookies">Option 1: If you’re only using essential cookies</h3>
---
> <h3 id="option-1-if-youre-only-using-essential-cookies">Option 1: If you’re only using essential cookies</h3>
1526,1527c1512
< 
<           <span class="govuk-visually-hidden">server-side implementation – cookie banner</span>
---
>           <span class="govuk-visually-hidden">server-side implementation – cookie banner</span>
1531c1516
<       <iframe title="Server-side implementation – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/server-side/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Server-side implementation – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/server-side/index.html" frameBorder="0" loading="lazy"></iframe>
1840,1841c1811
< 
<           <span class="govuk-visually-hidden">server-side implementation with confirmation – cookie banner</span>
---
>           <span class="govuk-visually-hidden">server-side implementation with confirmation – cookie banner</span>
1845c1815
<       <iframe title="Server-side implementation with confirmation – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/server-side-confirmation/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Server-side implementation with confirmation – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/server-side-confirmation/index.html" frameBorder="0" loading="lazy"></iframe>
2140,2141c2096
< 
<           <span class="govuk-visually-hidden">server-side implementation with multiple messages and question visible – cookie banner</span>
---
>           <span class="govuk-visually-hidden">server-side implementation with multiple messages and question visible – cookie banner</span>
2145c2100
<       <iframe title="Server-side implementation with multiple messages and question visible – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/server-side-multiple-messages-question-visible/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Server-side implementation with multiple messages and question visible – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/server-side-multiple-messages-question-visible/index.html" frameBorder="0" loading="lazy"></iframe>
2519,2520c2450
< 
<           <span class="govuk-visually-hidden">server-side implementation with multiple messages and confirmation visible – cookie banner</span>
---
>           <span class="govuk-visually-hidden">server-side implementation with multiple messages and confirmation visible – cookie banner</span>
2524c2454
<       <iframe title="Server-side implementation with multiple messages and confirmation visible – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/server-side-multiple-messages-confirmation-visible/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Server-side implementation with multiple messages and confirmation visible – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/server-side-multiple-messages-confirmation-visible/index.html" frameBorder="0" loading="lazy"></iframe>
2910,2911c2816
< 
<           <span class="govuk-visually-hidden">client-side implementation – cookie banner</span>
---
>           <span class="govuk-visually-hidden">client-side implementation – cookie banner</span>
2915c2820
<       <iframe title="Client-side implementation – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/client-side/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Client-side implementation – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/client-side/index.html" frameBorder="0" loading="lazy"></iframe>
3279,3280c3161
< 
<           <span class="govuk-visually-hidden">accepted replacement message – cookie banner</span>
---
>           <span class="govuk-visually-hidden">accepted replacement message – cookie banner</span>
3284c3165
<       <iframe title="Accepted replacement message – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/client-side-accepted/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Accepted replacement message – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/client-side-accepted/index.html" frameBorder="0" loading="lazy"></iframe>
3647,3648c3505
< 
<           <span class="govuk-visually-hidden">rejected replacement message – cookie banner</span>
---
>           <span class="govuk-visually-hidden">rejected replacement message – cookie banner</span>
3652c3509
<       <iframe title="Rejected replacement message – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/client-side-rejected/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Rejected replacement message – Cookie Banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/client-side-rejected/index.html" frameBorder="0" loading="lazy"></iframe>
4018c3852
< <h3 id="if-you-re-using-essential-cookies-and-analytics-cookies">If you’re using essential cookies and analytics cookies</h3>
---
> <h3 id="if-youre-using-essential-cookies-and-analytics-cookies">If you’re using essential cookies and analytics cookies</h3>
4025,4026c3859
< 
<           <span class="govuk-visually-hidden">default – cookie banner second</span>
---
>           <span class="govuk-visually-hidden">default – cookie banner second</span>
4030c3863
<       <iframe title="Default – Cookie Banner second example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/default/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Default – Cookie Banner second example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/components/cookie-banner/default/index.html" frameBorder="0" loading="lazy"></iframe>
4327c4147
< <h3 id="if-you-re-using-more-than-one-type-of-non-essential-cookie">If you’re using more than one type of non-essential cookie</h3>
---
> <h3 id="if-youre-using-more-than-one-type-of-non-essential-cookie">If you’re using more than one type of non-essential cookie</h3>
diff -r -w -B deploy/currentpublic/components/date-input/index.html deploy/public/components/date-input/index.html
1843c1813
< <h3 id="if-you-re-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
---
> <h3 id="if-youre-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
diff -r -w -B deploy/currentpublic/components/notification-banner/index.html deploy/public/components/notification-banner/index.html
1170,1171c1170
< 
<           <span class="govuk-visually-hidden">default – notification banner</span>
---
>           <span class="govuk-visually-hidden">default – notification banner</span>
1175c1174
<       <iframe title="Default – notification banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable app-example__frame--s" src="/components/notification-banner/default/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Default – notification banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable app-example__frame--s" src="/components/notification-banner/default/index.html" frameBorder="0" loading="lazy"></iframe>
1384,1385c1375
< 
<           <span class="govuk-visually-hidden">whole service – notification banner</span>
---
>           <span class="govuk-visually-hidden">whole service – notification banner</span>
1389c1379
<       <iframe title="Whole service – notification banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable app-example__frame--s" src="/components/notification-banner/whole-service/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Whole service – notification banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable app-example__frame--s" src="/components/notification-banner/whole-service/index.html" frameBorder="0" loading="lazy"></iframe>
1556c1539
< <h2 id="telling-the-user-about-something-that-s-happening-elsewhere">Telling the user about something that’s happening elsewhere</h2>
---
> <h2 id="telling-the-user-about-something-thats-happening-elsewhere">Telling the user about something that’s happening elsewhere</h2>
1567,1568c1550
< 
<           <span class="govuk-visually-hidden">default – notification banner second</span>
---
>           <span class="govuk-visually-hidden">default – notification banner second</span>
1572c1554
<       <iframe title="Default – notification banner second example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable app-example__frame--s" src="/components/notification-banner/default/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Default – notification banner second example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable app-example__frame--s" src="/components/notification-banner/default/index.html" frameBorder="0" loading="lazy"></iframe>
1756,1757c1730
< 
<           <span class="govuk-visually-hidden">success banner – notification banner</span>
---
>           <span class="govuk-visually-hidden">success banner – notification banner</span>
1761c1734
<       <iframe title="Success banner – notification banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable app-example__frame--s" src="/components/notification-banner/success/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Success banner – notification banner example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable app-example__frame--s" src="/components/notification-banner/success/index.html" frameBorder="0" loading="lazy"></iframe>
diff -r -w -B deploy/currentpublic/components/radios/index.html deploy/public/components/radios/index.html
1621c1603
< <h3 id="if-you-re-asking-one-question-on-the-page">If you’re asking one question on the page</h3>
---
> <h3 id="if-youre-asking-one-question-on-the-page">If you’re asking one question on the page</h3>
2071c2035
< <h3 id="if-you-re-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
---
> <h3 id="if-youre-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
5285c5117
< <h4 id="if-it-s-a-yes-or-no-question">If it’s a ‘yes’ or ‘no’ question</h4>
---
> <h4 id="if-its-a-yes-or-no-question">If it’s a ‘yes’ or ‘no’ question</h4>
5287c5119
< <h4 id="if-there-are-two-options-which-are-not-yes-and-no-">If there are two options which are not ‘yes’ and ‘no’</h4>
---
> <h4 id="if-there-are-two-options-which-are-not-yes-and-no">If there are two options which are not ‘yes’ and ‘no’</h4>
5291c5123
< <h4 id="if-it-s-a-conditionally-revealed-question">If it’s a conditionally revealed question</h4>
---
> <h4 id="if-its-a-conditionally-revealed-question">If it’s a conditionally revealed question</h4>
diff -r -w -B deploy/currentpublic/components/text-input/index.html deploy/public/components/text-input/index.html
1602c1586
< <h3 id="if-you-re-asking-one-question-on-the-page">If you’re asking one question on the page</h3>
---
> <h3 id="if-youre-asking-one-question-on-the-page">If you’re asking one question on the page</h3>
2037c2005
< <h3 id="if-you-re-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
---
> <h3 id="if-youre-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
6582c6371
< <h3 id="how-and-when-to-spellcheck-a-user-s-input">How and when to spellcheck a user’s input</h3>
---
> <h3 id="how-and-when-to-spellcheck-a-users-input">How and when to spellcheck a user’s input</h3>
diff -r -w -B deploy/currentpublic/components/textarea/index.html deploy/public/components/textarea/index.html
2135c2093
< <h3 id="if-you-re-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
---
> <h3 id="if-youre-asking-more-than-one-question-on-the-page">If you’re asking more than one question on the page</h3>
diff -r -w -B deploy/currentpublic/design-system-team/index.html deploy/public/design-system-team/index.html
970c970
<       <h1 id="gov-uk-design-system-and-prototype-team">GOV.UK Design System and Prototype team</h1>
---
>       <h1 id="govuk-design-system-and-prototype-team">GOV.UK Design System and Prototype team</h1>
975c975
< <h2 id="gov-uk-design-system-team">GOV.UK Design System team</h2>
---
> <h2 id="govuk-design-system-team">GOV.UK Design System team</h2>
1000c1000
< <h2 id="gov-uk-prototype-team">GOV.UK Prototype team</h2>
---
> <h2 id="govuk-prototype-team">GOV.UK Prototype team</h2>
diff -r -w -B deploy/currentpublic/get-started/extending-and-modifying-components/index.html deploy/public/get-started/extending-and-modifying-components/index.html
1076c1076
< <h2 id="avoid-overwriting-gov-uk-design-system-code">Avoid overwriting GOV.UK Design System code</h2>
---
> <h2 id="avoid-overwriting-govuk-design-system-code">Avoid overwriting GOV.UK Design System code</h2>
1080c1080
< <pre><code class="lang-html"><span class="hljs-tag">&lt;<span class="hljs-name">div</span> <span class="hljs-attr">class</span>=<span class="hljs-string">&quot;app-interruption-card&quot;</span>&gt;</span>
---
> <pre><code class="language-html"><span class="hljs-tag">&lt;<span class="hljs-name">div</span> <span class="hljs-attr">class</span>=<span class="hljs-string">&quot;app-interruption-card&quot;</span>&gt;</span>
1086c1086
< <pre><code class="lang-css"><span class="hljs-selector-class">.app-interruption-card</span> <span class="hljs-selector-class">.govuk-button</span> {
---
> <pre><code class="language-css"><span class="hljs-selector-class">.app-interruption-card</span> <span class="hljs-selector-class">.govuk-button</span> {
1107c1107
< <pre><code class="lang-css"><span class="hljs-selector-class">.app-</span>\!<span class="hljs-selector-tag">-reference-number-width</span> {
---
> <pre><code class="language-css"><span class="hljs-selector-class">.app-</span>\!<span class="hljs-selector-tag">-reference-number-width</span> {
1111c1111
< <pre><code class="lang-html"><span class="hljs-tag">&lt;<span class="hljs-name">span</span> <span class="hljs-attr">class</span>=<span class="hljs-string">&quot;app-!-reference-number-width&quot;</span>&gt;</span>
---
> <pre><code class="language-html"><span class="hljs-tag">&lt;<span class="hljs-name">span</span> <span class="hljs-attr">class</span>=<span class="hljs-string">&quot;app-!-reference-number-width&quot;</span>&gt;</span>
1119c1119
< <pre><code class="lang-html"><span class="hljs-tag">&lt;<span class="hljs-name">div</span> <span class="hljs-attr">class</span>=<span class="hljs-string">&quot;app-interruption-card&quot;</span>&gt;</span>
---
> <pre><code class="language-html"><span class="hljs-tag">&lt;<span class="hljs-name">div</span> <span class="hljs-attr">class</span>=<span class="hljs-string">&quot;app-interruption-card&quot;</span>&gt;</span>
1125c1125
< <pre><code class="lang-css"><span class="hljs-selector-class">.app-button--inverse</span> {
---
> <pre><code class="language-css"><span class="hljs-selector-class">.app-button--inverse</span> {
diff -r -w -B deploy/currentpublic/get-started/focus-states/index.html deploy/public/get-started/focus-states/index.html
1071c1071,1072
< </code></pre><h3 id="make-other-focusable-elements-accessible">Make other focusable elements accessible</h3>
---
> </code></pre>
> <h3 id="make-other-focusable-elements-accessible">Make other focusable elements accessible</h3>
diff -r -w -B deploy/currentpublic/get-started/production/index.html deploy/public/get-started/production/index.html
1053c1053
< <h2 id="include-gov-uk-frontend-in-your-project">Include GOV.UK Frontend in your project</h2>
---
> <h2 id="include-govuk-frontend-in-your-project">Include GOV.UK Frontend in your project</h2>
1075c1075
< <h2 id="start-using-the-gov-uk-page-template">Start using the GOV.UK page template</h2>
---
> <h2 id="start-using-the-govuk-page-template">Start using the GOV.UK page template</h2>
diff -r -w -B deploy/currentpublic/patterns/addresses/index.html deploy/public/patterns/addresses/index.html
1329c1315
< <p>Use the <code>autocomplete</code> attribute on each individual address field to help users enter their address more quickly. This lets you specify each input’s purpose so browsers can autofill the information on a user’s behalf if they’ve entered it previously.</p>
---
> <p>Use the <code>autocomplete</code> attribute on each individual address field to help users enter their address more quickly. This lets you specify each input’s purpose so browsers can autofill the information on a user’s behalf if they’ve entered it previously.</p>
diff -r -w -B deploy/currentpublic/patterns/contact-a-department-or-service-team/index.html deploy/public/patterns/contact-a-department-or-service-team/index.html
1293c1278
< <h3 id="write-telephone-numbers-in-the-gov-uk-style">Write telephone numbers in the GOV.UK style</h3>
---
> <h3 id="write-telephone-numbers-in-the-govuk-style">Write telephone numbers in the GOV.UK style</h3>
1337c1320
< <h3 id="tell-users-how-long-they-ll-have-to-wait">Tell users how long they’ll have to wait</h3>
---
> <h3 id="tell-users-how-long-theyll-have-to-wait">Tell users how long they’ll have to wait</h3>
diff -r -w -B deploy/currentpublic/patterns/create-accounts/index.html deploy/public/patterns/create-accounts/index.html
1192c1192
< <h3 id="never-use-national-insurance-numbers-to-verify-a-user-s-identity">Never use National Insurance numbers to verify a user’s identity</h3>
---
> <h3 id="never-use-national-insurance-numbers-to-verify-a-users-identity">Never use National Insurance numbers to verify a user’s identity</h3>
diff -r -w -B deploy/currentpublic/patterns/equality-information/index.html deploy/public/patterns/equality-information/index.html
1325c1312
< <h3 id="asking-about-date-of-birth-age-">Asking about date of birth (age)</h3>
---
> <h3 id="asking-about-date-of-birth-age">Asking about date of birth (age)</h3>
3462c3353
< <h3 id="questions-we-d-like-to-answer-through-research">Questions we’d like to answer through research</h3>
---
> <h3 id="questions-wed-like-to-answer-through-research">Questions we’d like to answer through research</h3>
diff -r -w -B deploy/currentpublic/patterns/names/index.html deploy/public/patterns/names/index.html
1302c1298
< <h3 id="do-not-spellcheck-user-s-names">Do not spellcheck user’s names</h3>
---
> <h3 id="do-not-spellcheck-users-names">Do not spellcheck user’s names</h3>
1340c1335
< <h3 id="avoid-asking-for-a-person-s-title">Avoid asking for a person’s title</h3>
---
> <h3 id="avoid-asking-for-a-persons-title">Avoid asking for a person’s title</h3>
diff -r -w -B deploy/currentpublic/patterns/question-pages/index.html deploy/public/patterns/question-pages/index.html
1603,1604c1579
< 
<           <span class="govuk-visually-hidden">explanatory text – question pages</span>
---
>           <span class="govuk-visually-hidden">explanatory text – question pages</span>
1608c1583
<       <iframe title="Explanatory text – Question pages example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/patterns/question-pages/explanatory-text/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Explanatory text – Question pages example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/patterns/question-pages/explanatory-text/index.html" frameBorder="0" loading="lazy"></iframe>
diff -r -w -B deploy/currentpublic/patterns/start-using-a-service/index.html deploy/public/patterns/start-using-a-service/index.html
1181c1181
< <h2 id="choosing-a-format-for-a-start-point-on-gov-uk">Choosing a format for a start point on GOV.UK</h2>
---
> <h2 id="choosing-a-format-for-a-start-point-on-govuk">Choosing a format for a start point on GOV.UK</h2>
1201c1201
< <pre><code class="lang-plaintext">{button start}[Button text goes here](https://servicename.service.gov.uk/first-page-within-service){/button}
---
> <pre><code class="language-plaintext">{button start}[Button text goes here](https://servicename.service.gov.uk/first-page-within-service){/button}
diff -r -w -B deploy/currentpublic/patterns/step-by-step-navigation/index.html deploy/public/patterns/step-by-step-navigation/index.html
1230c1230
< <h3 id="use-both-and-and-or-">Use both ‘and’ and ‘or’</h3>
---
> <h3 id="use-both-and-and-or">Use both ‘and’ and ‘or’</h3>
diff -r -w -B deploy/currentpublic/patterns/task-list-pages/index.html deploy/public/patterns/task-list-pages/index.html
1239,1240c1239
< 
<           <span class="govuk-visually-hidden">have you completed this section – task list pages</span>
---
>           <span class="govuk-visually-hidden">have you completed this section – task list pages</span>
1244c1243
<       <iframe title="Have you completed this section – Task list pages example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/patterns/task-list-pages/have-you-completed-this-section/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Have you completed this section – Task list pages example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/patterns/task-list-pages/have-you-completed-this-section/index.html" frameBorder="0" loading="lazy"></iframe>
1333,1334c1324
< 
<           <span class="govuk-visually-hidden">have you completed this section error – task list pages</span>
---
>           <span class="govuk-visually-hidden">have you completed this section error – task list pages</span>
1338c1328
<       <iframe title="Have you completed this section error – Task list pages example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/patterns/task-list-pages/have-you-completed-this-section-error/index.html" frameBorder="0" loading="lazy"></iframe>
---
>       <iframe title="Have you completed this section error – Task list pages example" data-module="app-example-frame" class="app-example__frame app-example__frame--resizable" src="/patterns/task-list-pages/have-you-completed-this-section-error/index.html" frameBorder="0" loading="lazy"></iframe>
diff -r -w -B deploy/currentpublic/patterns/telephone-numbers/index.html deploy/public/patterns/telephone-numbers/index.html
1359c1353
< <pre><code class="lang-html"><span class="hljs-tag">&lt;<span class="hljs-name">a</span> <span class="hljs-attr">href</span>=<span class="hljs-string">&quot;tel:+442079476330&quot;</span>&gt;</span>020 7947 6330<span class="hljs-tag">&lt;/<span class="hljs-name">a</span>&gt;</span>
---
> <pre><code class="language-html"><span class="hljs-tag">&lt;<span class="hljs-name">a</span> <span class="hljs-attr">href</span>=<span class="hljs-string">&quot;tel:+442079476330&quot;</span>&gt;</span>020 7947 6330<span class="hljs-tag">&lt;/<span class="hljs-name">a</span>&gt;</span>
1365c1359
< <h3 id="write-telephone-numbers-in-the-gov-uk-style">Write telephone numbers in the GOV.UK style</h3>
---
> <h3 id="write-telephone-numbers-in-the-govuk-style">Write telephone numbers in the GOV.UK style</h3>
diff -r -w -B deploy/currentpublic/styles/layout/index.html deploy/public/styles/layout/index.html
1319c1310
< <h3 id="row-1-two-thirds-br-row-2-two-thirds-and-one-third">Row 1: Two-thirds <br>Row 2: Two-thirds and one-third</h3>
---
> <h3 id="row-1-two-thirds-row-2-two-thirds-and-one-third">Row 1: Two-thirds <br>Row 2: Two-thirds and one-third</h3>
1569c1543
< <h3 id="two-thirds">Two-thirds</h3>
---
> <h3 id="two-thirds-1">Two-thirds</h3>
diff -r -w -B deploy/currentpublic/styles/page-template/index.html deploy/public/styles/page-template/index.html
1579c1518
< <pre><code class="lang-javascript">{% set bodyClasses = <span class="hljs-string">&quot;EXAMPLE-CLASS&quot;</span> %}
---
> <pre><code class="language-javascript">{% set bodyClasses = <span class="hljs-string">&quot;EXAMPLE-CLASS&quot;</span> %}
1583c1522
< <pre><code class="lang-javascript">{% block bodyEnd %}
---
> <pre><code class="language-javascript">{% block bodyEnd %}
1590c1529
< <pre><code class="lang-javascript">{% block header %}
---
> <pre><code class="language-javascript">{% block header %}
diff -r -w -B deploy/currentpublic/styles/spacing/index.html deploy/public/styles/spacing/index.html
1255c1255
< <pre><code class="lang-scss"><span class="hljs-keyword">@include</span> govuk-responsive-padding(<span class="hljs-number">6</span>);
---
> <pre><code class="language-scss"><span class="hljs-keyword">@include</span> govuk-responsive-padding(<span class="hljs-number">6</span>);
1259c1259
< <pre><code class="lang-scss"><span class="hljs-keyword">@include</span> govuk-responsive-margin(<span class="hljs-number">6</span>, <span class="hljs-string">&quot;bottom&quot;</span>);
---
> <pre><code class="language-scss"><span class="hljs-keyword">@include</span> govuk-responsive-margin(<span class="hljs-number">6</span>, <span class="hljs-string">&quot;bottom&quot;</span>);
1264c1264
< <pre><code class="lang-scss"><span class="hljs-attribute">padding-top</span>: govuk-spacing(<span class="hljs-number">6</span>);
---
> <pre><code class="language-scss"><span class="hljs-attribute">padding-top</span>: govuk-spacing(<span class="hljs-number">6</span>);
1268c1268
< <pre><code class="lang-scss"><span class="hljs-attribute">margin-top</span>: govuk-spacing(-<span class="hljs-number">3</span>);
---
> <pre><code class="language-scss"><span class="hljs-attribute">margin-top</span>: govuk-spacing(-<span class="hljs-number">3</span>);

@domoscargin domoscargin marked this pull request as ready for review November 19, 2022 00:54
Copy link
Contributor

@colinrotherham colinrotherham left a comment

Choose a reason for hiding this comment

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

Looks like this was a huge painful job. Must be good to reach this point 😊

Love the ideas for what could come next too

@@ -48,9 +48,14 @@ use `govuk-colour("red")` rather than `$govuk-error-colour`.
<td class="app-colour-list-column app-colour-list-column--colour">
<code>{{colour.colour}}</code>
</td>
{% if colour.notes %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny bit strange seeing an if/else duplicate the table cell <td> but I understand why 😔

Not something we can fix with {{- or -}} instead?

{{- colour.notes -}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah, I meant to flag this one actually - I originally went with your suggestion, but that generated diffs and I was trying to smoosh the diff as much as possible to make reviewing it simpler.

Happy to do it this way instead though!

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker either way, just stood out from the crowd 😊

@romaricpascal
Copy link
Member

Damn, that sounds like quite the ordeal to get everything in order, nice one! 🙌🏻

Only thing I could think of for the line break issue would be to use a custom tokenizer to sort things out, but that'll likely be too low level. Looks like we're screwed by GFM specifying that closing/ending tags representing HTML blocks need to start the line. Marked seems kind enough to allow with a little leading spacing](https://github.com/markedjs/marked/blob/master/src/rules.js#L18), but again very low level, and conflicting with the indented code blocks (something I'd happily do without in Markdown, but that's a personal opinion).

You may have already looked into this, though 😄 And given things are in good shape, we may as well move forward and merge this?

We use `metalsmith-in-place` to compile our markdown and nunjucks. That in turn has a dependency on `jstransformer`, which then requires transformers of the required type (in this case, `jstransformer-marked`).

We have `marked` as a direct dependency, but we're actually at the whim of `jstransformer-marked`'s dependency tree when it comes to processing our files with `metalsmith-in-place`.

v1.0.3 of `jstransformer-marked` uses `marked` ^0.3.9 (0.3.19 in our package-lock).

v1.2.0 uses `marked` 4.0.18.

This creates... problems.
Pedantic rules require angle brackets around raw URLs in order to autolink them. This was a bit buggy in early versions of Marked, which is why it's flown under our radar till we bumped Marked.

Switching to Github Flavoured Markdown should negate the need to do this and allow future editors not to think about it, but it's probably still worth being widely compliant in case anything changes down the line.
With pedantic rules, we must separate a list from the preceding text with a blank line.

This is more relaxed in Github Flavoured Markdown, but no reason not to go for wider compliance.
We were previously flaky on this - we mangled one email address, but left all others unmangled.

I'm not sure how useful this is nowadays, since I'm sure crawlers are smart enough to interpret mangled addresses.

Alternatively, we can leave as is and all email addresses will be mangled.
Marked defaults to GFM. We've been using pedantic rules and somehow have avoided errors with fenced code blocks, which `markdown.pl` doesn't support. I'm guessing that was a bug with early versions of Marked which has now been fixed, so our fenced code blocks don't work with a newer version of Marked.

Best to ditch pedantry and fix GFM-related errors, as the team is deeply into the Github ecosystem anyway, and it requires less effort from content editors.
Switching to Github Flavoured Markdown results in a LOT of differences between rendering.

The bulk of these are related to blank lines in HTML blocks, which fall foul of rules 6 and 7 of HTML blocks in GFM (see: https://github.github.com/gfm/#html-blocks), and cause rendering errors.

We can (painstakingly) remove all these blank lines to produce a compiled output that only differs in desirable ways. This is mostly fine, but it does mean that there are now no empty lines in code samples, which may or may not be a blocker.
@domoscargin
Copy link
Contributor Author

You may have already looked into this, though 😄 And given things are in good shape, we may as well move forward and merge this?

My thinking with any low-level Marked-based approach is to wait until we've reviewed our use of Metalsmith (which is on our roadmap) - I suspect we may end up switching to something like 11ty, which provides Nunjucks-in-markdown support more natively, and thus might be easier to configure properly.

If the code sample spacing proves to be an issue for users in the meantime, we can look at other solutions.

Thanks, both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

3 participants