Fix stubborn elements outside visible area indicator arrow (#6367)

**Problem:** The elements outside visible area indicator arrow sometimes shows up even when the element is obviously inside the visible area. **Fix:** Use the fresh metadata to detect the elements outside visible area, because otherwise the frame data might be stale on certain interactions (e.g. insertion). Also, simplified the code by removing some pre-cursor-positioning code that was not relevant anymore. | Before | After | |---------|-----------| | ![Kapture 2024-09-13 at 14 23 17](https://github.com/user-attachments/assets/0e9af6f4-7522-4461-a776-ca92cf6aaf21) | ![Kapture 2024-09-13 at 14 24 03](https://github.com/user-attachments/assets/4a8d9d69-2682-4ae2-b82b-a8d6e8ed8005) | Fixes #6366
concrete-utopia · Sep 13, 2024 · a4ac2e4 · a4ac2e4
1 parent c67b665
commit a4ac2e4
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 68 deletions.
diff --git a/editor/src/components/canvas/controls/elements-outside-visible-area-hooks.tsx b/editor/src/components/canvas/controls/elements-outside-visible-area-hooks.tsx
@@ -1,43 +1,23 @@
 import React from 'react'
 import { MetadataUtils } from '../../../core/model/element-metadata-utils'
 import { mapDropNulls } from '../../../core/shared/array-utils'
-import * as EP from '../../../core/shared/element-path'
-import type { CanvasRectangle, WindowPoint, WindowRectangle } from '../../../core/shared/math-utils'
+import type { CanvasRectangle, WindowPoint } from '../../../core/shared/math-utils'
 import {
   boundingRectangleArray,
   getRectCenter,
-  isFiniteRectangle,
   rectangleIntersection,
   windowRectangle,
 } from '../../../core/shared/math-utils'
-import type { ElementPath } from '../../../core/shared/project-file-types'
-import { Substores, useEditorState, useRefEditorState } from '../../editor/store/store-hook'
+import { Substores, useEditorState } from '../../editor/store/store-hook'
 import { canvasPointToWindowPoint } from '../dom-lookup'
 
-const topBarHeight = 40 // px
-
-type ElementOutsideVisibleArea = {
-  path: ElementPath
-  rect: WindowRectangle
-}
-
 export type ElementOutsideVisibleAreaIndicator = {
   position: WindowPoint
 }
 
 export function useElementsOutsideVisibleArea(): ElementOutsideVisibleAreaIndicator | null {
   const canvasBounds = document.getElementById('canvas-root')?.getBoundingClientRect()
 
-  const selectedViews = useEditorState(
-    Substores.selectedViews,
-    (store) => store.editor.selectedViews,
-    'useElementsOutsideVisibleArea selectedViews',
-  )
-
-  const storeRef = useRefEditorState((store) => ({
-    jsxMetadata: store.editor.jsxMetadata,
-  }))
-
   const canvasScale = useEditorState(
     Substores.canvasOffset,
     (store) => store.editor.canvas.scale,
@@ -49,76 +29,63 @@ export function useElementsOutsideVisibleArea(): ElementOutsideVisibleAreaIndica
     'useElementsOutsideVisibleArea canvasOffset',
   )
 
-  const framesByPathString = React.useMemo(() => {
-    const frames: { [key: string]: CanvasRectangle } = {}
-    for (const path of selectedViews) {
-      const metadata = MetadataUtils.findElementByElementPath(storeRef.current.jsxMetadata, path)
-      if (
-        metadata != null &&
-        metadata.globalFrame != null &&
-        isFiniteRectangle(metadata.globalFrame)
-      ) {
-        frames[EP.toString(path)] = metadata.globalFrame
-      }
-    }
-    return frames
-  }, [storeRef, selectedViews])
-
   const scaledCanvasArea = React.useMemo(() => {
     if (canvasBounds == null) {
       return null
     }
     const scaleRatio = canvasScale > 1 ? canvasScale : 1
     return windowRectangle({
       x: canvasBounds.x * scaleRatio,
-      y: (canvasBounds.y - topBarHeight) * scaleRatio,
+      y: canvasBounds.y * scaleRatio,
       width: canvasBounds.width,
       height: canvasBounds.height,
     })
   }, [canvasBounds, canvasScale])
 
-  const elementsOutsideVisibleArea = React.useMemo(() => {
-    return mapDropNulls((path: ElementPath): ElementOutsideVisibleArea | null => {
-      if (scaledCanvasArea == null) {
-        return null
-      }
-      const frame = framesByPathString[EP.toString(path)]
-      if (frame == null) {
+  const selectedElementsFrames: CanvasRectangle[] = useEditorState(
+    Substores.metadata,
+    (store) => {
+      return mapDropNulls(
+        (view) => MetadataUtils.getFrameOrZeroRectInCanvasCoords(view, store.editor.jsxMetadata),
+        store.editor.selectedViews,
+      )
+    },
+    'useElementsOutsideVisibleArea selectedElementsFrames',
+  )
+
+  return React.useMemo(() => {
+    if (scaledCanvasArea == null) {
+      return null
+    }
+
+    const framesOutsideVisibleArea = mapDropNulls((frame) => {
+      if (frame.width === 0 || frame.height === 0) {
         return null
       }
 
       const topLeftPoint = canvasPointToWindowPoint(frame, canvasScale, canvasOffset)
-      const elementRect = windowRectangle({
+      const elementWindowRect = windowRectangle({
         x: topLeftPoint.x,
-        y: topLeftPoint.y - topBarHeight,
+        y: topLeftPoint.y,
         width: frame.width * canvasScale,
         height: frame.height * canvasScale,
       })
-
-      const isOutsideVisibleArea = rectangleIntersection(scaledCanvasArea, elementRect) == null
-      if (!isOutsideVisibleArea) {
+      if (rectangleIntersection(scaledCanvasArea, elementWindowRect) != null) {
         return null
       }
 
-      return {
-        path: path,
-        rect: elementRect,
-      }
-    }, selectedViews)
-  }, [selectedViews, canvasOffset, canvasScale, scaledCanvasArea, framesByPathString])
+      return elementWindowRect
+    }, selectedElementsFrames)
 
-  return React.useMemo((): ElementOutsideVisibleAreaIndicator | null => {
-    if (elementsOutsideVisibleArea.length === 0) {
-      return null
-    }
-    const windowRect = boundingRectangleArray(elementsOutsideVisibleArea.map((a) => a.rect))
+    const windowRect = boundingRectangleArray(framesOutsideVisibleArea)
     if (windowRect == null) {
       return null
     }
+
     return {
       position: getRectCenter(windowRect),
     }
-  }, [elementsOutsideVisibleArea])
+  }, [selectedElementsFrames, canvasOffset, canvasScale, scaledCanvasArea])
 }
 
 export function getIndicatorAngleToTarget(from: WindowPoint, to: WindowPoint): number {

diff --git a/editor/src/core/performance/__snapshots__/performance-regression-tests.spec.tsx.snap b/editor/src/core/performance/__snapshots__/performance-regression-tests.spec.tsx.snap
@@ -88,8 +88,6 @@ Array [
   "/Symbol(react.memo)()///UtopiaSpiedExoticType(Symbol(react.fragment))",
   "/Symbol(react.forward_ref)(Styled(div))/div/Symbol(react.forward_ref)(Styled(div))/div",
   "/UtopiaSpiedFunctionComponent(SimpleFlexColumn)/div/ElementsOutsideVisibleAreaIndicator/Symbol(react.memo)(IndicatorArrow)",
-  "/div/ElementsOutsideVisibleAreaIndicator/IndicatorArrow/Symbol(react.memo)(Icon)",
-  "/div/ElementsOutsideVisibleAreaIndicator/IndicatorArrow/div:data-testid='indicator-elements-outside-visible-area'",
   "/div/div/NavigatorComponent/Symbol(react.memo)()",
   "/div/div/NavigatorComponent/Symbol(react.memo)(NavigatorDragLayer)",
   "/div/div/NavigatorComponent/UtopiaSpiedClass(AutoSizer)",
@@ -750,8 +748,6 @@ Array [
   "/Symbol(react.memo)()///UtopiaSpiedExoticType(Symbol(react.fragment))",
   "/Symbol(react.forward_ref)(Styled(div))/div/Symbol(react.forward_ref)(Styled(div))/div",
   "/UtopiaSpiedFunctionComponent(SimpleFlexColumn)/div/ElementsOutsideVisibleAreaIndicator/Symbol(react.memo)(IndicatorArrow)",
-  "/div/ElementsOutsideVisibleAreaIndicator/IndicatorArrow/Symbol(react.memo)(Icon)",
-  "/div/ElementsOutsideVisibleAreaIndicator/IndicatorArrow/div:data-testid='indicator-elements-outside-visible-area'",
   "/div/div/NavigatorComponent/Symbol(react.memo)()",
   "/div/div/NavigatorComponent/Symbol(react.memo)(NavigatorDragLayer)",
   "/div/div/NavigatorComponent/UtopiaSpiedClass(AutoSizer)",

diff --git a/editor/src/core/performance/performance-regression-tests.spec.tsx b/editor/src/core/performance/performance-regression-tests.spec.tsx
@@ -183,7 +183,7 @@ describe('React Render Count Tests -', () => {
 
     const renderCountAfter = renderResult.getNumberOfRenders()
     // if this breaks, GREAT NEWS but update the test please :)
-    expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`532`)
+    expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`530`)
     expect(renderResult.getRenderInfo()).toMatchSnapshot()
   })
 
@@ -249,7 +249,7 @@ describe('React Render Count Tests -', () => {
 
     const renderCountAfter = renderResult.getNumberOfRenders()
     // if this breaks, GREAT NEWS but update the test please :)
-    expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`657`)
+    expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`655`)
     expect(renderResult.getRenderInfo()).toMatchSnapshot()
   })
 })