hmrc / hmrc-frontend

Apache License 2.0
9 stars 19 forks source link

Using the govuk-branded layout for v13 kits. #287

Closed nataliecarey closed 7 months ago

nataliecarey commented 1 year ago

In v13 we've moved the main layout from app/views/layout.html to app/views/layouts/main.html which means that the Account Menu page doesn't actually work for v13 kits out of the box.

This PR extends the govuk-branded layout provided by the prototype kit when using v13.

hmrc-web-operations commented 1 year ago

Thank you for submitting a pull request on this HMRC repository. A member of the HMRC team will review your changes and action accordingly.

HMRC reviewer: please reference this runbook for guidance on how to proceed.

hmrc-web-operations commented 1 year ago

Thank you for submitting a pull request on this HMRC repository. A member of the HMRC team will review your changes and action accordingly.

HMRC reviewer: please reference this runbook for guidance on how to proceed.

oscarduignan commented 1 year ago

Thank you @nataliecarey! I missed this last week, I'm on support tomorrow so I'll add a changelog and get this merged 🤝

oscarduignan commented 1 year ago

@nataliecarey it seems like the releaseVersion isn't available when previewing CleanShot 2023-04-24 at 16 53 02@2x I couldn't see from taking a quick look at the govuk-prototype-kit why it wasn't available when previewing (works ok after creating)

then alongside that possible bug - should I make this change something like

{% if releaseVersion and "v13." in releaseVersion }}

so that the logic will work if if releaseVersion isn't set for people using old version of the prototype kit

I haven't really taken a close look at how people could have used this layout before so wasn't sure if I missed something on that last bit

want me to write up anything about the releaseVersion bit anywhere as a bug?

oscarduignan commented 1 year ago

steps to recreate the issue I'm seeing above locally:

  1. get hmrc-frontend locally with version of this PR and build it
    gh repo clone hmrc/hmrc-frontend && cd hmrc-frontend && gh pr checkout 287 && nvm install && npm install && npm run build
  2. create a repo and install the local version of hmrc-frontend then start it up
    npx govuk-prototype-kit@latest create ../example-prototype && cd ../example-prototype && nvm install --lts && npm install && npm install hmrc-frontend@../hmrc-frontend/package && npm run dev
  3. preview creating layout with that by going to http://localhost:3000/manage-prototype/templates/view?package=hmrc-frontend&template=%2Fhmrc%2Ftemplates%2Faccount-header.html
  4. and you should see the error, and you can fiddle with it by editing hmrc-frontend/package/hmrc/layouts/account-header.html
nataliecarey commented 1 year ago

Yeah, I see the problem and can see why it comes up - we use a different Nunjucks instance to render previews and that doesn't have all the same settings as the main one.

I'll flag it to the team today.

The problem now is that any version 13.6.1 or below won't have this fix therefore users of HMRC Frontend would need to upgrade to the latest version (once we've released a fix) in order for the preview to work, does that sound like a reasonable request for those users?

oscarduignan commented 1 year ago

yup I think that's fine to request, anyone on a recent version shouldn't have a problem upgrading and it's not a big blocker - doesn't actually prevent them from creating a page with the layout template - so should be easy for us to suggest the work around if it ever comes up from someone trying to use it, thanks @nataliecarey 🙌

oscarduignan commented 7 months ago

bit of housekeeping, closing because I think this is sorted now 🤞