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

Switch to Highlight.js syntax highlighting #230

Merged
merged 12 commits into from
Feb 21, 2025

Conversation

frankieroberto
Copy link
Contributor

@frankieroberto frankieroberto commented Feb 19, 2025

This removes the existing syntax highlighting which uses Prism, which runs client-side, and swaps it for Highlight.js, which runs server-side.

It copies across the colours and styling for both inline code examples and longer code blocks from the NHS Service manual (where they’ve been tweaked to ensure a good colour contrast).

The longer code blocks use a new codeSnippet macro, which also takes a language parameter that is passed to Highlight.js and used for syntax highlighting.

Resolves #21

Screenshots

Type Before After
HTML Screenshot 2025-02-20 at 10 18 59 Screenshot 2025-02-20 at 10 21 10
JavaScript Screenshot 2025-02-20 at 10 17 49 Screenshot 2025-02-20 at 10 17 31
Nunjucks Screenshot 2025-02-20 at 10 16 12 Screenshot 2025-02-20 at 10 16 00
Inline code reference Screenshot 2025-02-20 at 10 16 20 Screenshot 2025-02-20 at 10 16 31

@frankieroberto frankieroberto temporarily deployed to nhs-prototyp-switch-to--zxpf0t February 19, 2025 23:29 Inactive
@frankieroberto
Copy link
Contributor Author

The one caveat to this is that Highlight.js doesn't seem to support Nunjucks (see supported languages). For the Nunjucks code snippets I’ve set it to use the javascript syntax highlighting, which is what the NHS Service Manual website does too, and that seems to work ok.

However, mixed HTML + Nunjucks code doesn’t work that well at all:

Screenshot 2025-02-20 at 10 35 11

There’s possibly a plugin out there that adds proper Nunjucks support though?

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.

Excellent work, massive improvement 🎉

Added some comments but please don't treat them as blockers

Can easily follow them up another day

@frankieroberto frankieroberto temporarily deployed to nhs-prototyp-switch-to--zxpf0t February 20, 2025 17:00 Inactive
@colinrotherham
Copy link
Contributor

Has the chosen language stopped working in the last push?

Highlighting not correct

@frankieroberto frankieroberto temporarily deployed to nhs-prototyp-switch-to--zxpf0t February 21, 2025 11:45 Inactive
@frankieroberto
Copy link
Contributor Author

Has the chosen language stopped working in the last push?

Well spotted! I, error, introduced a typo. 😊 Fixed in e0744b5.

@colinrotherham colinrotherham self-requested a review February 21, 2025 11:49
@frankieroberto frankieroberto temporarily deployed to nhs-prototyp-switch-to--zxpf0t February 21, 2025 11:55 Inactive
@frankieroberto frankieroberto temporarily deployed to nhs-prototyp-switch-to--zxpf0t February 21, 2025 11:57 Inactive
@frankieroberto frankieroberto merged commit 49e3f1c into main Feb 21, 2025
4 checks passed
@frankieroberto frankieroberto deleted the switch-to-highlightjs-syntax-highlighting branch February 21, 2025 12:22
@frankieroberto
Copy link
Contributor Author

Follow-up issue created in the backlog for exploring a fix for examples that mix HTML and Nunjucks: #231

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

Successfully merging this pull request may close these issues.

Improve syntax highlighting
2 participants