diff --git a/src/controllers/resultsController.js b/src/controllers/resultsController.js index aee6192e..387d5420 100644 --- a/src/controllers/resultsController.js +++ b/src/controllers/resultsController.js @@ -71,9 +71,15 @@ class ResultsController extends PageController { href: '' } }) + // pagination is on the 'table' tab, so we want to ensure clicking those + // links takes us to a page with the table tab *selected* + const pagination = responseDetails.getPagination(req.params.pageNumber) + for (const item of pagination.items) { + item.href = item.href + '#table-tab' + } req.form.options.mappings = responseDetails.getFieldMappings() req.form.options.geometries = responseDetails.getGeometries() - req.form.options.pagination = responseDetails.getPagination(req.params.pageNumber) + req.form.options.pagination = pagination req.form.options.id = req.params.id req.form.options.lastPage = `/check/status/${req.params.id}` } else { diff --git a/src/views/check/results/no-errors.html b/src/views/check/results/no-errors.html index af213617..bcaa3791 100644 --- a/src/views/check/results/no-errors.html +++ b/src/views/check/results/no-errors.html @@ -4,9 +4,48 @@ {% from 'govuk/components/radios/macro.njk' import govukRadios %} {% from 'govuk/components/error-summary/macro.njk' import govukErrorSummary %} {% from "govuk/components/pagination/macro.njk" import govukPagination %} +{% from "govuk/components/tabs/macro.njk" import govukTabs %} {% from '../../components/dataset-banner.html' import datasetBanner %} {% from "../../components/table.html" import table %} +{% set mapTab %} +

Map

+ {% if options.geometries %} +
+ {% endif %} +{% endset -%} + +{% set tableTab %} +

Table

+ + {{ table(options.tableParams) }} + + {% if options.pagination.items | length > 1 %} + {{ govukPagination({ + previous: { + href: '/check/results/'+options.id+'/'+options.pagination.previousPage + '#table-tab' + } if options.pagination.previousPage else undefined, + next: { + href: '/check/results/'+options.id+'/'+options.pagination.nextPage + '#table-tab' + } if options.pagination.nextPage else undefined, + items: options.pagination.items + })}} + {% endif %} + +{% endset -%} + +{% set tabClick = "selectTab(event)" %} +{% set tabItemsSelected = { + map: { label: "Map", + id: "map-tab", + panel: { html: mapTab, attributes: { } }, + attributes: { onclick: tabClick } } } %} +{% set tabItemsDeselected = { + table: { label: "Dataset table", + id: "table-tab", + panel: { html: tableTab, attributes: { } }, + attributes: { onclick: tabClick } } +} %} {% set serviceType = 'Check' %} {% set pageName = 'Check your data before you continue' %} @@ -51,27 +90,53 @@

