Skip to content

Commit c6e4fcd

Browse files
rossjonesrossjones
authored
Use table view for range selection (#291)
The range_selector macro did not use the tableView and therefore there was a duplication of code, and an error where an extra tbody was included in tables with no header chrome. This commit removes the range_selector in favour of the tableView, and simplifies the header/footer selection step.
1 parent 865ae37 commit c6e4fcd

10 files changed

Lines changed: 43 additions & 96 deletions

File tree

‎lib/importer/assets/js/selectable_table.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ window.addEventListener("load", function() {
721721

722722
const headerRowCellCount = table.querySelectorAll('th[scope="col"]').length
723723
const headerRowCount = table.querySelectorAll('thead tr').length
724-
const headerRowOffset = headerRowCellCount > 0 ? headerRowCount : 1;
724+
const headerRowOffset = headerRowCellCount > 0 ? headerRowCount : 0;
725725
var columnOffset = 0;
726726

727727
// If there are row numbers, the column numbers will be off as it includes the

‎lib/importer/govuk-prototype-kit.config.json‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
"stylesheets": [
5555
"/assets/css/error.css",
5656
"/assets/css/sheet_selector.css",
57-
"/assets/css/range_selector.css",
5857
"/assets/css/selectable_table.css"
5958
],
6059
"nunjucksPaths": [
@@ -81,10 +80,6 @@
8180
"macroName": "importerFooterSelector",
8281
"importFrom": "importer/macros/footer_selector.njk"
8382
},
84-
{
85-
"macroName": "importerRangeSelector",
86-
"importFrom": "importer/macros/range_selector.njk"
87-
},
8883
{
8984
"macroName": "tableView",
9085
"importFrom": "importer/macros/table_view.njk"

‎lib/importer/nunjucks/importer/macros/field_mapper.njk‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,4 @@
8181
</tbody>
8282
</table>
8383

84-
85-
86-
8784
{% endmacro %}

‎lib/importer/nunjucks/importer/macros/footer_selector.njk‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
1-
2-
{% from "importer/macros/range_selector.njk" import importerRangeSelector %}
1+
{% from "importer/macros/table_view.njk" import importerTableView %}
32

43
{% macro importerFooterSelector(data, count=10) %}
54
{% set rows = importerGetTrailingRows(data, count) %}
65
{% set caption = importerGetTableCaption(data, "Last", count) %}
76

8-
{{ importerRangeSelector(rows, caption, false, true) }}
7+
<div>
8+
<input type="hidden" name="importer:selection:TLRow" id="importer:selection:TLRow"/>
9+
<input type="hidden" name="importer:selection:TLCol" id="importer:selection:TLCol"/>
10+
<input type="hidden" name="importer:selection:BRRow" id="importer:selection:BRRow"/>
11+
<input type="hidden" name="importer:selection:BRCol" id="importer:selection:BRCol"/>
12+
</div>
13+
14+
<div class="rd-range-selector">
15+
{{ importerTableView(data, rows, caption=caption, headerMode="none") }}
16+
</div>
17+
918
{% endmacro %}

‎lib/importer/nunjucks/importer/macros/header_selector.njk‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
11

2-
{% from "importer/macros/range_selector.njk" import importerRangeSelector %}
2+
{% from "importer/macros/table_view.njk" import importerTableView %}
33

44
{% macro importerHeaderSelector(data, start, count=10) %}
55
{% set rows = importerGetRows(data, start, count) %}
66
{% set caption = importerGetTableCaption(data, "First", count) %}
77

8-
{{ importerRangeSelector(rows, caption, false, true) }}
8+
<div>
9+
<input type="hidden" name="importer:selection:TLRow" id="importer:selection:TLRow"/>
10+
<input type="hidden" name="importer:selection:TLCol" id="importer:selection:TLCol"/>
11+
<input type="hidden" name="importer:selection:BRRow" id="importer:selection:BRRow"/>
12+
<input type="hidden" name="importer:selection:BRCol" id="importer:selection:BRCol"/>
13+
</div>
14+
15+
<div class="rd-range-selector">
16+
{{ importerTableView(data, rows, caption=caption, headerMode="none") }}
17+
</div>
918
{% endmacro %}

‎lib/importer/nunjucks/importer/macros/range_selector.njk‎

Lines changed: 0 additions & 61 deletions
This file was deleted.

‎lib/importer/nunjucks/importer/macros/sheet_selector.njk‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,15 @@
5353

5454
<div class="rd-sheet-selector-previews">
5555
{% for sheet in sheets %}
56+
{% set sheetData = sheet.data %}
5657
<div class="hidden">
57-
{% if sheet.data.rows == null %}
58+
{% if sheetData .rows == null %}
5859
<div class="govuk-body">
5960
Sheet '{{ sheet.name }}' is empty
6061
</div>
6162
{% else %}
6263
{% set caption = importerGetTableCaption(data, "First", 10, sheet.name) %}
63-
{{ importerTableView(data, sheet.data, caption=caption, headerMode="none") }}
64+
{{ importerTableView(data, sheetData.rows, caption=caption, headerMode="none") }}
6465
{% endif %}
6566
</div>
6667
{% endfor %}

‎lib/importer/nunjucks/importer/macros/table_view.njk‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11

2-
{% macro importerTableView(session, data, caption=false, headerMode="none" ) %}
3-
{% set moreRowsCount = data.extraRecordCount %}
2+
{% macro importerTableView(session, rows, caption=false, headerMode="none", moreRowsCount=0 ) %}
43
{% set headers = importerHeaderRowDisplay(session, headerMode) %}
54

65
{{
@@ -9,10 +8,11 @@
98
showHeaders: true,
109
headers: headers,
1110
showRowNumbers: true,
12-
rows: data.rows,
11+
rows: rows,
1312
moreRowsCount: moreRowsCount
1413
} )
1514
}}
15+
1616
{% endmacro %}
1717

1818
{#
@@ -51,18 +51,18 @@ Renders a table view using the provided parameters to determine what is being sh
5151
<th scope="row" class="rowIndex">{{ rowObj.index }}</th>
5252
{% endif %}
5353
{% for cell in rowObj.row %}
54-
<td
55-
{% if cell.colspan %}
56-
colspan="{{ cell.colspan }}"
57-
{% endif %}
58-
{% if cell.rowspan %}
59-
rowspan="{{ cell.rowspan }}"
60-
{% endif %}
54+
<td
55+
{% if cell.colspan %}
56+
colspan="{{ cell.colspan }}"
57+
{% endif %}
58+
{% if cell.rowspan %}
59+
rowspan="{{ cell.rowspan }}"
60+
{% endif %}
6161

62-
{% if cell.error %}
63-
class="validation-error"
64-
{% endif %}
65-
>{{cell.value }}</td>
62+
{% if cell.error %}
63+
class="validation-error"
64+
{% endif %}
65+
>{{cell.value }}</td>
6666
{% endfor %}
6767
</tr>
6868
{% endfor %}

‎lib/importer/src/functions.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ const importerGetRows = (data, start, count) => {
6969
const importerGetTrailingRows = (data, count) => {
7070
const session_data = data[IMPORTER_SESSION_KEY];
7171
const session = new session_lib.Session(session_data)
72-
7372
return sheets_lib.GetTrailingRows(session.backendSid, session.sheet, count);
7473
}
7574

@@ -246,7 +245,7 @@ const parseNumberFromString = (s) => {
246245
}
247246

248247
const importerRowHasHeader = (rowObj) => {
249-
return rowObj.row.some( (element) => element.error )
248+
return rowObj.row.some( (element) => element && element.error )
250249
}
251250

252251

‎lib/importer/src/index.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,15 +231,14 @@ exports.Initialise = (config, router, prototypeKit) => {
231231
response.status(404);
232232
return;
233233
}
234-
235234
const session = new session_lib.Session(session_data)
236-
237235
// Find the selected range, or default to the first row (0,0)->(0,maxCol)
238236
session.headerRange = getSelectionFromRequest(
239237
request,
240238
session,
241239
/* optional */ false,
242240
);
241+
243242
// Ensure the session is persisted. Currently in session, eventually another way
244243
request.session.data[IMPORTER_SESSION_KEY] = session;
245244

@@ -261,7 +260,6 @@ exports.Initialise = (config, router, prototypeKit) => {
261260
}
262261

263262
const session = new session_lib.Session(session_data)
264-
265263
let selectionRange = getSelectionFromRequest(
266264
request,
267265
session,

0 commit comments

Comments
 (0)