GOV.UK Frontend 4.4.0

[matthew-shaw]

Remaining issues if anyone else looks at this:

Character count component

Test failing: "when neither maxlength nor maxwords are set"

Errors:

  File "/home/mash/govuk-frontend-jinja/govuk_frontend_jinja/templates/components/character-count/macro.html", line 20, in template
    {%- if params.charactersUnderLimitText %}{{govukPluralisedI18nAttributes('characters-under-limit', params.charactersUnderLimitText)}}{% endif %}
  File "/home/mash/.pyenv/versions/3.9.15/lib/python3.9/site-packages/jinja2/runtime.py", line 777, in _invoke
    rv = self._func(*arguments)
  File "/home/mash/govuk-frontend-jinja/govuk_frontend_jinja/templates/macros/i18n.html", line 14, in template
    {% for pluralType, message in pluralForms %} data-i18n.{{translationKey}}.{{pluralType}}="{{message | escape}}"{% endfor %}
ValueError: too many values to unpack (expected 2)

and

  File "/home/mash/govuk-frontend-jinja/govuk_frontend_jinja/templates/components/character-count/macro.html", line 19, in template
    {%- if hasNoLimit and params.textareaDescriptionText %}{{govukPluralisedI18nAttributes('textarea-description', {other: params.textareaDescriptionText})}}{% endif %}
  File "/home/mash/.pyenv/versions/3.9.15/lib/python3.9/site-packages/jinja2/runtime.py", line 777, in _invoke
    rv = self._func(*arguments)
  File "/home/mash/govuk-frontend-jinja/govuk_frontend_jinja/templates/macros/i18n.html", line 14, in template
    {% for pluralType, message in pluralForms %} data-i18n.{{translationKey}}.{{pluralType}}="{{message | escape}}"{% endfor %}
ValueError: not enough values to unpack (expected 2, got 0)

Thoughts:

Looking at the test input params I can't see why the i18n macro is being called with the incorrect number of params. Other components that have had i18n support added in this release have similar tests that pass.

Tabs component

Test failing: "html as text"

Errors:

Unexpected closing tag "p". It may happen when the tag has already been closed by another tag. For more info see https://www.w3.org/TR/html5/syntax.html#closing-elements-that-have-implied-end-tags]

Thoughts:

The HTML within the text param does appear to be escaped into its <p>Panel 1 content</p> form, but for some reason is being treated as though it's not, resulting in an invalid HTML parsing error from govuk-frontend-diff. Again very confusing as other components test for escaped html within text attributes in the same way and they are passing.

[matthew-shaw]

Thanks so much for taking the time to look at this @leohemsted, really appreciate your contributions. Running the tests with a --exclude tabs flag results in 568 passing tests now. I'm going to look into changing govuk-frontend-diff in the short term to allow us to get this release out ASAP. Following that, to re-write the test suite using pytest. Thanks again 👍🏻

[andymantell]

Superb detective work @leohemsted! I'm not completely averse to releasing another govuk-frontend-diff if we know what the fix should be.

Rewriting the govuk-frontend-jinja tests is 100% the right way to fix it (And I have been badgering Matt to do this for a while 😆). In the short term though @matthew-shaw - it would be even easier to simply bypass this set of tests in the pipeline for this repo and manually verify that the output is correct. The tests are obviously an important thing in keeping the library stable over time, but if you are sufficiently happy that your output is correct, and you can confidently point to why the tests are failing then I don't see why you wouldn't simply release this package anyway.

And then crack on with rewriting the tests, or fixing govuk-frontend-diff in the short term. But I don't think this needs to hold you up.

Favour pragmatism over idealism...

[andymantell]

govuk-frontend-diff@1.1.2 is published now containing the fixed from https://github.com/surevine/govuk-frontend-diff/pull/88. In order to use it you just need to update the dockerfile where it downloads it:

https://github.com/LandRegistry/govuk-frontend-jinja/blob/main/tests/utils/Dockerfile