Merge pull request #154 from NHSDigital/PPHA-262-height-page-updates

Themitchell · web-flow · commit aff9f32141a9 · 2025-11-26T11:31:14.000Z
PPHA-262: height page updates
diff --git a/lung_cancer_screening/core/jinja2/layout.jinja b/lung_cancer_screening/core/jinja2/layout.jinja
@@ -9,6 +9,20 @@
 
 {% block pageTitle %}Lung Cancer Screening - NHS{% endblock %}
 
+{% block beforeContent %}
+  {% if back_link_url %}
+    <nav>
+      {{
+        backLink({
+          "href": back_link_url,
+          "text": "Back"
+        })
+      }}
+    </nav>
+  {% endif %}
+{% endblock beforeContent %}
+
+
 {% block content %}
   {% block page_content %}
   {% endblock page_content %}
diff --git a/lung_cancer_screening/nhsuk_forms/decimal_field.py b/lung_cancer_screening/nhsuk_forms/decimal_field.py
@@ -11,6 +11,7 @@ def __init__(
     ):
         kwargs["template_name"] = "input.jinja"
 
+        self.suffix = kwargs.pop("suffix", None)
         self.hint = hint
         self.classes = classes
         self.label_classes = label_classes
diff --git a/lung_cancer_screening/nhsuk_forms/imperial_height_field.py b/lung_cancer_screening/nhsuk_forms/imperial_height_field.py
@@ -45,18 +45,34 @@ class ImperialHeightField(forms.MultiValueField):
     def __init__(self, *args, **kwargs):
         error_messages = kwargs.get("error_messages", {})
 
+        between_feet="Feet must be between 4 and 8"
+
         feet_kwargs = {
+            "min_value": 4,
+            "max_value": 8,
             "error_messages": {
-                'invalid': 'Feet must be in whole numbers',
                 **error_messages,
+                'invalid': 'Feet must be in whole numbers',
+                'min_value': between_feet,
+                'max_value': between_feet,
+                'incomplete': between_feet,
             },
         }
+
+        between_inches = "Inches must be between 0 and 11"
+
         inches_kwargs = {
+            "min_value": 0,
+            "max_value": 11,
             "error_messages": {
-                'invalid': 'Inches must be in whole numbers',
                 **error_messages,
+                'invalid': 'Inches must be in whole numbers',
+                'min_value': between_inches,
+                'max_value': between_inches,
+                'incomplete': between_inches,
             },
         }
