Merge pull request #3260 from penpot/hiru-component-outline-color

🐛 Detect correctly color of outlines and controls of components

common/src/app/common/types/component.cljc

@@ -40,7 +40,19 @@
   [shape]
   (some? (:main-instance? shape)))
 
-(defn is-main-instance?
+(defn in-component-copy?
+  "Check if the shape is inside a component non-main instance."
+  [shape]
+  (some? (:shape-ref shape)))
+
+(defn in-component-copy-not-root?
+  "Check if the shape is inside a component non-main instance and
+  is not the root shape."
+  [shape]
+  (and (some? (:shape-ref shape))
+       (nil? (:component-id shape))))
+
+(defn main-instance-of?
   "Check if this shape is the root of the main instance of the given component."
   [shape-id page-id component]
   (and (= shape-id (:main-instance-id component))
@@ -58,18 +70,6 @@
   (and (some? (:component-id shape))
        (= (:component-file shape) library-id)))
 
-(defn in-component-copy?
-  "Check if the shape is inside a component non-main instance."
-  [shape]
-  (some? (:shape-ref shape)))
-
-(defn in-component-copy-not-root?
-  "Check if the shape is inside a component non-main instance and
-  is not the root shape."
-  [shape]
-  (and (some? (:shape-ref shape))
-       (nil? (:component-id shape))))
-
 (defn detach-shape
   "Remove the links and leave it as a plain shape, detached from any component."
   [shape]

common/src/app/common/types/container.cljc

@@ -109,6 +109,23 @@
     :else
     (get-component-shape objects (get objects (:parent-id shape)) options))))
 
