Appointment status bar

[swebberuk]

Description

1. Copied CSS from prototype and added to main.scss
2. Copied HTML from prototype and added to new appointment-status-bar/template.jinja
3. Updated layout-app.jinja to include appointment-status-bar/template.jinja

What appointment status bar looks like with changes from this PR:

How appointment status bar looks in prototype:

Jira link

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

Review notes

Review checklist

• Check database queries are correctly scoped to current_provider

[swebberuk]

I'll need to alter this if condition - story states "The bar should only display if the current user is the one that has the appointment 'in progress'". Don't know if we've already done something similar somewhere else?

[MatMoore]

The only thing that comes to mind is for workflow transitions like starting appointments, we have to check both whether the appointment is startable (based on its state) and whether the current user is permitted to start the appointment.

[swebberuk]

Have tried to implement adding a show_status_bar_for function in AppointmentPresenter:

def show_status_bar_for(self, user):
    # The appointment status bar should only display if the current user
    # is the one that has the appointment 'in progress'
    current_status = self._appointment.current_status
    return (
        current_status.state == AppointmentStatus.IN_PROGRESS
        and user.nhs_uid == current_status.created_by.nhs_uid
    )

There was a suggestion that the current owner should be stored the appointment table, rather than determining using self._appointment.current_status. If we think that would be useful then I could look at doing that as a separate task.

[edwardhorsford]

The prototype has a quite explicit concept of "currently running the workflow in this session" - which I'm not sure prod (yet?) has - not sure what the closest equivalent on prod would be.

[swebberuk]

Also need to add a yellow tag if the appointment is 'special'. If I try:

{% if presented_appointment.is_special_appointment %}
{{ tag(presented_appointment.special_appointment_tag_properties) }}
{% endif %}

Then the "Special appointment" text doesn't align with the other text. This is because special_appointment_tag_properties contains nhsuk-u-margin-top-2.

Looks okay if, rather than using special_appointment_tag_properties, I add the HTML myself - but seems wrong that have to do this:

{% if presented_appointment.is_special_appointment %}
<strong class="nhsuk-tag nhsuk-tag--yellow nhsuk-u-nowrap">
  Special appointment
</strong>
{% endif %}

[MatMoore]

Yeah it's generally better to use the macros if we can, as it's easy to keep up to date with changes to the design system that way.

Can we split special_appointment_tag_properties into two methods, one for each use case? Or make it a method accepting parameters?

If it helps you could also split out a presenter just for this component

[swebberuk]

Yes, I can split into two methods, but not sure what to name them. Does anyone have any suggestions please?

I could name the new version something like special_appointment_status_bar_tag_properties but that is quite wordy and specific to the status bar. e.g.:

@cached_property
def special_appointment_status_bar_tag_properties(self):
	return {
		"classes": "nhsuk-tag--yellow nhsuk-u-margin-left-1",
		"text": "Special appointment",
	}

[MatMoore]

I was thinking you could give the presented appointment a member status_bar, pass that to this component instead of the presented_appointment itself, and then call this property as presented_status_bar.tag_properties

Any other suggestions welcome though, I haven't thought about this in too much detail.

[MatMoore]

Have a think about it, but for components like this that might be reusable by other services, we could consider following the design system style of having each macro take a single params argument, rather than presented_appointment as you have here.

I don't particularly like this style but given that we are already following the conventions of macro.jinja / template.jinja files, and namespacing all the css, it might make it easier for other teams to lift and shift.

[swebberuk]

I did rename presented_appointment to params. I then wanted to add another parameter user. It seemed easier to understand when using two parameters, rather than combining into a Dict, so reverted to using: appointment_status_bar(user, presented_appointment). If we want to go with a single parameter named params then I can do that.

[MatMoore]

In that case, shall we just have a single jinja file instead of splitting the macro.jinja and template.jinja?

[swebberuk]

Combined macro.jinja and template.jinja into single file named appointment-status-bar.jinja

[MatMoore]

The only thing that comes to mind is for workflow transitions like starting appointments, we have to check both whether the appointment is startable (based on its state) and whether the current user is permitted to start the appointment.

[MatMoore]

Yeah it's generally better to use the macros if we can, as it's easy to keep up to date with changes to the design system that way.

Can we split special_appointment_tag_properties into two methods, one for each use case? Or make it a method accepting parameters?

If it helps you could also split out a presenter just for this component

[MatMoore]

Will having this in the header mean it's skipped by the skip link? This doesn't seem right to me for something that we want users to perceive for clinical safety reasons.

[swebberuk]

Have asked about skip link in screening-manage-ucd Slack channel.

[swebberuk]

Here is a link to the slack conversation: https://screening-discovery.slack.com/archives/C082XV3GQVA/p1764595157468479?thread_ts=1764594205.836459&cid=C082XV3GQVA

[MatMoore]

I assume this scss is temporary but these colours and paddings should be aligned with the design system tokens where possible

[swebberuk]

Have updated to use tokens where I could find them (e.g. $color_nhsuk-white, nhsuk-spacing(4)). Couldn't find tokens for #00386e.

[MatMoore]

Is it worth extracting a var for it in that case, like $color_app-dark-blue? Or maybe using the colour functions of sass to adjust the shade of an existing colour?

[swebberuk]

Added

$color_app-dark-blue: #00386e;

[MatMoore]

I was thinking you could give the presented appointment a member status_bar, pass that to this component instead of the presented_appointment itself, and then call this property as presented_status_bar.tag_properties

Any other suggestions welcome though, I haven't thought about this in too much detail.

[github-actions]

The review app is available at this URL:
https://pr-783.manage-breast-screening.non-live.screening.nhs.uk
You must authenticate with HTTP basic authentication. Ask the team for credentials.

[MatMoore]

Think this is almost good to merge, just have some questions about the scss.

[MatMoore]

Is it worth extracting a var for it in that case, like $color_app-dark-blue? Or maybe using the colour functions of sass to adjust the shade of an existing colour?

[MatMoore]

should this be nhsuk-typography-weight-bold?

[swebberuk]

replaced with @include nhsuk-typography-weight-bold(true);

[MatMoore]

Where does the 144px come from here? Does it need to be calculated from something else?

[MatMoore]

In that case, shall we just have a single jinja file instead of splitting the macro.jinja and template.jinja?

[edwardhorsford]

I imagine it might be good to put these in their own named file inside a folder like _components?

[edwardhorsford]

The prototype has a quite explicit concept of "currently running the workflow in this session" - which I'm not sure prod (yet?) has - not sure what the closest equivalent on prod would be.

[edwardhorsford]

If you check the scss from the prototype, this was background-color: nhsuk-shade($nhsuk-brand-colour, 40%); which is probably clearer for how we chose this number / where it came from.

[edwardhorsford]

I somewhat wonder if these and other items within the summary list should use a definition list. @colinrotherham any thoughts?