+
         fields = (
             IntegerField(**feet_kwargs),
             IntegerField(**inches_kwargs),
diff --git a/lung_cancer_screening/nhsuk_forms/integer_field.py b/lung_cancer_screening/nhsuk_forms/integer_field.py
@@ -12,6 +12,7 @@ def __init__(
     ):
         kwargs["template_name"] = "input.jinja"
 
+        self.suffix = kwargs.pop("suffix", None)
         self.hint = hint
         self.classes = classes
         self.label_classes = label_classes
diff --git a/lung_cancer_screening/nhsuk_forms/jinja2/imperial-height-input.jinja b/lung_cancer_screening/nhsuk_forms/jinja2/imperial-height-input.jinja
@@ -21,7 +21,6 @@
     {{ input({
       "label": {
         "text": "Feet",
-        "classes": "nhsuk-fieldset__legend--m",
         "isPageHeading": false
       },
       "hint": {
@@ -30,23 +29,24 @@
       "id": field.auto_id + "_0",
       "name": field.html_name + "_0",
       "value": field.subwidgets.0.data.value,
-      "classes": "nhsuk-input--width-2" + field_error_classes,
-      "type": "number"
+      "classes": "nhsuk-input--width-4" + field_error_classes,
+      "type": "number",
+      "suffix": "ft"
     }) }}
   </div>
 
   <div class="multi-field-input__item">
     {{ input({
       "label": {
         "text": "Inches",
-        "classes": "nhsuk-fieldset__legend--m",
         "isPageHeading": false
       },
       "id": field.auto_id + "_1",
       "name": field.html_name + "_1",
       "value": field.subwidgets.1.data.value,
-      "classes": "nhsuk-input--width-2" + field_error_classes,
-      "type": "number"
+      "classes": "nhsuk-input--width-4" + field_error_classes,
+      "type": "number",
+      "suffix": "in"
     }) }}
   </div>
 </div>
diff --git a/lung_cancer_screening/nhsuk_forms/jinja2/input.jinja b/lung_cancer_screening/nhsuk_forms/jinja2/input.jinja
@@ -15,6 +15,7 @@
     "hint": {
       "text": unbound_field.hint
     } if unbound_field.hint,
+    "suffix": unbound_field.suffix if unbound_field.suffix,
     "id": field.auto_id,
     "name": field.html_name,
     "value": field.value() or "",
diff --git a/lung_cancer_screening/nhsuk_forms/tests/unit/test_decimal_field.py b/lung_cancer_screening/nhsuk_forms/tests/unit/test_decimal_field.py
@@ -0,0 +1,38 @@
+from django.test import TestCase
+from django.forms import Form
+from ...decimal_field import DecimalField
+
+
+class TestDecimalField(TestCase):
+    def test_renders_nhs_input(self):
+        class TestForm(Form):
+            field = DecimalField(label="Abc", initial=1, max_value=10)
+        self.assertHTMLEqual(
+            TestForm()["field"].as_field_group(),
+            """
+            <div class="nhsuk-form-group">
+                <label class="nhsuk-label" for="id_field">
+                    Abc
+                </label><input class="nhsuk-input" id="id_field" name="field" type="number" value="1">
+            </div>
+            """,
+        )
+
+    def test_renders_nhs_input_with_suffix(self):
+        class TestForm(Form):
+            field = DecimalField(label="Abc", initial=1, max_value=10, suffix="cm")
+        self.assertHTMLEqual(
+            TestForm()["field"].as_field_group(),
+            """
+            <div class="nhsuk-form-group">
+                <label class="nhsuk-label" for="id_field">
+                    Abc
+                </label>
+                <div class="nhsuk-input__wrapper">
+                    <input class="nhsuk-input" id="id_field" name="field" type="number" value="1">
+                    <div class="nhsuk-input__suffix" aria-hidden="true">cm</div>
+                </div>
+            </>
+            """,
+        )
+
diff --git a/lung_cancer_screening/nhsuk_forms/tests/unit/test_integer_field.py b/lung_cancer_screening/nhsuk_forms/tests/unit/test_integer_field.py
@@ -3,11 +3,10 @@
 from ...integer_field import IntegerField
 
 
-class TestForm(Form):
-    field = IntegerField(label="Abc", initial=1, max_value=10)
-
 class TestIntegerField(TestCase):
     def test_renders_nhs_input(self):
+        class TestForm(Form):
+            field = IntegerField(label="Abc", initial=1, max_value=10)
         self.assertHTMLEqual(
             TestForm()["field"].as_field_group(),
             """
@@ -19,3 +18,20 @@ def test_renders_nhs_input(self):
             """,
         )
 
+    def test_renders_nhs_input_with_suffix(self):
+        class TestForm(Form):
+            field = IntegerField(label="Abc", initial=1, max_value=10, suffix="suffix")
+        self.assertHTMLEqual(
+            TestForm()["field"].as_field_group(),
+            """
+            <div class="nhsuk-form-group">
+                <label class="nhsuk-label" for="id_field">
+                    Abc
+                </label>
+                <div class="nhsuk-input__wrapper">
+                    <input class="nhsuk-input" id="id_field" name="field" type="number" value="1">
+                    <div class="nhsuk-input__suffix" aria-hidden="true">suffix</div>
+                </div>
+            </div>
+            """,
+        )
diff --git a/lung_cancer_screening/questions/forms/imperial_height_form.py b/lung_cancer_screening/questions/forms/imperial_height_form.py
@@ -1,6 +1,6 @@
 from django import forms
 
-from ...nhsuk_forms.imperial_height_form import ImperialHeightField
+from ...nhsuk_forms.imperial_height_field import ImperialHeightField
 from ..models.response_set import ResponseSet
 
 class ImperialHeightForm(forms.ModelForm):
@@ -16,7 +16,6 @@ def __init__(self, *args, **kwargs):
             require_all_fields=False,
             error_messages={
                 'required': 'Enter your height',
-                'incomplete': 'Enter your height'
             }
         )
 
diff --git a/lung_cancer_screening/questions/forms/metric_height_form.py b/lung_cancer_screening/questions/forms/metric_height_form.py
@@ -11,11 +11,14 @@ def __init__(self, *args, **kwargs):
         self.instance.participant = self.participant
 
         self.fields["height"] = DecimalField(
+            decimal_places=1,
             label="Centimetres",
             classes="nhsuk-input--width-4",
             error_messages={
                 'required': 'Enter your height',
-            }
+                "max_decimal_places": "Centimetres must be to 1 decimal place, for example 185.5cm",
+            },
+            suffix="cm"
         )
 
     def clean_height(self):
diff --git a/lung_cancer_screening/questions/jinja2/height.jinja b/lung_cancer_screening/questions/jinja2/height.jinja
@@ -3,6 +3,11 @@
 {% from 'nhsuk/components/back-link/macro.jinja' import backLink %}
 {% from 'nhsuk/components/fieldset/macro.jinja' import fieldset %}
 
