From 1e418adba55c4fc31b5cc23c0728e4e19c7deed5 Mon Sep 17 00:00:00 2001
From: Sean Parsons <217400+seanparsons@users.noreply.github.com>
Date: Wed, 29 May 2024 11:14:04 +0100
Subject: [PATCH] fix(navigator) Navigator icon colours are more consistent.
 (#5773)

- Extracted out the logic for `color`, `iconType` and `transform` properties.
- Only have the one place in `LayoutIcon` that creates the `Icn` using the
  extracted out properties.
---
 .../navigator/navigator-item/layout-icon.tsx  | 183 +++++++-----------
 1 file changed, 72 insertions(+), 111 deletions(-)

diff --git a/editor/src/components/navigator/navigator-item/layout-icon.tsx b/editor/src/components/navigator/navigator-item/layout-icon.tsx
index 59af6e1c3487..c96e980a5fad 100644
--- a/editor/src/components/navigator/navigator-item/layout-icon.tsx
+++ b/editor/src/components/navigator/navigator-item/layout-icon.tsx
@@ -60,7 +60,12 @@ const isZeroSizedDivSelector = createCachedSelector(
 
 export const LayoutIcon: React.FunctionComponent<React.PropsWithChildren<LayoutIconProps>> =
   React.memo((props) => {
-    const { elementWarnings, color, warningText: propsWarningText, navigatorEntry } = props
+    const {
+      elementWarnings,
+      color: baseColor,
+      warningText: propsWarningText,
+      navigatorEntry,
+    } = props
     const { iconProps, isPositionAbsolute } = useLayoutOrElementIcon(navigatorEntry)
 
     const addAbsoluteMarkerToIcon = isPositionAbsolute
@@ -104,128 +109,84 @@ export const LayoutIcon: React.FunctionComponent<React.PropsWithChildren<LayoutI
       [navigatorEntry],
     )
 
-    const icon = React.useMemo(() => {
-      const defaults = {
-        ...iconProps,
-        color: color,
-        style: { opacity: 'var(--iconOpacity)' },
-      }
+    const { color, iconType, transform } = React.useMemo(() => {
+      let colorToReturn = baseColor
+      let iconTypeToReturn = iconProps.type
+      let transformToReturn: string | undefined = undefined
       if (props.override != null) {
-        return (
-          <Icn
-            category='navigator-element'
-            type={props.override}
-            width={12}
-            height={12}
-            testId={iconTestId}
-            color={props.color}
-          />
-        )
-      }
-      if (isZeroSized) {
-        return (
-          <Icn
-            category='navigator-element'
-            type='zerosized-div'
-            testId={iconTestId}
-            color={color === 'subdued' ? 'subdued' : 'main'}
-            width={12}
-            height={12}
-          />
-        )
-      }
-      if (isInvalidOverrideNavigatorEntry(navigatorEntry)) {
-        return (
-          <Icn
-            category='navigator-element'
-            type='warningtriangle'
-            tooltipText={navigatorEntry.message}
-            testId={iconTestId}
-            color={color}
-            width={12}
-            height={12}
-          />
-        )
+        iconTypeToReturn = props.override
+        if (baseColor === 'white') {
+          colorToReturn = baseColor
+        } else if (props.override === 'row' || props.override === 'column') {
+          colorToReturn = 'primary'
+        } else {
+          colorToReturn = baseColor
+        }
+      } else if (isZeroSized) {
+        iconTypeToReturn = 'zerosized-div'
+        colorToReturn = baseColor === 'subdued' ? 'subdued' : 'main'
+      } else if (isInvalidOverrideNavigatorEntry(navigatorEntry)) {
+        iconTypeToReturn = 'warningtriangle'
+        colorToReturn = baseColor
       } else if (warningText == null) {
-        return (
-          <div
-            style={{
-              // with the current design, for absolute elements we apply a 4 corner overlay over the icon,
-              // so we shrink it to make it visually fit inside the box
-              transform: addAbsoluteMarkerToIcon ? 'scale(.8)' : undefined,
-            }}
-          >
-            <Icn
-              {...defaults}
-              color={
-                defaults.color == 'white'
-                  ? 'white'
-                  : defaults.color == 'component'
-                  ? 'component'
-                  : defaults.type === 'row' ||
-                    defaults.type === 'column' ||
-                    defaults.type === 'flex-column' ||
-                    defaults.type === 'flex-row' ||
-                    defaults.type === 'grid'
-                  ? 'primary'
-                  : defaults.color
-              }
-              category='navigator-element'
-              width={12}
-              height={12}
-              testId={iconTestId}
-            />
-          </div>
-        )
+        transformToReturn = addAbsoluteMarkerToIcon ? 'scale(.8)' : undefined
+        if (baseColor === 'white') {
+          colorToReturn = 'white'
+        } else if (baseColor === 'component') {
+          colorToReturn = 'component'
+        } else if (
+          iconProps.type === 'row' ||
+          iconProps.type === 'column' ||
+          iconProps.type === 'flex-column' ||
+          iconProps.type === 'flex-row' ||
+          iconProps.type === 'grid'
+        ) {
+          colorToReturn = 'primary'
+        } else {
+          colorToReturn = baseColor
+        }
       } else if (isErroredGroup) {
-        return (
-          <Icn
-            category='navigator-element'
-            type='group-problematic'
-            tooltipText={warningText}
-            testId={iconTestId}
-            color={color}
-            width={12}
-            height={12}
-          />
-        )
+        iconTypeToReturn = 'group-problematic'
       } else if (isErroredGroupChild) {
-        return (
-          <Icn
-            {...defaults}
-            testId={iconTestId}
-            tooltipText={warningText}
-            category='navigator-element'
-            width={12}
-            height={12}
-          />
-        )
+        iconTypeToReturn = iconProps.type
       } else {
-        return (
+        iconTypeToReturn = 'warningtriangle'
+      }
+      return { color: colorToReturn, iconType: iconTypeToReturn, transform: transformToReturn }
+    }, [
+      addAbsoluteMarkerToIcon,
+      baseColor,
+      iconProps.type,
+      isErroredGroup,
+      isErroredGroupChild,
+      isZeroSized,
+      navigatorEntry,
+      props.override,
+      warningText,
+    ])
+
+    const icon = React.useMemo(() => {
+      return (
+        <div
+          style={{
+            // with the current design, for absolute elements we apply a 4 corner overlay over the icon,
+            // so we shrink it to make it visually fit inside the box
+            transform: transform,
+          }}
+        >
           <Icn
             category='navigator-element'
-            type='warningtriangle'
-            tooltipText={warningText}
+            type={iconType}
+            tooltipText={warningText ?? undefined}
+            style={{ opacity: 'var(--iconOpacity)' }}
             testId={iconTestId}
             color={color}
             width={12}
             height={12}
           />
-        )
-      }
-    }, [
-      iconProps,
-      color,
-      isZeroSized,
-      navigatorEntry,
-      warningText,
-      isErroredGroup,
-      isErroredGroupChild,
-      iconTestId,
-      addAbsoluteMarkerToIcon,
-      props.override,
-      props.color,
-    ])
+        </div>
+      )
+    }, [transform, iconType, warningText, iconTestId, color])
 
     const marker = React.useMemo(() => {
       if (warningText != null && isErroredGroupChild) {