Skip to content

Conversation

@MatMoore
Copy link
Contributor

@MatMoore MatMoore commented Nov 12, 2025

Description

This is the form for adding breast cancer history.

This one has multiple questions that are presented in a 2 column layout.

The year field is not done yet - waiting for #739 to be merged and then I'll rebase.

Screenshot of the new form Screenshot of the new form with errors

Jira link

https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11446

Review notes

  • "Additional details" has quite a big bottom margin which I've left unchanged for now. It comes from the character count component.
  • I'm not sure if any additional validation is needed besides the year validation. If only one breast is selected in the first question, does that need to limit what you can select in the subsequent questions? At the moment any selection is possible.

Review checklist

  • Check database queries are correctly scoped to current_provider

@MatMoore MatMoore changed the base branch from main to dtos-114466-add-breast-cancer-history November 12, 2025 12:49
Base automatically changed from dtos-114466-add-breast-cancer-history to main November 13, 2025 09:27
@MatMoore MatMoore force-pushed the dtos-114466-add-breast-cancer-history-2 branch 3 times, most recently from 9f29c1e to a4a2379 Compare November 20, 2025 11:54
@MatMoore MatMoore changed the title [wip] breast cancer history add form Breast cancer history add form Nov 20, 2025
@MatMoore MatMoore marked this pull request as ready for review November 20, 2025 12:04
@MatMoore MatMoore requested a review from a team as a code owner November 20, 2025 12:04
@MatMoore MatMoore force-pushed the dtos-114466-add-breast-cancer-history-2 branch from aedeebe to c0b34a4 Compare November 20, 2025 16:33
@github-actions
Copy link

github-actions bot commented Nov 20, 2025

The review app at this URL has been deleted:
https://pr-708.manage-breast-screening.non-live.screening.nhs.uk

if explicitly_set_html:
return explicitly_set_html

if isinstance(self.form, FormWithConditionalFields):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just to check I've understood it correctly, I think this is to avoid us having to add to breast_cancer_history_item_form.jinja lines like:

{% do form.systemic_treatments.add_conditional_html('OTHER', form.systemic_treatments_other_treatment_details.as_field_group()) %}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep 👍🏻

Copy link
Contributor

@swebberuk swebberuk 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 to me.👍

Need to resolve conflicts before merging. The year validation could be done separately once the YearField from PR #739 has been merged.

The design system radios and checkboxes components take a "conditional"
param when defining the options. This allows you to add arbitrary html
that is conditionally shown when the option is selected.

Now that we have FormWithConditionalFields, we can declare the
relationships between conditionally dependent fields.

This commit extends that so that the conditional param is generated
based on those declarations, so you don't need to explicitly tell
the field what the conditional html is in the template.

The old method for setting arbitrary html is still there in case we need
it for more complex cases, where we might want to conditionally show
more than just a simple field.
There are two new patterns here for arranging checkboxes
and radios into a grid. This is because we want to show
options relating to the right breast on the left
and left breast on the right, to match the perspective
of the mammographer facing the participant.

In one case, we are treating right breast and left breast as separate
fields, and rendering them side by side. Both fields have their own
legends and are validated independently.

In the other case, we model the question as a single field and we just
arrange the options. We implement this by calling the component twice
for the same field, but passing in different options.
The design system checkboxes component allows marking options as
exclusive, meaning they toggle all the other options off when selected.
@MatMoore MatMoore force-pushed the dtos-114466-add-breast-cancer-history-2 branch from c0b34a4 to 3ae8884 Compare November 24, 2025 14:05
@MatMoore MatMoore merged commit 2b67447 into main Nov 24, 2025
13 checks passed
@MatMoore MatMoore deleted the dtos-114466-add-breast-cancer-history-2 branch November 24, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants