From 07f8e1981a3527e179be0149c07f6c50f63946c1 Mon Sep 17 00:00:00 2001
From: Andrew Kingston <andrew@kingston.dev>
Date: Thu, 21 Mar 2024 13:22:03 +0000
Subject: [PATCH] Improve client indicators by properly caching all properties
 to avoid inconsistent and stale states

---
 .../src/components/preview/DNDHandler.svelte  |  3 +-
 .../components/preview/HoverIndicator.svelte  |  7 +-
 .../src/components/preview/Indicator.svelte   |  6 --
 .../components/preview/IndicatorSet.svelte    | 99 ++++++++++---------
 .../preview/SelectionIndicator.svelte         |  3 +-
 5 files changed, 60 insertions(+), 58 deletions(-)

diff --git a/packages/client/src/components/preview/DNDHandler.svelte b/packages/client/src/components/preview/DNDHandler.svelte
index e93c42b863..767efc9e3a 100644
--- a/packages/client/src/components/preview/DNDHandler.svelte
+++ b/packages/client/src/components/preview/DNDHandler.svelte
@@ -345,8 +345,7 @@
 <IndicatorSet
   componentId={$dndParent}
   color="var(--spectrum-global-color-static-green-500)"
-  zIndex="930"
-  transition
+  zIndex={920}
   prefix="Inside"
 />
 
diff --git a/packages/client/src/components/preview/HoverIndicator.svelte b/packages/client/src/components/preview/HoverIndicator.svelte
index 69e13e4219..d204d77f49 100644
--- a/packages/client/src/components/preview/HoverIndicator.svelte
+++ b/packages/client/src/components/preview/HoverIndicator.svelte
@@ -1,9 +1,11 @@
 <script>
   import { onMount, onDestroy } from "svelte"
   import IndicatorSet from "./IndicatorSet.svelte"
-  import { dndIsDragging, hoverStore } from "stores"
+  import { dndIsDragging, hoverStore, builderStore } from "stores"
 
   $: componentId = $hoverStore.hoveredComponentId
