Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 1st pass issues with bill & bill licence pages #511

Merged
merged 7 commits into from
Nov 7, 2023

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented Nov 7, 2023

https://eaflood.atlassian.net/browse/WATER-4155
https://eaflood.atlassian.net/browse/WATER-4156

After a first pass of the new view bill and bill-licence pages we've built some issues have been spotted. This change resolves

Broken screen reader text in 'View transactions link'**

We overlooked updating the context property being referenced in a refactor.

Fix undefined in view bill licence

We were only setting, for example, the additionalChargesDetail variable in the template if there were additional charges. But we were then referencing additionalChargesDetail when setting detailChargeInfo.

{% set detailChargeInfo = detailChargeInfo + additionalChargesDetail + adjustmentsDetail + '</p>' + elementDetail %}

This is why the template was showing undefinedundefined when a transaction had no additional charges or adjustments. By flipping the logic, i.e. always instantiating a variable but only populating it when something exists we avoid Nunjucks thinking additionalChargesDetail or adjustmentsDetail are ever undefined.

Use set address for view bill backlink

The initial requirement for the story was that however a user came to the view bill page the backlink should take them to where they came from. This could be 1 of 3 places

  • bill run summary
  • billing account summary
  • billing account all bills

When you first come to the view bill page we use referrer to generate the backlink. At this point we can get you back to where you came from.

But that list is incomplete with the addition of the view bill-licence page (see WATER-4156 ). So, you come to the view bill page, then click through to the view bill-licence page and then go back, now referrer is the view bill-licence. This results in the user being stuck in a loop.

  [source] --> [view bill    ] --> [view bill-licence] --> [view bill               ]
               [back = source]     [back = view bill ]     [back = view bill-licence]

Having discussed this with our designer, it was agreed having the backlink attempt to recreate what the back button does is a type of anti-pattern. Backlinks we build into the pages should help a user navigate through a planned journey. Essentially, if you come from, for example, billing account summary, you have jumped from the licence summary journey to the bill runs journey. As such when you go back, you should go back through the bill runs journey.

Better backlink text

Following on from our decision to always have backlinks take you to a specific point in a planned journey we also agreed we'd make it clearer where the backlink will take you in the UI.

We're using the advice in the design system for back links to use different link text.

For more complex user journeys, consider using different link text, like ‘Go back to [page]’. For example, in an admin system with many different areas. In this case, if you used ‘Back’, it might not be clear to users what they are going back to.

We've applied this advice to both the view bill and view bill-licence pages.

https://eaflood.atlassian.net/browse/WATER-4155
https://eaflood.atlassian.net/browse/WATER-4156

After a first pass of the new view bill and bill-licence pages we've built some issues have been spotted. This is a 'quick-fix' change for those.

- fix outdated reference to licence number in screen reader text
- fix content info in header
@Cruikshanks Cruikshanks added the enhancement New feature or request label Nov 7, 2023
@Cruikshanks Cruikshanks self-assigned this Nov 7, 2023
@Cruikshanks Cruikshanks added bug Something isn't working and removed enhancement New feature or request labels Nov 7, 2023
Clearly forgot to update the property we were referring to during a refactor.
We were only setting, for example, the `additionalChargesDetail` variable in the template if there were additional charges. But we were then referencing `additionalChargesDetail` when setting `detailChargeInfo`.

```
{% set detailChargeInfo = detailChargeInfo + additionalChargesDetail + adjustmentsDetail + '</p>' + elementDetail %}
```

This is why the template was showing `undefinedundefined` when a transaction has no additional charges or adjustments. By flipping the logic, i.e. always instantiating a variable but only populating it when something exists we avoid Nunjucks thinking `additionalChargesDetail` or `adjustmentsDetail` are ever undefined.
The initial requirement for the story was that however a user comes to the new bill view page the back link should take them to where they came from. This could be 1 of 3 places currently

- bill run summary
- billing account summary
- billing account all bills

When you first come to the view bill page we use [referrer](#486) to generate the back link. At this point we can get you back to where you came from.

But that list is incomplete with the addition of the view bill-licence page (see WATER-4156 ). So, you come to the view bill page, then click through to the view bill-licence page and then go back, now `referrer` is the view bill-licence. This results in the user being stuck in a loop.

```
  [source] --> [view bill    ] --> [view bill-licence] --> [view bill               ]
               [back = source]     [back = view bill ]     [back = view bill-licence]
```

Having discussed this with our designer, we both agree having the back link attempt to recreate what the back button does is an anti-pattern. Back links we build into the pages should help a user navigate through a _planned_ journey. Essentially, if you come from, for example billing account summary, you have jumped from the licence summary journey to the bill runs journey. As such when you go back, you should go back through the bill runs journey.

There are relevant points that allow the user to switch back, for example clicking the Billing account link in the view bill page. If the user wants back button behaviour then the button is always there.
Following on from our decision to always have the back link in the view bill page take a user to the bill run irrespective of where they came from we also agreed we'd make that clear in the UI.

We're using the [advice in the design system for back links](https://design-system.service.gov.uk/components/back-link/) to use different link text.

> For example, in an admin system with many different areas. In this case, if you used 'Back', it might not be clear to users what they are going back to.
Now we've expanded what the back link says in view bill we've agreed we also need to make it clearer in view bill-licence, if only to be consistent.
@Cruikshanks Cruikshanks marked this pull request as ready for review November 7, 2023 17:19
@Cruikshanks Cruikshanks merged commit aa02929 into main Nov 7, 2023
4 checks passed
@Cruikshanks Cruikshanks deleted the fix-initial-issues-with-new-bill-views branch November 7, 2023 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant