Add proper validation for nesting illegal combinations of components

packages/builder/src/builderStore/store/frontend.js

@@ -182,7 +182,70 @@ export const getFrontendStore = () => {
           return state
         })
       },
+      validate: screen => {
+        // Recursive function to find any illegal children in component trees
+        const findIllegalChild = (
+          component,
+          illegalChildren = [],
+          legalDirectChildren = []
+        ) => {
+          const type = component._component
+          if (illegalChildren.includes(type)) {
+            return type
+          }
+          if (
+            legalDirectChildren.length &&
+            !legalDirectChildren.includes(type)
+          ) {
+            return type
+          }
+          if (!component?._children?.length) {
+            return
+          }
+
+          const definition = store.actions.components.getDefinition(
+            component._component
+          )
+
+          // Reset whitelist for direct children
+          legalDirectChildren = []
+          if (definition?.legalDirectChildren?.length) {
+            legalDirectChildren = definition.legalDirectChildren.map(x => {
+              return `@budibase/standard-components/${x}`
+            })
+          }
+
+          // Append blacklisted components and remove duplicates
+          if (definition?.illegalChildren?.length) {
+            const blacklist = definition.illegalChildren.map(x => {
+              return `@budibase/standard-components/${x}`
+            })
+            illegalChildren = [...new Set([...illegalChildren, ...blacklist])]
+          }
+
+          // Recurse on all children
+          for (let child of component._children) {
+            const illegalChild = findIllegalChild(
+              child,
+              illegalChildren,
+              legalDirectChildren
+            )
+            if (illegalChild) {
+              return illegalChild
+            }
+          }
+        }
+
+        // Validate the entire tree and throw and error if an illegal child is
+        // found anywhere
+        const illegalChild = findIllegalChild(screen.props)
+        if (illegalChild) {
+          const def = store.actions.components.getDefinition(illegalChild)
+          throw `A ${def.name} can't be inserted here`
+        }
+      },
       save: async screen => {
+        store.actions.screens.validate(screen)
         const state = get(store)
         const creatingNewScreen = screen._id === undefined
         const savedScreen = await API.saveScreen(screen)
@@ -624,16 +687,24 @@ export const getFrontendStore = () => {
         }
         let newComponentId
 
+        // Remove copied component if cutting, regardless if pasting works
+        let componentToPaste = cloneDeep(state.componentToPaste)
+        if (componentToPaste.isCut) {
+          store.update(state => {
+            delete state.componentToPaste
+            return state
+          })
+        }
+
         // Patch screen
         const patch = screen => {
           // Get up to date ref to target
           targetComponent = findComponent(screen.props, targetComponent._id)
           if (!targetComponent) {
-            return
+            return false
           }
-          const cut = state.componentToPaste.isCut
+          const cut = componentToPaste.isCut
-          const originalId = state.componentToPaste._id
+          const originalId = componentToPaste._id
-          let componentToPaste = cloneDeep(state.componentToPaste)
           delete componentToPaste.isCut
 
           // Make new component unique if copying
@@ -688,11 +759,8 @@ export const getFrontendStore = () => {
         const targetScreenId = targetScreen?._id || state.selectedScreenId
         await store.actions.screens.patch(patch, targetScreenId)
 
+        // Select the new component
         store.update(state => {
-          // Remove copied component if cutting
-          if (state.componentToPaste.isCut) {
-            delete state.componentToPaste
-          }
           state.selectedScreenId = targetScreenId
           state.selectedComponentId = newComponentId
           return state

packages/builder/src/pages/builder/app/[application]/design/[screenId]/_components/AppPreview.svelte

@@ -222,8 +222,7 @@
         console.warn(`Client sent unknown event type: ${type}`)
       }
     } catch (error) {
-      console.warn(error)
+      notifications.error(error || "Error handling event from app preview")
-      notifications.error("Error handling event from app preview")
     }
   }
 

packages/builder/src/pages/builder/app/[application]/design/[screenId]/components/[componentId]/_components/navigation/ComponentKeyHandler.svelte

@@ -80,10 +80,9 @@
         event.preventDefault()
         event.stopPropagation()
       }
-      return handler(component)
+      return await handler(component)
     } catch (error) {
-      console.error(error)
+      notifications.error(error || "Error handling key press")
-      notifications.error("Error handling key press")
     }
   }
 

