Skip to content

Conversation

@MohammadIqbalAD-NHS
Copy link
Contributor

No description provided.

@oliverbeumkes-nhs oliverbeumkes-nhs dismissed their stale review December 17, 2024 17:23

I didn't realise this was a draft

@MohammadIqbalAD-NHS MohammadIqbalAD-NHS marked this pull request as ready for review December 18, 2024 09:09
Copy link
Contributor

@abid-nhs abid-nhs left a 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

>
Privacy notice
</NHSFooter.ListItem>
<ServiceUpdatesLink />
Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor

@abid-nhs abid-nhs left a 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

@MohammadIqbalAD-NHS MohammadIqbalAD-NHS merged commit ad25b30 into main Jan 8, 2025
11 checks passed
@MohammadIqbalAD-NHS MohammadIqbalAD-NHS deleted the PRMP-1330 branch January 8, 2025 16:15
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