-
Notifications
You must be signed in to change notification settings - Fork 0
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
[PRMP-1330] - Add service-updates-link to footer #491
base: main
Are you sure you want to change the base?
Conversation
I didn't realise this was a draft
app/src/components/generic/serviceUpdatesLink/ServiceUpdatesLink.tsx
Outdated
Show resolved
Hide resolved
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.
It would be worthwhile modifying Footer.test.txs to have the first test/it block to check all the links are present
@@ -15,6 +16,7 @@ function Footer() { | |||
> | |||
Privacy notice | |||
</NHSFooter.ListItem> | |||
<ServiceUpdatesLink /> |
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.
As discussed I was wondering if it would be worth making the <NHSFooter.ListItem for ServiceUpdatesLink in the same file rather than separate one like the privacy link Footer list item. This might help with consistency especially since the ServiceUpdatesLink might not be usable in other places as its made as a NHSFooter.listitem
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 have modified this to no longer use a component but rather a constant within the Footer.tsx
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.
looks good however cypress tests seems to be failing
No description provided.