Update grid layout action to provide explicit values for all variables and simplify inheritance logic

packages/client/src/components/app/container/GridContainer.svelte

@@ -53,42 +53,40 @@
 
   onMount(() => {
     let observer
-    if ($builderStore.inBuilder) {
+    // Set up an observer to watch for changes in metadata attributes of child
-      // Set up an observer to watch for changes in metadata attributes of child
+    // components, as well as child addition and deletion
-      // components, as well as child addition and deletion
+    observer = new MutationObserver(mutations => {
-      observer = new MutationObserver(mutations => {
+      for (let mutation of mutations) {
-        for (let mutation of mutations) {
+        const { target, type, addedNodes, removedNodes } = mutation
-          const { target, type, addedNodes, removedNodes } = mutation
+        if (target === ref) {
-          if (target === ref) {
+          if (addedNodes[0]?.classList?.contains("component")) {
-            if (addedNodes[0]?.classList?.contains("component")) {
+            // We've added a new child component inside the grid, so we need
-              // We've added a new child component inside the grid, so we need
+            // to consider it when determining required rows
-              // to consider it when determining required rows
+            storeChild(addedNodes[0])
-              storeChild(addedNodes[0])
+          } else if (removedNodes[0]?.classList?.contains("component")) {
-            } else if (removedNodes[0]?.classList?.contains("component")) {
+            // We've removed a child component inside the grid, so we need
-              // We've removed a child component inside the grid, so we need
+            // to stop considering it when determining required rows
-              // to stop considering it when determining required rows
+            removeChild(removedNodes[0])
-              removeChild(removedNodes[0])
-            }
-          } else if (
-            type === "attributes" &&
-            target.parentNode === ref &&
-            target.classList.contains("component")
-          ) {
-            // We've updated the size or position of a child
-            storeChild(target)
           }
+        } else if (
+          type === "attributes" &&
+          target.parentNode === ref &&
+          target.classList.contains("component")
+        ) {
+          // We've updated the size or position of a child
+          storeChild(target)
         }
-      })
+      }
-      observer.observe(ref, {
+    })
-        childList: true,
+    observer.observe(ref, {
-        attributes: true,
+      childList: true,
-        subtree: true,
+      attributes: true,
-        attributeFilter: [
+      subtree: true,
-          "data-grid-desktop-row-end",
+      attributeFilter: [
-          "data-grid-mobile-row-end",
+        "data-grid-desktop-row-end",
-        ],
+        "data-grid-mobile-row-end",
-      })
+      ],
-    }
+    })
 
     // Now that the observer is set up, we mark the grid as mounted to mount
     // our child components
@@ -138,26 +136,11 @@
 <style>
   .grid {
     position: relative;
-
-    /*
-      Prevent cross-grid variable inheritance. The other variables for alignment
-      are always set on each component, so we don't need to worry about
-      inheritance.
-    */
-    --grid-desktop-col-start: initial;
-    --grid-desktop-col-end: initial;
-    --grid-desktop-row-start: initial;
-    --grid-desktop-row-end: initial;
-    --grid-mobile-col-start: initial;
-    --grid-mobile-col-end: initial;
-    --grid-mobile-row-start: initial;
-    --grid-mobile-row-end: initial;
   }
-
   .grid,
   .underlay {
     height: var(--height) !important;
-    min-height: none !important;
+    min-height: 0 !important;
     max-height: none !important;
     display: grid;
     grid-template-rows: repeat(var(--rows), calc(var(--row-size) * 1px));
@@ -206,27 +189,10 @@
     margin: calc(var(--grid-spacing) * 1px);
 
     /* On desktop, use desktop metadata and fall back to mobile */
-    /* Position vars */
+    --col-start: var(--grid-desktop-col-start, var(--grid-mobile-col-start));
-    --col-start: var(--grid-desktop-col-start, var(--grid-mobile-col-start, 1));
+    --col-end: var(--grid-desktop-col-end, var(--grid-mobile-col-end));
-    --col-end: var(
+    --row-start: var(--grid-desktop-row-start, var(--grid-mobile-row-start));
-      --grid-desktop-col-end,
-      var(
-        --grid-mobile-col-end,
-        round(
-          up,
-          calc(
-            (var(--grid-spacing) * 2 + var(--default-width)) / var(--col-size) +
-              1
-          )
-        )
-      )
-    );
-
-    /* Row end is always provided by the gridLayout action */
-    --row-start: var(--grid-desktop-row-start, var(--grid-mobile-row-start, 1));
     --row-end: var(--grid-desktop-row-end, var(--grid-mobile-row-end));
-
-    /* Flex vars */
     --h-align: var(--grid-desktop-h-align, var(--grid-mobile-h-align));
     --v-align: var(--grid-desktop-v-align, var(--grid-mobile-v-align));
 
@@ -247,24 +213,10 @@
 
   /* On mobile, use mobile metadata and fall back to desktop */
   .grid.mobile :global(> .component) {
-    /* Position vars */
+    --col-start: var(--grid-mobile-col-start, var(--grid-desktop-col-start));
-    --col-start: var(--grid-mobile-col-start, var(--grid-desktop-col-start, 1));
+    --col-end: var(--grid-mobile-col-end, var(--grid-desktop-col-end));
-    --col-end: var(
+    --row-start: var(--grid-mobile-row-start, var(--grid-desktop-row-start));
-      --grid-mobile-col-end,
-      var(
-        --grid-desktop-col-end,
-        round(
-          up,
-          calc(
-            (var(--spacing) * 2 + var(--default-width)) / var(--col-size) + 1
-          )
-        )
-      )
-    );
-    --row-start: var(--grid-mobile-row-start, var(--grid-desktop-row-start, 1));
     --row-end: var(--grid-mobile-row-end, var(--grid-desktop-row-end));
-
-    /* Flex vars */
     --h-align: var(--grid-mobile-h-align, var(--grid-desktop-h-align));
     --v-align: var(--grid-mobile-v-align, var(--grid-desktop-v-align));
   }
@@ -273,11 +225,12 @@
   .grid :global(> .component > *) {
     flex: 0 0 auto !important;
   }
-  .grid:not(.mobile) :global(> .component.grid-desktop-grow > *) {
+  .grid:not(.mobile)
+    :global(> .component[data-grid-desktop-v-align="stretch"] > *) {
     flex: 1 1 0 !important;
     height: 0 !important;
   }
-  .grid.mobile :global(> .component.grid-mobile-grow > *) {
+  .grid.mobile :global(> .component[data-grid-mobile-v-align="stretch"] > *) {
     flex: 1 1 0 !important;
     height: 0 !important;
   }

packages/client/src/utils/grid.js

@@ -82,57 +82,62 @@ export const gridLayout = (node, metadata) => {
       builderStore.actions.selectComponent(id)
     }
 
-    // Generate base set of grid CSS vars based for this component
+    // Determine default width and height of component
     let width = errored ? 500 : definition.size?.width || 200
     let height = errored ? 60 : definition.size?.height || 200
     width += 2 * GridSpacing
     height += 2 * GridSpacing
-    const hAlign = errored ? "stretch" : definition?.grid?.hAlign || "stretch"
+    let vars = {
-    const vAlign = errored ? "stretch" : definition?.grid?.vAlign || "center"
-    const vars = {
       "--default-width": width,
       "--default-height": height,
-      "--grid-desktop-h-align": hAlign,
-      "--grid-mobile-h-align": hAlign,
-      "--grid-desktop-v-align": vAlign,
-      "--grid-mobile-v-align": vAlign,
-
-      // Variables for automatically determining grid height
-      "--grid-desktop-row-end": Math.ceil(height / GridRowHeight) + 1,
-      "--grid-mobile-row-end": Math.ceil(height / GridRowHeight) + 1,
     }
 
-    // Extract any other CSS variables from the saved component styles
+    // Generate defaults for all grid params
-    for (let style of Object.keys(styles)) {
+    const defaults = {
-      if (style.startsWith("--")) {
+      [GridParams.HAlign]: definition?.grid?.hAlign || "stretch",
-        vars[style] = styles[style]
+      [GridParams.VAlign]: definition?.grid?.vAlign || "center",
-        delete styles[style]
+      [GridParams.ColStart]: 1,
-      }
+      [GridParams.ColEnd]:
+        "round(up, calc((var(--grid-spacing) * 2 + var(--default-width)) / var(--col-size) + 1))",
+      [GridParams.RowStart]: 1,
+      [GridParams.RowEnd]: Math.ceil(height / GridRowHeight) + 1,
+    }
+
+    // Specify values for all grid params for all devices, and strip these CSS
+    // variables from the styles being applied to the inner component, as we
+    // want to apply these to the wrapper instead
+    for (let param of Object.values(GridParams)) {
+      let dVar = getGridVar(Devices.Desktop, param)
+      let mVar = getGridVar(Devices.Mobile, param)
+      vars[dVar] = styles[dVar] || styles[mVar] || defaults[param]
+      vars[mVar] = styles[mVar] || styles[dVar] || defaults[param]
+      delete styles[dVar]
+      delete styles[mVar]
+    }
+
+    // Apply some overrides depending on component state
+    if (errored) {
+      vars[getGridVar(Devices.Desktop, GridParams.HAlign)] = "stretch"
+      vars[getGridVar(Devices.Mobile, GridParams.HAlign)] = "stretch"
+      vars[getGridVar(Devices.Desktop, GridParams.VAlign)] = "stretch"
+      vars[getGridVar(Devices.Mobile, GridParams.VAlign)] = "stretch"
     }
 
     // Apply some metadata to data attributes to speed up lookups
-    const desktopRowEnd = `${vars["--grid-desktop-row-end"]}`
+    const addDataTag = (tagName, device, param) => {
-    const mobileRowEnd = `${vars["--grid-mobile-row-end"]}`
+      const val = `${vars[getGridVar(device, param)]}`
-    if (node.dataset.gridDesktopRowEnd !== desktopRowEnd) {
+      if (node.dataset[tagName] !== val) {
-      node.dataset.gridDesktopRowEnd = desktopRowEnd
+        node.dataset[tagName] = val
-    }
+      }
-    if (node.dataset.gridMobileRowEnd !== mobileRowEnd) {
-      node.dataset.gridMobileRowEnd = mobileRowEnd
     }
+    addDataTag("gridDesktopRowEnd", Devices.Desktop, GridParams.RowEnd)
+    addDataTag("gridMobileRowEnd", Devices.Mobile, GridParams.RowEnd)
+    addDataTag("gridDesktopVAlign", Devices.Desktop, GridParams.VAlign)
+    addDataTag("gridMobileVAlign", Devices.Mobile, GridParams.VAlign)
 
     // Apply all CSS variables to the wrapper
     node.style = buildStyleString(vars)
 
-    // Toggle classes to specify whether our children should fill
-    node.classList.toggle(
-      GridClasses.DesktopFill,
-      vars[getGridVar(Devices.Desktop, GridParams.VAlign)] === "stretch"
-    )
-    node.classList.toggle(
-      GridClasses.MobileFill,
-      vars[getGridVar(Devices.Mobile, GridParams.VAlign)] === "stretch"
-    )
-
     // Add a listener to select this node on click
     if (interactive) {
       node.addEventListener("click", selectComponent, false)