Conversation
…view-from-message-plan
…view-from-message-plan
alexnuttall
changed the title
…view-from-message-plan
…view-from-message-plan
| lockNumber, | ||
| }); | ||
| const backLinkHref = | ||
| hideBackLinks || lockNumber === undefined |
There was a problem hiding this comment.
Is lockNumber === undefined because it's a submitted template?
There was a problem hiding this comment.
I didn't add lock number handling to the new pages because it's consistent with what we did for the PDF equivalent. The status can be PROOF_APPROVED or SUBMITTED, so the template is effectively read-only at this point. Showing the 'invalid' page (in a new tab) if the template has moved to SUBMITTED doesn't seem that helpful. The template could move from PROOF_APPROVED to DELETED - you would get 'invalid template' in that case
| <NotifyBackLink>{content.backLink.text}</NotifyBackLink> | ||
| </Link> | ||
| {backLinkHref && ( | ||
| <Link href={backLinkHref} passHref legacyBehavior> |
There was a problem hiding this comment.
Could you use NHSNotifyBackLink?
There was a problem hiding this comment.
That component includes the nhsuk-back-link class, which adds a < glyph, not matching the design for the bottom link
| @@ -0,0 +1,66 @@ | |||
| import { | |||
There was a problem hiding this comment.
I don't think this component is a molecule it's closer to an organism or a template/page? What do you think?
There was a problem hiding this comment.
I'm happy to move it to organisms if you think it fits there - I feel like the distinction molecule/organism isn't that helpful to be honest - at least it feels that way because we're not really using organisms
| await expect(templateBlock.defaultTemplateCard.templateName).toHaveText( | ||
| templates.AUTHORING_LETTER.name | ||
| ); | ||
| await expect(templateBlock.defaultTemplateCard.templateLink).toHaveText( |
There was a problem hiding this comment.
Do you think its worth testing the opening in a new tab behaviour? Or that we have the correct attributes on the element?
There was a problem hiding this comment.
Added DOM assertions - actually navigating to the page is covered in e2e
There was a problem hiding this comment.
Adding that test made me realise the other language and accessible links hadn't been updated, thanks
| rel="noopener noreferrer" | ||
| target="_blank" | ||
| > | ||
| Learn more |
There was a problem hiding this comment.
Based on the ticket screenshots, yes
| it('matches snapshot without PDF file', () => { | ||
| const container = render( | ||
| <LetterRenderIframe tab='shortFormRender' pdfUrl={null} /> | ||
| <LetterRenderIframe renderType='shortFormRender' pdfUrl={null} /> |
There was a problem hiding this comment.
Not an issue at all but I thought I'd just say - feel free to igore and resolve.
Given this rename from tab to renderType any benefit in changing shortFormRender to just short| long | initial?
There was a problem hiding this comment.
It was that at one point - the idea of changing it to shortFormRender was that it allows you to use the string to do a lookup on the 'files' object on the template. There doesn't seem to be a clear winner between the two approaches IMO
…view-from-message-plan
…view-from-message-plan
frontend/src/components/molecules/MessagePlanCascadePreview/MessagePlanCascadePreview.tsx
Outdated
Show resolved
Hide resolved
4 participants
Description
updated pages:
Note: 'Learn more' links have been removed, to match the design, since I took the screenshots
templates/message-plans/choose-standard-english-letter-template/b0b6ec01-721f-4580-8637-5920c5dbe0ca/preview-template/653bfc62-8ee2-419d-b0cd-ab19a237592c?lockNumber=1
templates/message-plans/choose-large-print-letter-template/b0b6ec01-721f-4580-8637-5920c5dbe0ca/preview-template/df460f0e-6af7-4c49-832f-fd80b7abdd91?lockNumber=1
templates/message-plans/review-and-move-to-production/a4ce1f87-5877-42e6-9019-92ebc7d6e13f
templates/message-plans/review-and-move-to-production/a4ce1f87-5877-42e6-9019-92ebc7d6e13f/preview-template/653bfc62-8ee2-419d-b0cd-ab19a237592c
templates/message-plans/preview-message-plan/a4ce1f87-5877-42e6-9019-92ebc7d6e13f
templates/message-plans/preview-message-plan/a4ce1f87-5877-42e6-9019-92ebc7d6e13f/preview-template/653bfc62-8ee2-419d-b0cd-ab19a237592c
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.