[DTOSS-10296] split participant details into multiple sections#250
[DTOSS-10296] split participant details into multiple sections#250
Conversation
| { | ||
| "key": { | ||
| "text": "Address" | ||
| }, | ||
| "value": { | ||
| "html": address_html | ||
| } | ||
| }, |
There was a problem hiding this comment.
The NHS summary list guidance wasn't updated with Showing missing information (GOV.UK) but what if the address is empty here?
There was a problem hiding this comment.
Same for phone and email I suppose
There was a problem hiding this comment.
Think I'll park this one as well, we've got tickets to add the edit forms, so when that is done we will be linking to those.
| {% endif %} | ||
|
|
||
| {% if mammogram.additional_information %} | ||
| <br>Additional information: {{ mammogram.additional_information | nl2br }} |
There was a problem hiding this comment.
Just me liking nicely indented HTML
If you wanted the new lines to match the (10 space) indent level of the first line:
| <br>Additional information: {{ mammogram.additional_information | nl2br }} | |
| <br>Additional information: {{ mammogram.additional_information | nl2br | indent(10) }} |
There was a problem hiding this comment.
There are indenting issues throughout so I'm going to leave this one for now. Higher priority is fixing the indenting in the NHS.UK frontend jinja components, especially the attributes macro which is very weird at the moment
colinrotherham
left a comment
colinrotherham
left a comment
There was a problem hiding this comment.
Changes are looking ace @MatMoore
Mostly comments unrelated to this PR that I've flagged
Although they raise good questions about checking one variables before presenting another, and/or whether we should show placeholder text (or omitting them entirely) when items don't exist
MatMoore
force-pushed
the
DTOSS-10296-split-participant-details
branch
from
5715df3 to
8ffb650
Compare
(not sure why this wasn't failing before)
This brings it inline with the general pattern of namespacing custom components. I've also made `__tag` the element and `--temporary` `--no-details` are modifiers.
For the time being, there are additional djlint issues that need correcting, but I'll fix them in a separate PR.
This is redundant - we can use the classes from the design system instead.
MatMoore
force-pushed
the
DTOSS-10296-split-participant-details
branch
from
3f8db9a to
9b9f57a
Compare
2 participants
Description
This PR splits the personal details card into 3 separate cards, each with their own summary list. I've extracted all of these and the appointments card into macros to simplify the show template. These may be reused later on the appointment page, but for now the appointment page still has a big combined card, so I've left the old participant-details template in the code.
I've also put in some tests for the bits I've extracted. These are not exhaustive, but I think we should move towards doing more testing at the component level as this is somewhere we have gaps in our unit test coverage, and I've noticed a couple of bugs slipping through.
Jira link
Review notes
The prototype is using a modified version of the summary list that allows extracting each row into a separate template, see this slack comment. While this is quite nice for reusing stuff across pages, I don't want to diverge from NHS.UK frontend components as it will add extra maintenance burden. For now I've just accepted a bit of duplication here.
I've also included a fix for a failing test - for some reason this didn't fail in CI but it did locally.
I've not added or removed any rows that weren't already there but there are some new bits in the prototype. I'm going to card these up separately.