Skip to content

[DTOSS-10410] Extract CharField subclass that renders itself using input or textarea components from the design system#230

Merged
MatMoore merged 6 commits intomainfrom
dtoss-9814-extract-charfield-intfield-subclasses
Aug 6, 2025
Merged

[DTOSS-10410] Extract CharField subclass that renders itself using input or textarea components from the design system#230
MatMoore merged 6 commits intomainfrom
dtoss-9814-extract-charfield-intfield-subclasses

Conversation

@MatMoore
Copy link
Copy Markdown
Contributor

@MatMoore MatMoore commented Jul 24, 2025

Description

This is a first step at cleaning up the templates, which have a lot of awkward logic in them for wrangling the form data into the right format for the design system components.

If you look at the working with forms guide, the simplest, but least flexible way to render a form in Django is like this:

<form action="/your-name/" method="post">
    {% csrf_token %}
    {{ form }}
    <input type="submit" value="Submit">
</form>

If you want to add more flexibility you can arrange the fields yourself, like this:

{{ form.non_field_errors }}
<div class="fieldWrapper">
  {{ form.subject.as_field_group }}
</div>
<div class="fieldWrapper">
  {{ form.message.as_field_group }}
</div>
<div class="fieldWrapper">
  {{ form.sender.as_field_group }}
</div>
<div class="fieldWrapper">
  {{ form.cc_myself.as_field_group }}
</div>

Until now, we have opted for a third approach, rendering form fields manually, but this is very error prone and verbose.

My aim is to make it possible for all forms to be written in the as_field_group style, while using NHS.UK frontend jinja to generate the HTML.

This PR makes this possible for the first couple of field types, text inputs and textareas.

Next steps:

  • IntegerFields
  • ChoiceFields
  • SplitDateInput

Jira link

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

How it works

When a template calls {{ form.subject.as_field_group }}, what this is doing is asking the BoundField object to render Django's internal field template, which in turn renders the template associated with the field's widget (or group of widgets).

Diagram showing the definition time objects (Field, Widget) and template rendering objects (BoundField, BoundWidget)

With some settings changes, we can swap out the templates used for this to do whatever we want. This allows us to extract awkward templating from "external" view templates into the "internal" templates, and reuse them across forms.

In order to use the templates, we need to subclass all the built in Field subclasses. This is likely to be useful for other reasons, e.g. we can use this to customise the default error messages per field type to be more in line with the design system guidance.

The second piece of the puzzle is that instead of having a field template that includes a widget template, we render the whole component in the field template and skip the widget template entirely. This is because the scope of a design system component is wider than a Django widget template, which typically renders only an input element, and not labels, errors etc.

Review notes

I've tried to keep the custom field subclasses as close as possible to the built in ones, so that anyone coming to this project with prior knowledge of Django will be able to create forms in the same way they normally would, and then it renders the proper NHS.UK component. To toggle between a text input and textarea, you need to pass widget=Textarea.

I have had to add some custom parameters, in order to preserve compatibility with all the design system's params. At the field level I've added:

  • hint for hint text
  • label_classes for applying extra classes to the label
  • classes for applying extra classes to the input

Widgets already allow passing through arbitrary attributes to add to the input field, so I've used this mechanism for the rows, columns, autocomplete params of the textarea component so as not to diverge from conventions too much. I think classes could be done this way but I thought it was simpler to keep it as a field level param.

I've named the custom CharField as just CharField but I'm not sure if that is a good idea. Should we stick a prefix on it?

This switches Django's template rendering for forms to use the normal
template rendering mechanism, which allows us to customise the built-in
templates.

See https://docs.djangoproject.com/en/dev/ref/forms/renderers/#overriding-built-in-field-templates
This overrides the default "field" template that looks like this (jinja
version)

     {% if field.use_fieldset %}
       <fieldset{% if field.aria_describedby %} aria-describedby="{{ field.aria_describedby }}"{% endif %}>
       {% if field.label %}{{ field.legend_tag() }}{% endif %}
     {% else %}
       {% if field.label %}{{ field.label_tag() }}{% endif %}
     {% endif %}
     {% if field.help_text %}<div class="helptext"{% if field.auto_id %} id="{{ field.auto_id }}_helptext"{% endif %}>{{ field.help_text|safe }}</div>{% endif %}
     {{ field.errors }}
     {{ field }}
     {% if field.use_fieldset %}</fieldset>{% endif %}

