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

Make it easier to set a page title #2030

Merged
merged 4 commits into from
Mar 21, 2023
Merged

Make it easier to set a page title #2030

merged 4 commits into from
Mar 21, 2023

Conversation

nataliecarey
Copy link
Contributor

@nataliecarey nataliecarey commented Mar 10, 2023

@frankieroberto contributed a PR for this with the comment "One possible implementation for #1975."

I have taken that PR, added tests and changed the behaviour slightly. It now works as follows:

  • You can override the pageTitle block (as we have recommended so far) which gives you full control over the page title.
  • You can set a pageName variable and the title will have - <serviceName> - GOV.UK added to the end of it

Next steps:

  • @frankieroberto can you check my PR and make sure it's in line with your expectations.
  • @joelanman and @oli-rose28 can you take a look and see if this is behaviour we want to add to the kit - note I've updated the dash character to match Frankie's PR, I'm not sure which dash is right and which one is wrong but I've avoided a mismatch. If I've gone the wrong way feel free to change all the dashes in this PR to the correct type.

Copy link
Contributor

@frankieroberto frankieroberto left a comment

Choose a reason for hiding this comment

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

Looks great to me! I'm not actually sure which of the many dash characters should be used, but GOV.UK seems to use - (hyphen-minus)? Probably an (em dash) would look better, but maybe there’s a reason not to use this? 🤷

As a follow-up PR, we could set the title variable on the example template pages, and then re-use the title within the h1s?

@@ -10,7 +10,7 @@
{% endblock %}

{% block pageTitle %}
GOV.UK Prototype Kit
{% if title %}{{ title }} - {% endif %}{{ serviceName }} - {% if titleOrganisation %}{{ titleOrganisation }}{% else %}GOV.UK Prototype Kit{% endif %}
Copy link
Contributor

Choose a reason for hiding this comment

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

I’m not actually sure if titleOrganisation is needed? I've not seen anyone use that format and it’s not mentioned in the title style guide - but maybe you know of some instances?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I put that in is because we use - GOV.UK Prototype Kit and your example was - GOV.UK.

I took that to mean that you want titles in your prototype to end with - GOV.UK but I don't think we can recommend that as a default, so in your kit you can set titleOrganisation = "GOV.UK" and get the behaviour that was in your PR before I messed with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I fixed the dash to match what's on the style guide (probably what we were already using).

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn’t that still result in Section name - GOV.UK - GOV.UK Prototype Kit though? I think we’d want to use Section name - service name - GOV.UK, as per the style guide?

Is there any reason not to switch out GOV.UK Prototype Kit for GOV.UK as the hardcoded bit at the end? The prototypes are password-protected, and otherwise look and feel exactly like a real service, so I don't see any issues around accidentally misleading people?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops I misread the syntax! 🤦🏻‍♂️

I still think you could default to just - GOV.UK but it’s not a big deal.

Would also suggest renaming the variable to titleSuffix to be more generic, as I don’t think it’s generally recommended to include the organisation (eg department) name in the title?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree the default should be GOV.UK as per our guidelines

Copy link
Contributor

Choose a reason for hiding this comment

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

decided to drop org/suffix - this is the govuk branded template - the default should be GOV.UK. If we find use cases to change it we can investigate adding suffix then

@BenSurgisonGDS BenSurgisonGDS changed the title Set default page title using title variable Make it easier to set a page title Mar 21, 2023
@BenSurgisonGDS BenSurgisonGDS merged commit cddcc92 into main Mar 21, 2023
@BenSurgisonGDS BenSurgisonGDS deleted the title-variable branch March 21, 2023 18:37
@frankieroberto
Copy link
Contributor

🙌🏻

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.

4 participants