+{% set HEIGHT_UNIT = {
+    'imperial': 'feet and inches',
+    'metric': 'centimetres'
+} %}
+
 {% block beforeContent %}
 <nav>
   {{
@@ -19,15 +24,15 @@
   <div class="nhsuk-grid-column-two-thirds">
     <form action="{{ request.get_full_path() }}" method="POST" novalidate>
       {{ csrf_input }}
-      <h1 class="nhsuk-heading-l">What is your height?</h1>
+      <h1 class="nhsuk-heading-l">Enter your height</h1>
       <p>An accurate measurement is important.
 
       <p>You can measure your height at home with a measuring tape. Some pharmacies and gyms have machines to measure
         your height.
         {% call fieldset({
           "legend": {
-            "text": "Enter your height",
-            "classes": "nhsuk-label--m"
+            "text": "What is your height?",
+            "classes": "nhsuk-fieldset__legend nhsuk-fieldset__legend--m"
           }
         })
         %}
@@ -39,7 +44,7 @@
         {% endif %}
 
 
-      <p><a href="?unit={{ switch_to_unit }}">Switch to {{ switch_to_unit }}</a></p>
+      <p><a href="?unit={{ switch_to_unit }}">Switch to {{ HEIGHT_UNIT[switch_to_unit] }}</a></p>
 
       {% endcall %}
 
diff --git a/lung_cancer_screening/questions/jinja2/question_form.jinja b/lung_cancer_screening/questions/jinja2/question_form.jinja
@@ -7,17 +7,6 @@
   {% set error_message = { "text": error } %}
 {% endif %}
 
-{% block beforeContent %}
-  <nav>
-    {{
-      backLink({
-        "href": back_link_url,
-        "text": "Back"
-      })
-    }}
-  </nav>
-{% endblock beforeContent %}
-
 {% block page_content %}
   <div class="nhsuk-grid-row">
     <div class="nhsuk-grid-column-two-thirds">
diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_imperial_height_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_imperial_height_form.py
@@ -53,7 +53,7 @@ def test_is_invalid_with_missing_data(self):
             participant=self.participant,
             instance=self.response_set,
             data={
-                "height_imperial_0": "5",
+                # missing feet
                 # missing inches
             }
         )
@@ -63,6 +63,36 @@ def test_is_invalid_with_missing_data(self):
             ["Enter your height"]
         )
 
+    def test_is_invalid_with_missing_inches(self):
+        form = ImperialHeightForm(
+            participant=self.participant,
+            instance=self.response_set,
+            data={
+                "height_imperial_0": "5",
+                # missing inches
+            }
+        )
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            form.errors["height_imperial"],
+            ["Inches must be between 0 and 11"]
+        )
+
+    def test_is_invalid_with_missing_feet(self):
+        form = ImperialHeightForm(
+            participant=self.participant,
+            instance=self.response_set,
+            data={
+                #"height_imperial_0": "5",
+                "height_imperial_1": "5"
+            }
+        )
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            form.errors["height_imperial"],
+            ["Feet must be between 4 and 8"]
+        )
+
     def test_is_invalid_when_given_a_decimal_feet_value(self):
         form = ImperialHeightForm(
             participant=self.participant,
@@ -77,3 +107,63 @@ def test_is_invalid_when_given_a_decimal_feet_value(self):
             form.errors["height_imperial"],
             ["Feet must be in whole numbers"]
         )
+
+    def test_is_invalid_when_inches_under_11(self):
+        form = ImperialHeightForm(
+            participant=self.participant,
+            instance=self.response_set,
+            data={
+                "height_imperial_0": "5",
+                "height_imperial_1": "12"
+            }
+        )
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            form.errors["height_imperial"],
+            ["Inches must be between 0 and 11"]
+        )
+
+    def test_is_invalid_when_inches_over_0(self):
+        form = ImperialHeightForm(
+            participant=self.participant,
+            instance=self.response_set,
+            data={
+                "height_imperial_0": "5",
+                "height_imperial_1": "-1"
+            }
+        )
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            form.errors["height_imperial"],
+            ["Inches must be between 0 and 11"]
+        )
+
+    def test_is_invalid_when_feet_over_4(self):
+        form = ImperialHeightForm(
+            participant=self.participant,
+            instance=self.response_set,
+            data={
+                "height_imperial_0": "3",
+                "height_imperial_1": "10"
+            }
+        )
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            form.errors["height_imperial"],
+            ["Feet must be between 4 and 8"]
+        )
+
+    def test_is_invalid_when_feet_under_8(self):
+        form = ImperialHeightForm(
+            participant=self.participant,
+            instance=self.response_set,
+            data={
+                "height_imperial_0": "9",
+                "height_imperial_1": "0"
+            }
+        )
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            form.errors["height_imperial"],
+            ["Feet must be between 4 and 8"]
+        )
diff --git a/lung_cancer_screening/questions/tests/unit/forms/test_metric_height_form.py b/lung_cancer_screening/questions/tests/unit/forms/test_metric_height_form.py
diff --git a/tests/features/questionnaire.feature b/tests/features/questionnaire.feature

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`from django import forms`
`2`	`2`
`3`		`-from ...nhsuk_forms.imperial_height_form import ImperialHeightField`
	`3`	`+from ...nhsuk_forms.imperial_height_field import ImperialHeightField`
`4`	`4`	`from ..models.response_set import ResponseSet`
`5`	`5`
`6`	`6`	`class ImperialHeightForm(forms.ModelForm):`
`@@ -16,7 +16,6 @@ def __init__(self, args, *kwargs):`
`16`	`16`	`require_all_fields=False,`
`17`	`17`	`error_messages={`
`18`	`18`	`'required': 'Enter your height',`
`19`		`- 'incomplete': 'Enter your height'`
`20`	`19`	`}`
`21`	`20`	`)`
`22`	`21`