Skip to content

Commit

Permalink
View License page refactor to load pages mapped to controllers when n…
Browse files Browse the repository at this point in the history
…avigating by tabs.

This PR maps the generic controller view license controller to the summary tabs and associated data.

To switch using hrefs instead of anchor tabs the default GDS javascript is turned off and the html used instead of the component (this only supports anchor tags)

https://eaflood.atlassian.net/browse/WATER-4457

Context:

The current view licence page is built using tabs. The design system makes it very clear that tabs should only be used to allow users to switch between related pieces of information. The example it gives is cases (from a case management system) split over Past day, Past week, Past month and Past year.

They very clearly state

Do not use the tabs component if the total amount of content the tabs contain will make the page slow to load. For this reason, do not use the tabs component as a form of page navigation.

We may not have meant to start out using them in this way. But we’ve ended up there. And because of the issue of performance, we’ve made design decisions based on negating the issue rather than what users need.

For example, we show only the first 10 returns on the Returns tab and then a link to view the full list. This is because the tab component works on the basis that the data for all the tabs is fetched in one go, and the tabs are all generated on the first request. Then HTML magic switches what HTML panel the user sees when they click a tab.

Redesigning the view licence page is beyond the scope of the refactoring. But we can take steps to break away from the need to fetch and generate the tabs all in one go.
  • Loading branch information
jonathangoulding committed Apr 29, 2024
1 parent c216d12 commit 35db53a
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 86 deletions.
6 changes: 4 additions & 2 deletions app/controllers/licences.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,21 @@ async function returnsRequired (request, h) {
return h.redirect(`/system/return-requirements/${session.id}/start-date`)
}

async function view (request, h) {
async function viewSummary (request, h) {
const { id } = request.params

const data = await ViewLicenceService.go(id)

return h.view('licences/view.njk', {
activeNavBar: 'search',
activeTab: 'summary',
path: '/system/licences/' + request.params.id,
...data
})
}

module.exports = {
noReturnsRequired,
returnsRequired,
view
viewSummary
}
4 changes: 2 additions & 2 deletions app/routes/licence.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ const LicencesController = require('../controllers/licences.controller.js')
const routes = [
{
method: 'GET',
path: '/licences/{id}',
handler: LicencesController.view,
path: '/licences/{id}/summary',
handler: LicencesController.viewSummary,
options: {
auth: {
access: {
Expand Down
145 changes: 63 additions & 82 deletions app/views/licences/view.njk
Original file line number Diff line number Diff line change
Expand Up @@ -5,95 +5,76 @@
{% from "govuk/components/warning-text/macro.njk" import govukWarningText %}

{% block breadcrumbs %}
{{
govukBackLink({
text: 'Go back to search',
href: "/licences"
})
}}
{{ govukBackLink({
text: 'Go back to search',
href: "/licences"
}) }}
{% endblock %}

{% set summaryTabHtml %}
{% include "licences/tabs/summary.njk" %}
{% endset %}

{% set contactDetailsTabHtml %}
{% endset %}

{% set returnsTabHtml %}
{% endset %}

{% set communicationsTabHtml %}
{% endset %}

{% set billsTabHtml %}
{% endset %}

{% set chargeInformationTabHtml %}
{% endset %}

{% block content %}
{% if warning %}
{{ govukWarningText({
text: warning,
iconFallbackText: "Warning"
}) }}
{% endif %}
{% if warning %}
{{ govukWarningText({
text: warning,
iconFallbackText: "Warning"
}) }}
{% endif %}

{% if notification %}
{% if notification %}
{{ govukNotificationBanner({
html: '<strong>' + notification + '</strong>'
}) }}
{% if notification %}
{{ govukNotificationBanner({
html: '<strong>' + notification + '</strong>'
}) }}
{% endif %}
{% endif %}
{% endif %}

<span class="govuk-caption-l">{{ licenceName }}</span>
<h1 class="govuk-heading-l">Licence number {{ licenceRef }}</h1>
<span class="govuk-caption-l">{{ licenceName }}</span>
<h1 class="govuk-heading-l">Licence number {{ licenceRef }}</h1>

{% if registeredTo %}
<p>Registered to <a href="/user/{{ primaryUser.userId }}/status">{{ registeredTo }}</a></p>
{% endif %}

{% if registeredTo %}
<p>Registered to <a href="/user/{{primaryUser.userId}}/status">{{ registeredTo }}</a></p>
{% endif %}
<div class="govuk-tabs">
<h2 class="govuk-tabs__title">
Contents
</h2>
<ul class="govuk-tabs__list">
<li class="govuk-tabs__list-item govuk-tabs__list-item--selected">
<a class="govuk-tabs__tab" href="{{ path }}/summary">
Summary
</a>
</li>
<li class="govuk-tabs__list-item">
<a class="govuk-tabs__tab" href="{{ path }}/contact-details">
Contact details
</a>
</li>
<li class="govuk-tabs__list-item">
<a class="govuk-tabs__tab" href="{{ path }}/returns">
Returns
</a>
</li>
<li class="govuk-tabs__list-item">
<a class="govuk-tabs__tab" href="{{ path }}/communications">
Communications
</a>
</li>
<li class="govuk-tabs__list-item">
<a class="govuk-tabs__tab" href="{{ path }}/bills">
Bills
</a>
</li>
<li class="govuk-tabs__list-item">
<a class="govuk-tabs__tab" href="{{ path }}/charge-information">
Charge information
</a>
</li>
</ul>

{{ govukTabs({
items: [
{
label: "Summary",
id: "summary",
panel: {
html: summaryTabHtml
}
}, {
label: "Contact details",
id: "contact-details",
panel: {
html: contactDetailsTabHtml
}
}, {
label: "Returns",
id: "returns",
panel: {
html: returnsTabHtml
}
}, {
label: "Communications",
id: "communications",
panel: {
html: communicationsTabHtml
}
}, {
label: "Bills",
id: "bills",
panel: {
html: billsTabHtml
}
}, {
label: "Charge information",
id: "charge-information",
panel: {
html: chargeInformationTabHtml
}
}
]
}) }}
<div class="govuk-tabs__panel" id="{{ activeTab }}">
{% if activeTab === 'summary' %}
{% include "licences/tabs/summary.njk" %}
{% endif %}
</div>
</div>
{% endblock %}

0 comments on commit 35db53a

Please sign in to comment.