packages/builder/src/pages/builder/app/[application]/design/[screenId]/components/[componentId]/_components/navigation/ComponentTree.svelte

@@ -70,34 +70,12 @@
     closedNodes = closedNodes
   }
 
-  const onDrop = async (e, component) => {
+  const onDrop = async e => {
     e.stopPropagation()
     try {
-      const compDef = store.actions.components.getDefinition(
-        $dndStore.source?._component
-      )
-      if (!compDef) {
-        return
-      }
-      const compTypeName = compDef.name.toLowerCase()
-      const path = findComponentPath(currentScreen.props, component._id)
-
-      for (let pathComp of path) {
-        const pathCompDef = store.actions.components.getDefinition(
-          pathComp?._component
-        )
-        if (pathCompDef?.illegalChildren?.indexOf(compTypeName) > -1) {
-          notifications.warning(
-            `${compDef.name} cannot be a child of ${pathCompDef.name} (${pathComp._instanceName})`
-          )
-          return
-        }
-      }
-
       await dndStore.actions.drop()
     } catch (error) {
-      console.error(error)
+      notifications.error(error || "Error saving component")
-      notifications.error("Error saving component")
     }
   }
 
@@ -137,9 +115,7 @@
         on:dragstart={() => dndStore.actions.dragstart(component)}
         on:dragover={dragover(component, index)}
         on:iconClick={() => toggleNodeOpen(component._id)}
-        on:drop={e => {
+        on:drop={onDrop}
-          onDrop(e, component)
-        }}
         text={getComponentText(component)}
         icon={getComponentIcon(component)}
         withArrow={componentHasChildren(component)}

packages/builder/src/pages/builder/app/[application]/design/[screenId]/components/[componentId]/new/_components/NewComponentPanel.svelte

@@ -138,7 +138,7 @@
       await store.actions.components.create(component)
       $goto("../")
     } catch (error) {
-      notifications.error("Error creating component")
+      notifications.error(error || "Error creating component")
     }
   }
 

packages/client/manifest.json

@@ -4573,8 +4573,14 @@
     "styles": [
       "size"
     ],
-    "illegalChildren": ["grid", "section"],
+    "illegalChildren": ["section", "grid"],
-    "allowedDirectChildren": [""],
+    "legalDirectChildren": [
+      "container",
+      "tableblock",
+      "cardsblock",
+      "repeaterblock",
+      "formblock"
+    ],
     "size": {
       "width": 800,
       "height": 400

packages/client/src/components/Component.svelte

@@ -477,10 +477,10 @@
       let columns = 6
       let rows = 4
       if (definition.size?.width) {
-        columns = Math.round(definition.size.width / 100)
+        columns = Math.min(12, Math.round(definition.size.width / 100))
       }
       if (definition.size?.height) {
-        rows = Math.round(definition.size.height / 100)
+        rows = Math.min(12, Math.round(definition.size.height / 50))
       }
 
       // Ensure grid position styles are set

packages/client/src/components/preview/GridDNDHandler.svelte

@@ -12,7 +12,8 @@
 
   // We don't clear grid styles because that causes flashing, as the component
   // will revert to its original position until the save completes.
-  $: gridStyles && instance?.setEphemeralStyles(gridStyles)
+  $: gridStyles &&
+    instance?.setEphemeralStyles({ ...gridStyles, "z-index": 999 })
 
   const isChildOfGrid = e => {
     return (

packages/frontend-core/src/utils/utils.js

@@ -6,17 +6,29 @@
  */
 export const sequential = fn => {
   let queue = []
-  return async (...params) => {
+  return (...params) => {
-    queue.push(async () => {
+    return new Promise((resolve, reject) => {
-      await fn(...params)
+      queue.push(async () => {
-      queue.shift()
+        let data, error
-      if (queue.length) {
+        try {
-        await queue[0]()
+          data = await fn(...params)
+        } catch (err) {
+          error = err
+        }
+        queue.shift()
+        if (queue.length) {
+          queue[0]()
+        }
+        if (error) {
+          reject(error)
+        } else {
+          resolve(data)
+        }
+      })
+      if (queue.length === 1) {
+        queue[0]()
       }
     })
-    if (queue.length === 1) {
-      await queue[0]()
-    }
   }
 }