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

Added custom scss and theme files #128

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Added custom scss and theme files #128

wants to merge 4 commits into from

Conversation

mzayeddfe
Copy link

@mzayeddfe mzayeddfe commented Dec 9, 2024

Overview of changes

Why are these changes being made?

To make the analyst guide more accessible and to make the colours more cohesive with the theme.

Detailed description of changes

Both themes:

  • Made Roboto the default font for both themes with backups just in case it does not load (this was copied over from the cyborg theme)
  • Made sure there is a line break above and below all heading levels - can remove some if there are too many listed.
  • Added colours for hovering over links and visited links - can remove if too much
  • Changed the inline code font colour and background

Light theme:

  • Made the links in text, navigation bar and toc the same colour
  • Changed the code block styling to make it similar to the united theme while still using accessible colours. This is found in the printing-light.theme file.

Dark theme:

  • Made the links in text and navigation bar the same colour
  • Changed the colour of the links of the navigation at the bottom of the page to a more accessible colour
  • Changed the link and list item colours for breadcrumbs to make them more accessible

Issue ticket number/s and link

70

@mzayeddfe mzayeddfe linked an issue Dec 9, 2024 that may be closed by this pull request
4 tasks
@jen-machin
Copy link
Contributor

All looks really good to me! The only thing I'm not sure on is having a different link colour for visited links, maybe a bit too much? I do like the colour change when you hover over a link though 😄 I'll let Cam have a look too when he's back in case he disagrees!

Copy link
Contributor

@cjrace cjrace left a comment

Choose a reason for hiding this comment

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

A lot of nice approaches in here, will make it much easier to edit moving forwards too - well done!

I find the visited link colours a bit dominating / distracting, though in general I prefer no visited styling at all, so that might be more of my personal preference. I've seen navigation links in GOV.UK services increasingly have no visited styling so maybe there's a way to have the left navigation and right TOC without it, and only the links in the content having the visited styling? I don't have strong view on this though - happy for you and @jen-machin to agree what the styling looks like.

All the specific points aside from one in the original issue seem to be fixed now and checking with AxeDevtools didn't show any issues with contrast, so all good.

The only issue not addressed is the link breaks one (second point in the checklist), though given once that is done automatically in the styling it'll need a tonne of line breaks removing from every page, I think that might be better done as a separate task / PR anyway, so I'd suggest doing that rather than doing anything for it here

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.

Add our own custom styling
3 participants