+  $: selectedComponentId = $builderStore.selectedComponentId
+  $: selected = componentId === selectedComponentId
 
   const onMouseOver = e => {
     // Ignore if dragging
@@ -44,7 +46,6 @@
 <IndicatorSet
   componentId={$dndIsDragging ? null : componentId}
   color="var(--spectrum-global-color-static-blue-200)"
-  transition
-  zIndex="890"
+  zIndex={selected ? 890 : 910}
   allowResizeAnchors
 />
diff --git a/packages/client/src/components/preview/Indicator.svelte b/packages/client/src/components/preview/Indicator.svelte
index 03c585f1a1..0480ea91ce 100644
--- a/packages/client/src/components/preview/Indicator.svelte
+++ b/packages/client/src/components/preview/Indicator.svelte
@@ -1,5 +1,4 @@
 <script>
-  import { fade } from "svelte/transition"
   import { Icon } from "@budibase/bbui"
 
   export let top
@@ -11,7 +10,6 @@
   export let color
   export let zIndex
   export let componentId
-  export let transition = false
   export let line = false
   export let alignRight = false
   export let showResizeAnchors = false
@@ -31,10 +29,6 @@
 </script>
 
 <div
-  transition:fade|local={{
-    delay: transition ? 100 : 0,
-    duration: transition ? 100 : 0,
-  }}
   class="indicator"
   class:flipped
   class:line
diff --git a/packages/client/src/components/preview/IndicatorSet.svelte b/packages/client/src/components/preview/IndicatorSet.svelte
index fc040f226c..3cbd7e2464 100644
--- a/packages/client/src/components/preview/IndicatorSet.svelte
+++ b/packages/client/src/components/preview/IndicatorSet.svelte
@@ -4,28 +4,39 @@
   import { domDebounce } from "utils/domDebounce"
   import { builderStore } from "stores"
 
-  export let componentId
-  export let color
-  export let transition
-  export let zIndex
+  export let componentId = null
+  export let color = null
+  export let zIndex = 900
   export let prefix = null
   export let allowResizeAnchors = false
 
-  let indicators = []
+  const errorColor = "var(--spectrum-global-color-static-red-600)"
+  const defaultState = () => ({
+    // Cached props
+    componentId,
+    color,
+    zIndex,
+    prefix,
+    allowResizeAnchors,
+
+    // Computed state
+    indicators: [],
+    text: null,
+    icon: null,
+    insideGrid: false,
+    error: false,
+  })
+
   let interval
-  let text
-  let icon
-  let insideGrid = false
-  let errorState = false
-
-  $: visibleIndicators = indicators.filter(x => x.visible)
-  $: offset = $builderStore.inBuilder ? 0 : 2
-  $: componentId, debouncedUpdate()
-
+  let state = defaultState()
+  let nextState = null
   let updating = false
   let observers = []
   let callbackCount = 0
-  let nextIndicators = []
+
+  $: visibleIndicators = state.indicators.filter(x => x.visible)
+  $: offset = $builderStore.inBuilder ? 0 : 2
+  $: $$props, debouncedUpdate()
 
   const checkInsideGrid = id => {
     const component = document.getElementsByClassName(id)[0]
@@ -45,10 +56,10 @@
     if (callbackCount >= observers.length) {
       return
     }
-    nextIndicators[idx].visible =
-      nextIndicators[idx].insideSidePanel || entries[0].isIntersecting
+    nextState.indicators[idx].visible =
+      nextState.indicators[idx].insideSidePanel || entries[0].isIntersecting
     if (++callbackCount === observers.length) {
-      indicators = nextIndicators
+      state = nextState
       updating = false
     }
   }
@@ -60,7 +71,7 @@
 
     // Sanity check
     if (!componentId) {
-      indicators = []
+      state = defaultState()
       return
     }
 
@@ -69,25 +80,25 @@
     callbackCount = 0
     observers.forEach(o => o.disconnect())
     observers = []
-    nextIndicators = []
+    nextState = defaultState()
 
     // Check if we're inside a grid
     if (allowResizeAnchors) {
-      insideGrid = checkInsideGrid(componentId)
+      nextState.insideGrid = checkInsideGrid(componentId)
     }
 
     // Determine next set of indicators
     const parents = document.getElementsByClassName(componentId)
     if (parents.length) {
-      text = parents[0].dataset.name
-      if (prefix) {
-        text = `${prefix} ${text}`
+      nextState.text = parents[0].dataset.name
+      if (nextState.prefix) {
+        nextState.text = `${nextState.prefix} ${nextState.text}`
       }
       if (parents[0].dataset.icon) {
-        icon = parents[0].dataset.icon
+        nextState.icon = parents[0].dataset.icon
       }
     }
-    errorState = parents?.[0]?.classList.contains("error")
+    nextState.error = parents?.[0]?.classList.contains("error")
 
     // Batch reads to minimize reflow
     const scrollX = window.scrollX
@@ -103,8 +114,9 @@
 
     // If there aren't any nodes then reset
     if (!children.length) {
-      indicators = []
+      state = defaultState()
       updating = false
+      return
     }
 
     const device = document.getElementById("app-root")
@@ -120,7 +132,7 @@
       observers.push(observer)
 
       const elBounds = child.getBoundingClientRect()
-      nextIndicators.push({
+      nextState.indicators.push({
         top: elBounds.top + scrollY - deviceBounds.top - offset,
         left: elBounds.left + scrollX - deviceBounds.left - offset,
         width: elBounds.width + 4,
@@ -145,20 +157,17 @@
   })
 </script>
 
-{#key componentId}
-  {#each visibleIndicators as indicator, idx}
-    <Indicator
-      top={indicator.top}
-      left={indicator.left}
-      width={indicator.width}
-      height={indicator.height}
-      text={idx === 0 ? text : null}
-      icon={idx === 0 ? icon : null}
-      showResizeAnchors={allowResizeAnchors && insideGrid}
-      color={errorState ? "var(--spectrum-global-color-static-red-600)" : color}
-      {componentId}
-      {transition}
-      {zIndex}
-    />
-  {/each}
-{/key}
+{#each visibleIndicators as indicator, idx}
+  <Indicator
+    top={indicator.top}
+    left={indicator.left}
+    width={indicator.width}
+    height={indicator.height}
+    text={idx === 0 ? state.text : null}
+    icon={idx === 0 ? state.icon : null}
+    showResizeAnchors={state.allowResizeAnchors && state.insideGrid}
+    color={state.error ? errorColor : state.color}
+    componentId={state.componentId}
+    zIndex={state.zIndex}
+  />
+{/each}
diff --git a/packages/client/src/components/preview/SelectionIndicator.svelte b/packages/client/src/components/preview/SelectionIndicator.svelte
index bca0341628..a271389cbd 100644
--- a/packages/client/src/components/preview/SelectionIndicator.svelte
+++ b/packages/client/src/components/preview/SelectionIndicator.svelte
@@ -10,7 +10,6 @@
 <IndicatorSet
   componentId={$builderStore.selectedComponentId}
   {color}
-  zIndex="910"
-  transition
+  zIndex={900}
   allowResizeAnchors
 />