-
Notifications
You must be signed in to change notification settings - Fork 328
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
Fix rendering of Back link's href
and text
for falsy values
#5191
Conversation
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for 6d99967 |
const $component = document.querySelector('.govuk-back-link') | ||
expect($component).toHaveAttribute('href', '/home') | ||
it.each(['', 0, false, null, undefined])( | ||
'displays `Back` for `%s`', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this test is misnamed, as it appears to be testing whether href
is #
when a value is falsy or undefined, not the link text.
The test for the link text is on line 78.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, good catch, I'll rename.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One note about a test having the wrong name, but otherwise this seems to do what it sets out to do, and it has tests now! Hurray!
Reverts to the behaviour of 5.4.0, which ensured consistency across falsy values, and importantly that a 'Back' text was displayed in case of empty strings (and a '#' value for the `href` attribute, but there's less difference with a ''). Instead of reverting the commit, I used the `boolean` option from Nunjucks' `default` filter to keep the tidy up of the templates introduced in fc704d1.
9d50dcb
to
6d99967
Compare
Fix rendering of Back link's `href` and `text` for falsy values
Reverts to the behaviour of 5.4.0, which ensured consistency across falsy values, and importantly that a 'Back' text was displayed in case of empty strings (and a '#' value for the
href
attribute, but there's less difference with a '').Adds tests to ensure we keep an eye on the behaviour in future template changes, or at least make sure that any deviation is intentional (and tested).
Instead of reverting the commit, I used the
boolean
option from Nunjucks'default
filter to keep the tidy up of the templates introduced in fc704d1.Fixes #5189