Merged
Conversation
…main (#317) * Update backend.js Updating for tables * Added for table testing * Some minor updates to testing * Some updates to tables_test and added a metadata file * Installed our locally "vendored" patched copy of sheet.js and used it to get defined tables Excel.js only offered an async API to load files, so we couldn't use it :'-( # Conflicts: # lib/importer/package.json # lib/importer/src/dudk/backend.js * Licence compliance requirements for SheetJS fork * Added html and nkj files, as well as the associated functions in function.js and sheet.js for selecting user defined tables. * Added njk and html files as well as functions in functions.js and sheets.js to select and preview tables. * A range of edits to fix errros that campe up when testing the table selector and to extract the header row automatically * Several tweaks to code to get table previews to work. * Add files via upload * Delete backend.js * Delete sheets.js * Delete functions.js * Add files via upload * Add files via upload * Add files via upload * Add files via upload --------- Co-authored-by: Alaric Snell-Pym <alaric@register-dynamics.co.uk>
…ctly, and to ensure we preserve the new header range.
…er functions to accomodate tables into the header and footer pages
moved the check for footer rows in SessionPerformMappingJob from if(row) to its parent for loop, as we were having an issue where an empty row in the table footer (i.e. the row immediately below the table) was giving an error.
…pData step so that we can import data without visible headers
…amics/data-import into user-defined-tables
alaric-rd
approved these changes
Oct 28, 2025
Contributor
alaric-rd
left a comment
alaric-rd
left a comment
There was a problem hiding this comment.
Some feedback, but it's all just maybes!
| {% macro importerTableSelector(data, legend) %} | ||
| {% set selectedTable = data['importer.session'].table %} | ||
| {% set tables = importTablePreview(data) %} | ||
| {% set tableRowIndex = tables.length + 2 %} |
Contributor
There was a problem hiding this comment.
I'd pop a comment here saying why it's +2 specifically
| const rowRange = exports.GetRowRangeFromEnd(sid, sheet, count) | ||
| const rowRange = exports.GetRowRangeFromEnd(sid, sheet, count); | ||
| const wantedRows = Math.min(count, rowRange.end.row - hRange.end.row); | ||
| return backend.SessionGetInputSampleRows(sid, rowRange, 0, 0, wantedRows)[2]; |
Contributor
There was a problem hiding this comment.
Another magic number 2 that probably wants explaining in a comment :-)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.