From acecea570499d26ddd453157d6abedfc86a43dff Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Thu, 29 Feb 2024 10:30:38 +0000 Subject: [PATCH] Refactor grid row actions to be more explicit and remove extraneous flags --- .../src/components/grid/cells/DataCell.svelte | 4 +- .../grid/overlays/KeyboardManager.svelte | 4 +- .../src/components/grid/stores/rows.js | 100 +++++++++--------- 3 files changed, 54 insertions(+), 54 deletions(-) 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 f4b0e97942..c8d27da2e7 100644 --- a/packages/frontend-core/src/components/grid/stores/rows.js +++ b/packages/frontend-core/src/components/grid/stores/rows.js @@ -3,6 +3,7 @@ import { fetchData } from "../../../fetch" import { NewRowID, RowPageSize } from "../lib/constants" import { tick } from "svelte" import { Helpers } from "@budibase/bbui" +import { isValid } from "@budibase/string-templates" export const createStores = () => { const rows = writable([]) @@ -327,38 +328,31 @@ export const createActions = context => { get(fetch)?.getInitialData() } - // Patches a row with some changes - const updateRow = async ( - rowId, - changes, - options = { save: true, force: false } - ) => { + // 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) { - return - } - if (!options?.force && !Object.keys(changes || {}).length) { - return + + // Check this is a valid change + if (!row || !changesAreValid(row, changes)) { + return false } - // Abandon if no changes - if (!options?.force) { - 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]: { @@ -366,26 +360,30 @@ export const createActions = context => { ...changes, }, })) + return true + } - // Stop here if we don't want to persist the change - if (!options?.save && !options?.force) { + // 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 @@ -395,6 +393,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 @@ -402,15 +402,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 @@ -420,9 +422,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 @@ -442,7 +442,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() @@ -503,7 +503,7 @@ export const createActions = context => { duplicateRow, getRow, updateValue, - updateRow, + applyRowChanges, deleteRows, hasRow, loadNextPage, @@ -537,13 +537,13 @@ export const initialise = context => { }) // Ensure any unsaved changes are saved when changing cell - previousFocusedCellId.subscribe(id => { + 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) { - rows.actions.updateRow(rowId, null, { force: true }) + await rows.actions.applyRowChanges(rowId) } }) }