Merge pull request #13151 from Budibase/fix-grid-single-char-changes-v2

Fix single-character changes in grids not being saved

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,
       })
     },
   }

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()
       }
     }

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
+  // Checks if a changeset for a row actually mutates the row or not
-  const updateRow = async (rowId, changes, options = { save: true }) => {
+  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
+    // Add change to cache
-    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
     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
+  // Saves any pending changes to a row
-    if (!options?.save) {
+  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 => ({
+      // Mark as in progress
-        ...state,
+      inProgressChanges.update(state => ({ ...state, [rowId]: true }))
-        [rowId]: true,
-      }))
 
       // Update row
-      const saved = await datasource.actions.updateRow({
+      const changes = get(rowChangeCache)[rowId]
-        ...cleanRow(row),
+      const newRow = { ...cleanRow(row), ...changes }
-        ...get(rowChangeCache)[rowId],
+      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,
+    // Mark as completed
-      [rowId]: false,
+    inProgressChanges.update(state => ({ ...state, [rowId]: false }))
-    }))
   }
 
   // Updates a value of a row
-  const updateValue = async ({ rowId, column, value, save = true }) => {
+  const updateValue = async ({ rowId, column, value, apply = true }) => {
-    return await updateRow(rowId, { [column]: value }, { save })
+    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 => {
+    rowsToDelete.forEach(row => delete row.__idx)
-      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)
+    }
+  })
 }

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)) {

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,
       },
     },
   }