diff --git a/packages/frontend-core/src/components/grid/cells/DataCell.svelte b/packages/frontend-core/src/components/grid/cells/DataCell.svelte index cdaf28978a..d8cff26b9d 100644 --- a/packages/frontend-core/src/components/grid/cells/DataCell.svelte +++ b/packages/frontend-core/src/components/grid/cells/DataCell.svelte @@ -59,13 +59,13 @@ isReadonly: () => readonly, getType: () => column.schema.type, getValue: () => row[column.name], - setValue: (value, options = { save: true }) => { + setValue: (value, options = { apply: true }) => { validation.actions.setError(cellId, null) updateValue({ rowId: row._id, column: column.name, value, - save: options?.save, + apply: options?.apply, }) }, } diff --git a/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte b/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte index 8b0a0f0942..5e3a035d89 100644 --- a/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte +++ b/packages/frontend-core/src/components/grid/overlays/KeyboardManager.svelte @@ -217,14 +217,14 @@ const type = $focusedCellAPI.getType() if (type === "number" && keyCodeIsNumber(keyCode)) { // Update the value locally but don't save it yet - $focusedCellAPI.setValue(parseInt(key), { save: false }) + $focusedCellAPI.setValue(parseInt(key), { apply: false }) $focusedCellAPI.focus() } else if ( ["string", "barcodeqr", "longform"].includes(type) && (keyCodeIsLetter(keyCode) || keyCodeIsNumber(keyCode)) ) { // Update the value locally but don't save it yet - $focusedCellAPI.setValue(key, { save: false }) + $focusedCellAPI.setValue(key, { apply: false }) $focusedCellAPI.focus() } } diff --git a/packages/frontend-core/src/components/grid/stores/rows.js b/packages/frontend-core/src/components/grid/stores/rows.js index 34ba77afa2..5dc9413ccd 100644 --- a/packages/frontend-core/src/components/grid/stores/rows.js +++ b/packages/frontend-core/src/components/grid/stores/rows.js @@ -327,29 +327,31 @@ export const createActions = context => { get(fetch)?.getInitialData() } - // Patches a row with some changes - const updateRow = async (rowId, changes, options = { save: true }) => { + // Checks if a changeset for a row actually mutates the row or not + const changesAreValid = (row, changes) => { + const columns = Object.keys(changes || {}) + if (!row || !columns.length) { + return false + } + + // Ensure there is at least 1 column that creates a difference + return columns.some(column => row[column] !== changes[column]) + } + + // 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] - if (index == null || !Object.keys(changes || {}).length) { - return + + // Check this is a valid change + if (!row || !changesAreValid(row, changes)) { + return false } - // Abandon if no changes - let same = true - for (let column of Object.keys(changes)) { - if (row[column] !== changes[column]) { - same = false - break - } - } - if (same) { - return - } - - // Immediately update state so that the change is reflected + // Add change to cache rowChangeCache.update(state => ({ ...state, [rowId]: { @@ -357,26 +359,30 @@ export const createActions = context => { ...changes, }, })) + return true + } - // Stop here if we don't want to persist the change - if (!options?.save) { + // Saves any pending changes to a row + const applyRowChanges = async rowId => { + const $rows = get(rows) + const $rowLookupMap = get(rowLookupMap) + const index = $rowLookupMap[rowId] + const row = $rows[index] + if (row == null) { return } // Save change try { - inProgressChanges.update(state => ({ - ...state, - [rowId]: true, - })) + // Mark as in progress + inProgressChanges.update(state => ({ ...state, [rowId]: true })) // Update row - const saved = await datasource.actions.updateRow({ - ...cleanRow(row), - ...get(rowChangeCache)[rowId], - }) + const changes = get(rowChangeCache)[rowId] + const newRow = { ...cleanRow(row), ...changes } + const saved = await datasource.actions.updateRow(newRow) - // Update state after a successful change + // Update row state after a successful change if (saved?._id) { rows.update(state => { state[index] = saved @@ -386,6 +392,8 @@ export const createActions = context => { // Handle users table edge case await refreshRow(saved.id) } + + // Wipe row change cache now that we've saved the row rowChangeCache.update(state => { delete state[rowId] return state @@ -393,15 +401,17 @@ export const createActions = context => { } catch (error) { handleValidationError(rowId, error) } - inProgressChanges.update(state => ({ - ...state, - [rowId]: false, - })) + + // Mark as completed + inProgressChanges.update(state => ({ ...state, [rowId]: false })) } // Updates a value of a row - const updateValue = async ({ rowId, column, value, save = true }) => { - return await updateRow(rowId, { [column]: value }, { save }) + const updateValue = async ({ rowId, column, value, apply = true }) => { + const success = stashRowChanges(rowId, { [column]: value }) + if (success && apply) { + await applyRowChanges(rowId) + } } // Deletes an array of rows @@ -411,9 +421,7 @@ export const createActions = context => { } // Actually delete rows - rowsToDelete.forEach(row => { - delete row.__idx - }) + rowsToDelete.forEach(row => delete row.__idx) await datasource.actions.deleteRows(rowsToDelete) // Update state @@ -433,7 +441,7 @@ export const createActions = context => { newRow = newRows[i] // Ensure we have a unique _id. - // This means generating one for non DS+, overriting any that may already + // This means generating one for non DS+, overwriting any that may already // exist as we cannot allow duplicates. if (!$isDatasourcePlus) { newRow._id = Helpers.uuid() @@ -494,7 +502,7 @@ export const createActions = context => { duplicateRow, getRow, updateValue, - updateRow, + applyRowChanges, deleteRows, hasRow, loadNextPage, @@ -508,7 +516,14 @@ export const createActions = context => { } export const initialise = context => { - const { rowChangeCache, inProgressChanges, previousFocusedRowId } = context + const { + rowChangeCache, + inProgressChanges, + previousFocusedRowId, + previousFocusedCellId, + rows, + validation, + } = context // Wipe the row change cache when changing row previousFocusedRowId.subscribe(id => { @@ -519,4 +534,15 @@ export const initialise = context => { }) } }) + + // Ensure any unsaved changes are saved when changing cell + previousFocusedCellId.subscribe(async id => { + const rowId = id?.split("-")[0] + const hasErrors = validation.actions.rowHasErrors(rowId) + const hasChanges = Object.keys(get(rowChangeCache)[rowId] || {}).length > 0 + const isSavingChanges = get(inProgressChanges)[rowId] + if (rowId && !hasErrors && hasChanges && !isSavingChanges) { + await rows.actions.applyRowChanges(rowId) + } + }) } diff --git a/packages/frontend-core/src/components/grid/stores/ui.js b/packages/frontend-core/src/components/grid/stores/ui.js index 129d6614e5..da0558bb5b 100644 --- a/packages/frontend-core/src/components/grid/stores/ui.js +++ b/packages/frontend-core/src/components/grid/stores/ui.js @@ -16,6 +16,7 @@ export const createStores = context => { const hoveredRowId = writable(null) const rowHeight = writable(get(props).fixedRowHeight || DefaultRowHeight) const previousFocusedRowId = writable(null) + const previousFocusedCellId = writable(null) const gridFocused = writable(false) const isDragging = writable(false) const buttonColumnWidth = writable(0) @@ -48,6 +49,7 @@ export const createStores = context => { focusedCellAPI, focusedRowId, previousFocusedRowId, + previousFocusedCellId, hoveredRowId, rowHeight, gridFocused, @@ -129,6 +131,7 @@ export const initialise = context => { const { focusedRowId, previousFocusedRowId, + previousFocusedCellId, rows, focusedCellId, selectedRows, @@ -181,6 +184,13 @@ export const initialise = context => { lastFocusedRowId = id }) + // Remember the last focused cell ID so that we can store the previous one + let lastFocusedCellId = null + focusedCellId.subscribe(id => { + previousFocusedCellId.set(lastFocusedCellId) + lastFocusedCellId = id + }) + // Remove hovered row when a cell is selected focusedCellId.subscribe(cell => { if (cell && get(hoveredRowId)) { diff --git a/packages/frontend-core/src/components/grid/stores/validation.js b/packages/frontend-core/src/components/grid/stores/validation.js index 9c3927f9c9..70db076593 100644 --- a/packages/frontend-core/src/components/grid/stores/validation.js +++ b/packages/frontend-core/src/components/grid/stores/validation.js @@ -1,8 +1,23 @@ -import { writable, get } from "svelte/store" +import { writable, get, derived } from "svelte/store" +// Normally we would break out actions into the explicit "createActions" +// function, but for validation all these actions are pure so can go into +// "createStores" instead to make dependency ordering simpler export const createStores = () => { const validation = writable({}) + // Derive which rows have errors so that we can use that info later + const rowErrorMap = derived(validation, $validation => { + let map = {} + Object.entries($validation).forEach(([key, error]) => { + // Extract row ID from all errored cell IDs + if (error) { + map[key.split("-")[0]] = true + } + }) + return map + }) + const setError = (cellId, error) => { if (!cellId) { return @@ -13,11 +28,16 @@ export const createStores = () => { })) } + const rowHasErrors = rowId => { + return get(rowErrorMap)[rowId] + } + return { validation: { ...validation, actions: { setError, + rowHasErrors, }, }, }