[DTOSS-1110] add notification banners to the base layout and wire up to django.contrib.messages #562
Conversation
autoescape should generally be on all the time. We have a special case due to an issue with one of the macros. This was causing test templates to be rendered improperly.
MatMoore
force-pushed
the
DTOSS-1110-flash-message-layout
branch
from
70d2336 to
b3b30e9
Compare
These are distinct from the flash messages and are shown in a different part of the page.
The message framework doesn't have a way to pass structured data, but we can pass marked-safe HTML, as in this example from stack overflow[1]. This is complicated by us using Jinja, which has it's own way of marking strings safe, which is not recognised by the messages framework. So when *adding* an HTML message, we need to mark it with `mark_safe`, and when *rendering* the message we need to use `Markup`. Supporting html isn't essential, but it means we can show arbitrary content in banners, including the success banner with header shown in the design system example[2] [1]: https://stackoverflow.com/questions/2053258/how-do-i-output-html-in-a-message-in-the-new-django-messages-framework [2]: https://service-manual.nhs.uk/design-system/components/notification-banners
This is temporary pending a content review; it's just to prove that we can use arbitrary HTML for notification messages.
As per the prototype, we don't want notifications autofocused, particularly on appointment pages, as they are non-essential and distract from the task. We are assuming the default autofocus behaviour is helpful for screen reader users, but in our case we know all radiologists taking the mammograms will be sighted. This can be overriden if we need to make exceptions for other pages.
this ensures notifications will be shown
MatMoore
force-pushed
the
DTOSS-1110-flash-message-layout
branch
from
b3b30e9 to
208de91
Compare
This is not something thats supposed to happen, but if for some reason there are both kinds of messages, we should show them all. We can prevent this by not adding messages in form views when the form is invalid.
I'm unsure about doing this. I suspect if there are things to show, we should show them. But... the way of following the design system is to try to design the service well so that such a scenario isn't possible - eg you should never have a case of flash messages and errors at the same time because we ensure we never allow for success when there's been an error. |
| self.request, | ||
| messages.SUCCESS, | ||
| notification_with_heading( | ||
| heading="Symptom deleted", html=mark_safe(f"<p>Deleted {name}.</p>") |
There was a problem hiding this comment.
This looks fine to me, but I wonder if it's worth having the presenter handle construction of the html for various message types rather than the view.
There was a problem hiding this comment.
Yep makes sense. I'll address in a followup PR
malcolmbaig
left a comment
malcolmbaig
left a comment
There was a problem hiding this comment.
One minor observation but otherwise LG 👍🏼
|
The review app at this URL has been deleted: |
3 participants
Description
layout-app.jinjaThe content of the notification is provisional - we will have another ticket to review notification messages throughout the app.
Jira link
https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11110
Review notes
Supporting HTML in messages was slightly awkward due to django.contrib.messages supporting one form of marking strings HTML safe, and Jinja templates supporting another. But I think it's worth it to support arbitrary content in messages.
This will enable us to add info, success, warning messages. If we later decide to support
messages.ERRORmessages they should use the error summary rather than the notification banner. I've left error message out of this PR because I wasn't sure if we needed it and I don't want to add it before we have a use for it.If there are form errors,
the form template will render the error summary, and the base layout will skip the notification banner so that errors and banners are not shown at the same time (as per design system recommendations)we will still render any notification banners. It's up to us to avoid adding extra messages in form views when there are errors.The message framework also has a
messages.DEBUGlevel which I'm ignoring.