The `{{ field }}` in this template here triggers another template render
via the widget class. This is what allows you to swap out the widget at
form declaration time and get a different html element rendered.

Since the widget is more granular than our design system components,
we do not want to have this two-step rendering, and instead
of customising at a widget level, we will override the
field template per field type.

The idea is to have a subclass for every field type, and for that
to render the matching design system component.
@MatMoore MatMoore force-pushed the dtoss-9814-extract-charfield-intfield-subclasses branch 2 times, most recently from 570617a to 47f7b68 Compare August 4, 2025 16:04
@MatMoore MatMoore force-pushed the dtoss-9814-extract-charfield-intfield-subclasses branch from 47f7b68 to 506c71f Compare August 5, 2025 07:58
@MatMoore MatMoore changed the title Extract CharField helper that renders itself using input Extract CharField subclass that renders itself using input or textarea components from the design system Aug 5, 2025
@MatMoore MatMoore marked this pull request as ready for review August 5, 2025 09:15
@MatMoore MatMoore requested a review from malcolmbaig August 5, 2025 09:17
Copy link
Copy Markdown
Contributor

@colinrotherham colinrotherham Aug 5, 2025

Choose a reason for hiding this comment

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

What do you want to do about WCAG 2.2 Identify Input Purpose (Level AA)?

E.g. Adding type: 'tel' or autocomplete: 'address-line1' to fields

Similarly value added params like:

  1. spellcheck: false (supported by NHS.UK frontend)
  2. autocapitalize: "none" (not supported yet, available via params.attributes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suspect in nearly all cases we don't want autocomplete attributes, and indeed may need to do some extra work to try to prevent browsers autofilling.

We may want them on a pre-appointment questionnaire, but not in screening.

Type is one we should have though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We probably want spellcheck: false as a default for fields, but with the option we could turn it on. It might make sense for some of the freetext fields - though BSO's heavy use of abbreviations may mean it's more harm than good.

Copy link
Copy Markdown
Contributor Author

@MatMoore MatMoore Aug 5, 2025

Choose a reason for hiding this comment

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

I'll add in all the ones that are supported by NHS.UK frontend. With type, Django models each value as a different class, e.g. TelInput, SearchInput, so I'll see if I can mirror that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated this now and added a few more tests.

passing in arbitrary attributes:

CharField(
                label="Extra",
                widget=TextInput(
                    attrs=dict(
                        autocomplete="off",
                        inputmode="numeric",
                        spellcheck="false",
                        autocapitalize="none",
                        pattern=r"\d{3}",
                    )
                ),
            )

overriding type:

CharField(label="Ring ring", widget=TelInput)

@MatMoore MatMoore force-pushed the dtoss-9814-extract-charfield-intfield-subclasses branch from 256efa5 to 1bd4577 Compare August 5, 2025 13:54
@MatMoore MatMoore changed the title Extract CharField subclass that renders itself using input or textarea components from the design system [DTOSS-10410] Extract CharField subclass that renders itself using input or textarea components from the design system Aug 5, 2025
"field_name": field_name,
"details_field": form[field_name + "_details"],
})
"html": form[field_name + "_details"].as_field_group()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👌🏼

@malcolmbaig
Copy link
Copy Markdown
Contributor

Stray fixup comment in dfdc86c

Copy link
Copy Markdown
Contributor

@malcolmbaig malcolmbaig left a comment

Choose a reason for hiding this comment

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

👍🏼

The design system provides params.attributes as an escape hatch for
adding additional HTML attributes that don't have their own parameters.

Django also provides an attrs argument to the widget that does
a similar thing.

We can connect the two providing that we drop the default maxlength
attribute which we don't want.
e.g. TelInput, EmailInput, SearchInput etc. These just change the type
param.
@MatMoore MatMoore force-pushed the dtoss-9814-extract-charfield-intfield-subclasses branch from 1bd4577 to 8623967 Compare August 6, 2025 13:05
@MatMoore MatMoore merged commit c5bc0fc into main Aug 6, 2025
7 checks passed
@MatMoore MatMoore deleted the dtoss-9814-extract-charfield-intfield-subclasses branch August 6, 2025 13:25
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