+(defn in-component-main?
+  "Check if the shape is inside a component non-main instance.
+   
+   Note that we must iterate on the parents because non-root shapes in
+   a main component have not any discriminating attribute."
+  [objects shape]
+  (let [component-shape (get-component-shape objects shape {:allow-main? true})]
+    (:main-instance? component-shape)))
+
+(defn in-any-component?
+  "Check if the shape is part of any component (main or copy), wether it's
+   head or not."
+  [objects shape]
+  (or (ctk/in-component-copy? shape)
+      (ctk/main-instance? shape)
+      (in-component-main? objects shape)))
+
 (defn make-component-shape
   "Clone the shape and all children. Generate new ids and detach
   from parent and frame. Update the original shapes to have links

common/test/common_tests/types_file_test.cljc

@@ -109,7 +109,7 @@
     (t/is (= (:name p-group) "Group1"))
     (t/is (ctk/instance-of? p-group file-id (:id component1)))
     (t/is (not (:main-instance? p-group)))
-    (t/is (not (ctk/is-main-instance? (:id p-group) file-page-id component1)))
+    (t/is (not (ctk/main-instance-of? (:id p-group) file-page-id component1)))
     (t/is (ctk/is-main-of? c-group1 p-group))
 
     (t/is (= (:name p-shape) "Rect1"))

frontend/src/app/main/ui/workspace/viewport/outline.cljs

@@ -9,6 +9,8 @@
    [app.common.data :as d]
    [app.common.exceptions :as ex]
    [app.common.geom.shapes :as gsh]
+   [app.common.types.container :as ctn]
+   [app.main.refs :as refs]
    [app.main.ui.hooks :as hooks]
    [app.main.ui.shapes.attrs :as attrs]
    [app.util.object :as obj]
@@ -22,7 +24,6 @@
   [props]
   (let [shape    (obj/get props "shape")
         zoom     (obj/get props "zoom" 1)
-        color    (obj/get props "color")
         modifier (obj/get props "modifier")
 
         shape (gsh/transform-shape shape (:modifiers modifier))
@@ -35,6 +36,13 @@
             (or (ex/ignoring (upf/format-path (:content shape)))
                 "")))
 
+        ;; Note that we don't use mf/deref to avoid a repaint dependency here
+        objects (deref refs/workspace-page-objects)
+
+        color     (if (ctn/in-any-component? objects shape)
+                    "var(--color-component-highlight)"
+                    "var(--color-primary)")
+
         {:keys [x y width height selrect]} shape
 
         border-radius-attrs (attrs/extract-border-radius shape)
@@ -78,19 +86,16 @@
    ::mf/wrap [#(mf/memo' % (mf/check-props ["shapes" "zoom" "modifiers"]))]}
   [props]
 
-  (let [shapes (obj/get props "shapes")
-        zoom   (obj/get props "zoom")
-        modifiers   (obj/get props "modifiers")
-        color  (if (or (> (count shapes) 1) (nil? (:shape-ref (first shapes))))
-                 "var(--color-primary)" "var(--color-component-highlight)")]
+  (let [shapes    (obj/get props "shapes")
+        zoom      (obj/get props "zoom")
+        modifiers (obj/get props "modifiers")]
 
     (for [shape shapes]
       (let [modifier (get modifiers (:id shape))]
         [:& outline {:key (str "outline-" (:id shape))
                      :shape shape
                      :modifier modifier
-                     :zoom zoom
-                     :color color}]))))
+                     :zoom zoom}]))))
 
 (defn- show-outline?
   [shape]

frontend/src/app/main/ui/workspace/viewport/selection.cljs

@@ -11,6 +11,7 @@
    [app.common.geom.matrix :as gmt]
    [app.common.geom.point :as gpt]
    [app.common.geom.shapes :as gsh]
+   [app.common.types.container :as ctn]
    [app.common.types.shape :as cts]
    [app.main.data.workspace :as dw]
    [app.main.refs :as refs]
@@ -441,9 +442,13 @@
   (let [num (count shapes)
         {:keys [type] :as shape} (first shapes)
 
-        color (if (or (> num 1) (nil? (:shape-ref shape)))
-                selection-rect-color-normal
-                selection-rect-color-component)]
+        ;; Note that we don't use mf/deref to avoid a repaint dependency here
+        objects (deref refs/workspace-page-objects)
+
+        color (if (and (= num 1)
+                       (ctn/in-any-component? objects shape))
+                selection-rect-color-component
+                selection-rect-color-normal)]
     (cond
       (zero? num)
       nil
@@ -481,9 +486,13 @@
   (let [num (count shapes)
         {:keys [type] :as shape} (first shapes)
 
-        color (if (or (> num 1) (nil? (:shape-ref shape)))
-                selection-rect-color-normal
-                selection-rect-color-component)]
+        ;; Note that we don't use mf/deref to avoid a repaint dependency here
+        objects (deref refs/workspace-page-objects)
+
+        color (if (and (= num 1)
+                       (ctn/in-any-component? objects shape))
+                selection-rect-color-component
+                selection-rect-color-normal)]
     (cond
       (zero? num)
       nil

frontend/src/app/main/ui/workspace/viewport/widgets.cljs

@@ -10,6 +10,7 @@
    [app.common.data.macros :as dm]
    [app.common.geom.point :as gpt]
    [app.common.pages.helpers :as cph]
+   [app.common.types.container :as ctn]
    [app.common.types.shape-tree :as ctt]
    [app.common.uuid :as uuid]
    [app.main.data.workspace :as dw]
@@ -98,6 +99,15 @@
               #(mf/deferred % ts/raf)]}
   [{:keys [frame selected? zoom show-artboard-names? show-id? on-frame-enter on-frame-leave on-frame-select]}]
   (let [workspace-read-only? (mf/use-ctx ctx/workspace-read-only?)
+
+        ;; Note that we don't use mf/deref to avoid a repaint dependency here
+        objects (deref refs/workspace-page-objects)
+
+        color (when selected?
+                (if (ctn/in-any-component? objects frame)
+                  "var(--color-component-highlight)"
+                  "var(--color-primary-dark)"))
+
         on-pointer-down
         (mf/use-callback
          (mf/deps (:id frame) on-frame-select workspace-read-only?)
@@ -106,7 +116,7 @@
              (when (= 1 (.-which event))
                (dom/prevent-default event)
                (dom/stop-propagation event)
-                 (on-frame-select event (:id frame))))))
+               (on-frame-select event (:id frame))))))
 
         on-double-click
         (mf/use-callback
@@ -146,7 +156,7 @@
                 :width 12
                 :height 12
                 :class "workspace-frame-icon"
-                :style {:fill (when selected? "var(--color-primary-dark)")}
+                :style {:fill color}
                 :visibility (if show-artboard-names? "visible" "hidden")}
           [:use {:href "#icon-set-thumbnail"}]])
        [:text {:x text-pos-x
@@ -154,7 +164,7 @@
                :width (:width frame)
                :height 20
                :class "workspace-frame-label"
-               :style {:fill (when selected? "var(--color-primary-dark)")}
+               :style {:fill color}
                :visibility (if show-artboard-names? "visible" "hidden")
                :on-pointer-down on-pointer-down
                :on-double-click on-double-click