From 9ca6356694d23da4f5d2a2cf2d7218ced0fbbb1b Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Mon, 24 Jun 2024 08:25:20 +0100 Subject: [PATCH] Refactor row lookup map --- .../src/components/app/GridBlock.svelte | 7 ++- .../grid/controls/BulkDeleteHandler.svelte | 2 +- .../controls/BulkDuplicationHandler.svelte | 3 +- .../grid/overlays/KeyboardManager.svelte | 2 +- .../src/components/grid/stores/clipboard.js | 6 +-- .../src/components/grid/stores/rows.js | 54 ++++++++----------- .../src/components/grid/stores/ui.js | 18 +++---- 7 files changed, 38 insertions(+), 54 deletions(-) diff --git a/packages/client/src/components/app/GridBlock.svelte b/packages/client/src/components/app/GridBlock.svelte index 7502dc1ba8..5e0f32a971 100644 --- a/packages/client/src/components/app/GridBlock.svelte +++ b/packages/client/src/components/app/GridBlock.svelte @@ -124,13 +124,12 @@ return readable([]) } return derived( - [gridContext.selectedRows, gridContext.rowLookupMap, gridContext.rows], - ([$selectedRows, $rowLookupMap, $rows]) => { + [gridContext.selectedRows, gridContext.rowLookupMap], + ([$selectedRows, $rowLookupMap]) => { return Object.entries($selectedRows || {}) .filter(([_, selected]) => selected) .map(([rowId]) => { - const idx = $rowLookupMap[rowId] - return gridContext.rows.actions.cleanRow($rows[idx]) + return gridContext.rows.actions.cleanRow($rowLookupMap[rowId]) }) } ) diff --git a/packages/frontend-core/src/components/grid/controls/BulkDeleteHandler.svelte b/packages/frontend-core/src/components/grid/controls/BulkDeleteHandler.svelte index 8cf4492682..fae2985861 100644 --- a/packages/frontend-core/src/components/grid/controls/BulkDeleteHandler.svelte +++ b/packages/frontend-core/src/components/grid/controls/BulkDeleteHandler.svelte @@ -24,7 +24,7 @@ let promptQuantity = 0 $: rowsToDelete = Object.keys($selectedRows) - .map(rowId => $rows[$rowLookupMap[rowId]]) + .map(rowId => $rowLookupMap[rowId]) .filter(x => x != null) const handleBulkDeleteRequest = () => { diff --git a/packages/frontend-core/src/components/grid/controls/BulkDuplicationHandler.svelte b/packages/frontend-core/src/components/grid/controls/BulkDuplicationHandler.svelte index 98435a0500..a662c3fdb2 100644 --- a/packages/frontend-core/src/components/grid/controls/BulkDuplicationHandler.svelte +++ b/packages/frontend-core/src/components/grid/controls/BulkDuplicationHandler.svelte @@ -11,6 +11,7 @@ selectedRowCount, allVisibleColumns, selectedCells, + rowLookupMap, } = getContext("grid") const duration = 260 @@ -26,7 +27,7 @@ // duplicate rows const rowsToDuplicate = Object.keys($selectedRows).map(id => { - return rows.actions.getRow(id) + return $rowLookupMap[id] }) const newRows = await rows.actions.bulkDuplicate( rowsToDuplicate, diff --git a/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte b/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte index 5c189b155b..81bf06e5d2 100644 --- a/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte +++ b/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte @@ -228,7 +228,7 @@ // Determine the new position for this cell const { rowId, field } = parseCellID(sourceCellId) - const rowIdx = $rowLookupMap[rowId] + const rowIdx = $rowLookupMap[rowId].__idx const newRow = $rows[rowIdx + delta] if (!newRow) { return diff --git a/packages/frontend-core/src/components/grid/stores/clipboard.js b/packages/frontend-core/src/components/grid/stores/clipboard.js index c1a2334708..3349604f40 100644 --- a/packages/frontend-core/src/components/grid/stores/clipboard.js +++ b/packages/frontend-core/src/components/grid/stores/clipboard.js @@ -81,7 +81,6 @@ export const createActions = context => { if (multiCellCopy) { const $rowLookupMap = get(rowLookupMap) const $rowChangeCache = get(rowChangeCache) - const $rows = get(rows) // Extract value of each selected cell, accounting for the change cache let value = [] @@ -89,9 +88,8 @@ export const createActions = context => { const rowValues = [] for (let cellId of row) { const { rowId, field } = parseCellID(cellId) - const rowIndex = $rowLookupMap[rowId] const row = { - ...$rows[rowIndex], + ...$rowLookupMap[rowId], ...$rowChangeCache[rowId], } rowValues.push(row[field]) @@ -159,7 +157,7 @@ export const createActions = context => { const { rowId, field } = parseCellID($focusedCellId) const $rowLookupMap = get(rowLookupMap) const $columnLookupMap = get(columnLookupMap) - const rowIdx = $rowLookupMap[rowId] + const rowIdx = $rowLookupMap[rowId].__idx const colIdx = $columnLookupMap[field] // Get limits of how many rows and columns we're able to paste into diff --git a/packages/frontend-core/src/components/grid/stores/rows.js b/packages/frontend-core/src/components/grid/stores/rows.js index f9ae6dc206..9590ac0748 100644 --- a/packages/frontend-core/src/components/grid/stores/rows.js +++ b/packages/frontend-core/src/components/grid/stores/rows.js @@ -17,11 +17,23 @@ export const createStores = () => { const error = writable(null) const fetch = writable(null) + // Enrich rows with an index property and any pending changes + const enrichedRows = derived( + [rows, rowChangeCache], + ([$rows, $rowChangeCache]) => { + return $rows.map((row, idx) => ({ + ...row, + ...$rowChangeCache[row._id], + __idx: idx, + })) + } + ) + // Generate a lookup map to quick find a row by ID - const rowLookupMap = derived(rows, $rows => { + const rowLookupMap = derived(enrichedRows, $enrichedRows => { let map = {} - for (let i = 0; i < $rows.length; i++) { - map[$rows[i]._id] = i + for (let i = 0; i < $enrichedRows.length; i++) { + map[$enrichedRows[i]._id] = $enrichedRows[i] } return map }) @@ -36,18 +48,6 @@ export const createStores = () => { } }) - // Enrich rows with an index property and any pending changes - const enrichedRows = derived( - [rows, rowChangeCache], - ([$rows, $rowChangeCache]) => { - return $rows.map((row, idx) => ({ - ...row, - ...$rowChangeCache[row._id], - __idx: idx, - })) - } - ) - return { rows: { ...rows, @@ -189,12 +189,6 @@ export const createActions = context => { fetch.set(newFetch) }) - // Gets a row by ID - const getRow = id => { - const index = get(rowLookupMap)[id] - return index >= 0 ? get(rows)[index] : null - } - // Handles validation errors from the rows API and updates local validation // state, storing error messages against relevant cells const handleValidationError = (rowId, error) => { @@ -323,7 +317,8 @@ export const createActions = context => { const bulkDuplicate = async (rowsToDupe, progressCallback) => { // Find index of last row const $rowLookupMap = get(rowLookupMap) - const index = Math.max(...rowsToDupe.map(row => $rowLookupMap[row._id])) + const indices = rowsToDupe.map(row => $rowLookupMap[row._id]?.__idx) + const index = Math.max(...indices) const count = rowsToDupe.length // Clone and clean rows @@ -371,7 +366,7 @@ export const createActions = context => { // Get index of row to check if it exists const $rows = get(rows) const $rowLookupMap = get(rowLookupMap) - const index = $rowLookupMap[id] + const index = $rowLookupMap[id].__idx // Process as either an update, addition or deletion if (row) { @@ -420,10 +415,8 @@ export const createActions = context => { // Patches a row with some changes in local state, and returns whether a // valid pending change was made or not const stashRowChanges = (rowId, changes) => { - const $rows = get(rows) const $rowLookupMap = get(rowLookupMap) - const index = $rowLookupMap[rowId] - const row = $rows[index] + const row = $rowLookupMap[rowId] // Check this is a valid change if (!row || !changesAreValid(row, changes)) { @@ -449,10 +442,8 @@ export const createActions = context => { updateState = true, handleErrors = true, }) => { - const $rows = get(rows) const $rowLookupMap = get(rowLookupMap) - const index = $rowLookupMap[rowId] - const row = $rows[index] + const row = $rowLookupMap[rowId] if (row == null) { return } @@ -479,7 +470,7 @@ export const createActions = context => { if (savedRow?._id) { if (updateState) { rows.update(state => { - state[index] = savedRow + state[row.__idx] = savedRow return state.slice() }) } @@ -562,7 +553,7 @@ export const createActions = context => { const $rowLookupMap = get(rowLookupMap) rows.update(state => { for (let row of updated) { - const index = $rowLookupMap[row._id] + const index = $rowLookupMap[row._id].__idx state[index] = row } return state.slice() @@ -664,7 +655,6 @@ export const createActions = context => { addRow, duplicateRow, bulkDuplicate, - getRow, updateValue, applyRowChanges, deleteRows, diff --git a/packages/frontend-core/src/components/grid/stores/ui.js b/packages/frontend-core/src/components/grid/stores/ui.js index 3b9bc660b7..af27ab6442 100644 --- a/packages/frontend-core/src/components/grid/stores/ui.js +++ b/packages/frontend-core/src/components/grid/stores/ui.js @@ -65,16 +65,12 @@ export const deriveStores = context => { // Derive the row that contains the selected cell const focusedRow = derived( - [focusedRowId, rowLookupMap, rows], - ([$focusedRowId, $rowLookupMap, $rows]) => { - // Edge case for new rows + [focusedRowId, rowLookupMap], + ([$focusedRowId, $rowLookupMap]) => { if ($focusedRowId === NewRowID) { return { _id: NewRowID } } - - // All normal rows - const index = $rowLookupMap[$focusedRowId] - return $rows[index] + return $rowLookupMap[$focusedRowId] } ) @@ -119,8 +115,8 @@ export const deriveStores = context => { const targetInfo = parseCellID(targetCellId) // Row indices - const sourceRowIndex = $rowLookupMap[sourceInfo.rowId] - const targetRowIndex = $rowLookupMap[targetInfo.rowId] + const sourceRowIndex = $rowLookupMap[sourceInfo.rowId].__idx + const targetRowIndex = $rowLookupMap[targetInfo.rowId].__idx const lowerRowIndex = Math.min(sourceRowIndex, targetRowIndex) const upperRowIndex = Math.max(sourceRowIndex, targetRowIndex) @@ -206,7 +202,7 @@ export const createActions = context => { if (!newState[id]) { delete newState[id] } else { - lastSelectedIndex = get(rowLookupMap)[id] + lastSelectedIndex = get(rowLookupMap)[id].__idx } return newState }) @@ -220,7 +216,7 @@ export const createActions = context => { if (lastSelectedIndex == null) { return } - const thisIndex = get(rowLookupMap)[id] + const thisIndex = get(rowLookupMap)[id].__idx // Skip if indices are the same if (lastSelectedIndex === thisIndex) {