+ +
-
+
+ {# we want to hide the tab, when we have no data for it. We present the Map tab by default #} + {% set items = [] %} {% if options.geometries %} -
- {% endif %} - - {{ table(options.tableParams) }} - - {% if options.pagination.items | length > 1 %} - {{ govukPagination({ - previous: { - href: '/check/results/'+options.id+'/'+options.pagination.previousPage - } if options.pagination.previousPage else undefined, - next: { - href: '/check/results/'+options.id+'/'+options.pagination.nextPage - } if options.pagination.nextPage else undefined, - items: options.pagination.items - })}} + {%set _ = items.push(tabItemsSelected.map) %} {% endif %} + {%set _ = items.push(tabItemsDeselected.table) %} + {{ govukTabs({ items: items })}}
diff --git a/test/unit/noErrorsPage.test.js b/test/unit/noErrorsPage.test.js index 8e4bce44..b0efe324 100644 --- a/test/unit/noErrorsPage.test.js +++ b/test/unit/noErrorsPage.test.js @@ -82,5 +82,5 @@ describe('no Errors Page', () => { }) }) - paginationTemplateTests('results/no-errors.html', nunjucks) + paginationTemplateTests('results/no-errors.html', nunjucks, { hash: '#table-tab' }) }) diff --git a/test/unit/paginationTemplateTests.js b/test/unit/paginationTemplateTests.js index 600165eb..08f8fbb1 100644 --- a/test/unit/paginationTemplateTests.js +++ b/test/unit/paginationTemplateTests.js @@ -3,20 +3,21 @@ import jsdom from 'jsdom' const path = '/check/results/test' -const paginationTests = (template, nunjucks) => { +const paginationTests = (template, nunjucks, opts = {}) => { describe('pagination', () => { + const hash = opts.hash ?? '' it('correctly renders the pagination', () => { const pagination = { nextPage: 4, previousPage: 2, items: [ - { number: 1, href: `${path}/0`, current: false }, + { number: 1, href: `${path}/0${hash}`, current: false }, { ellipsis: true, href: '#' }, - { number: 2, href: `${path}/1`, current: false }, - { number: 3, href: `${path}/2`, current: true }, - { number: 4, href: `${path}/3`, current: false }, + { number: 2, href: `${path}/1${hash}`, current: false }, + { number: 3, href: `${path}/2${hash}`, current: true }, + { number: 4, href: `${path}/3${hash}`, current: false }, { ellipsis: true, href: '#' }, - { number: 10, href: `${path}/9`, current: false } + { number: 10, href: `${path}/9${hash}`, current: false } ] } @@ -29,7 +30,7 @@ const paginationTests = (template, nunjucks) => { errors: {} } - testPagination({ template, params, nunjucks }) + testPagination({ template, params, nunjucks, opts }) }) it('correctly renders the pagination when viewing the first page', () => { @@ -54,7 +55,7 @@ const paginationTests = (template, nunjucks) => { errors: {} } - testPagination({ template, nunjucks, params }) + testPagination({ template, nunjucks, params, opts }) }) it('correctly renders the pagination when viewing the last page', () => { @@ -79,12 +80,17 @@ const paginationTests = (template, nunjucks) => { errors: {} } - testPagination({ template, nunjucks, params }) + testPagination({ template, nunjucks, params, opts }) }) }) } -const testPagination = ({ template, nunjucks, params }) => { +/** + * - `hash` option - sometimes the pagination links might include a hash component (e.g. when using tabs) + * + * @param {{ template: string, nunjucks: Object, params: Object, opts: { hash?: string }}} options + */ +const testPagination = ({ template, nunjucks, params, opts = {} }) => { const html = nunjucks.render(template, params).replace(/(\r\n|\n|\r)/gm, '').replace(/\t/gm, '').replace(/\s+/g, ' ') const { id, pagination } = params.options @@ -97,9 +103,11 @@ const testPagination = ({ template, nunjucks, params }) => { let currentPaginationChild = 0 + const hash = opts.hash ?? '' + // Previous link if (pagination.previousPage) { - expect(paginationChildren[currentPaginationChild].children[0].href).toEqual(`/check/results/${id}/${pagination.previousPage}`) + expect(paginationChildren[currentPaginationChild].children[0].href).toEqual(`/check/results/${id}/${pagination.previousPage}${hash}`) currentPaginationChild++ } @@ -107,24 +115,34 @@ const testPagination = ({ template, nunjucks, params }) => { const pageNumberNodes = paginationChildren[currentPaginationChild].children expect(pageNumberNodes.length).toEqual(pagination.items.length) - pagination.items.forEach((item, index) => { - if (item.ellipsis) { - expect(pageNumberNodes[index].textContent).toEqual(' ⋯ ') + const tuples = { actual: [], expected: [] } + for (let index = 0; index < pagination.items.length; ++index) { + const pItem = pagination.items[index] + const nItem = pageNumberNodes[index] + const actual = [] + const expected = [] + if (pItem.ellipsis) { + expected.push(' ⋯ ') + actual.push(nItem.textContent) } else { - expect(pageNumberNodes[index].children[0].href).toEqual(item.href) - expect(pageNumberNodes[index].children[0].textContent).toEqual(` ${item.number} `) - if (item.current) { - expect(pageNumberNodes[index].className).toContain('current') - } else { - expect(pageNumberNodes[index].className).not.toContain('current') - } + expected.push(pItem.href) + actual.push(nItem.children[0].href) + + expected.push(` ${pItem.number} `) + actual.push(nItem.children[0].textContent) + + actual.push(pItem.current && nItem.className.includes('current') ? 'contains "current"' : 'no "current"') + expected.push(pItem.current ? 'contains "current"' : 'no "current"') } - }) + tuples.actual.push(actual) + tuples.expected.push(expected) + } + expect(tuples.actual).toStrictEqual(tuples.expected) currentPaginationChild++ // next link if (pagination.nextPage) { - expect(paginationChildren[currentPaginationChild].children[0].href).toEqual(`/check/results/${id}/${pagination.nextPage}`) + expect(paginationChildren[currentPaginationChild].children[0].href).toEqual(`/check/results/${id}/${pagination.nextPage}${hash}`) currentPaginationChild++ }