✨ Show added and deleted layers in the board that holds them

The compare dialog drew nothing for a deleted layer when the branch was
open, and nothing for an added one when main was: the diff named the
layer by id and label only, and the open file lacks it. The detail pane
was left with the layer's name and nothing else.

Added and deleted shape entries now carry the whole shape, as the
document that holds it has it, under `:shape`. The key names no side,
so it reads the same in both directions.

The previews use it. A layer added or deleted inside a board is shown
in that board on both sides, outlined in green or red on the side that
holds it, so the change reads as what the board gains or loses. The
facts of the detail pane (where it was, type, size) read it as well.

A board that moved or resized now draws at its own size on the side
the open file lacks: the diff leaves out `:selrect` and `:points`, so
they are rebuilt from position and size instead of kept from the open
side.

common/src/app/common/files/branch_merge.cljc

@@ -839,6 +839,19 @@
                                                    :base (get bo (:id e))
                                                    :main (get to (:id e))
                                                    :branch (get oo (:id e))))
+                                ;; an added or deleted shape is missing from
+                                ;; one of the two documents, and the compare
+                                ;; panel may have that one open: the entry
+                                ;; carries the FULL shape as the side that
+                                ;; holds it has it, so the panel can draw it
+                                ;; and say what it is. `:shape` names no side,
+                                ;; so `doc-sides` leaves it alone
+                                with-shape (fn [e]
+                                             (case (:status e)
+                                               :added   (assoc e :shape (get oo (:id e)))
+                                               :deleted (assoc e :shape (or (get bo (:id e))
+                                                                            (get to (:id e))))
+                                               e))
                                 passed (tree-policy-pass res bo to oo sbo sto reorders pid modes)]
                             {:changes   (mapv (fn [e]
                                                 ;; a `:refuse` entry names the
@@ -847,7 +860,7 @@
                                                 ;; display
                                                 (if (= :unsupported (:status e))
                                                   e
-                                                  (enrich e)))
+                                                  (-> e enrich with-shape)))
                                               (:changes passed))
                              :conflicts (mapv (comp rehydrate enrich) (:conflicts passed))}))
                         common)]

common/test/common_tests/files_branch_merge_test.cljc

@@ -154,6 +154,28 @@
     (t/is (= :deleted (:status c)))
     (t/is (= :s2 (:id c)))))
 
+(t/deftest added-and-deleted-carry-the-shape
+  ;; the open file lacks the shape on one side, so the entry carries it
+  ;; whole, as the document that holds it has it, in both directions
+  (let [base   (mkdata {:s1 {:id :s1 :name "A" :selrect {:x 1}}
+                        :s2 {:id :s2 :name "B" :selrect {:x 2}}})
+        branch (mkdata {:s1 {:id :s1 :name "A" :selrect {:x 1}}
+                        :s3 {:id :s3 :name "C" :selrect {:x 3}}})
+        merge  (into {} (map (juxt :id identity))
+                     (:changes (bm/compute-merge base base branch :branch->main)))
+        update (into {} (map (juxt :id identity))
+                     (:changes (bm/compute-merge base branch base :main->branch)))]
+    (t/is (= {:id :s2 :name "B" :selrect {:x 2}} (get-in merge [:s2 :shape])))
+    (t/is (= {:id :s3 :name "C" :selrect {:x 3}} (get-in merge [:s3 :shape])))
+    (t/is (= {:id :s2 :name "B" :selrect {:x 2}} (get-in update [:s2 :shape])))
+    (t/is (= {:id :s3 :name "C" :selrect {:x 3}} (get-in update [:s3 :shape])))))
+
+(t/deftest modified-carries-no-shape
+  (let [base   (mkdata {:s1 {:id :s1 :name "A" :fill "red"}})
+        branch (mkdata {:s1 {:id :s1 :name "A" :fill "blue"}})
+        r      (bm/compute-merge base base branch :branch->main)]
+    (t/is (not (contains? (first (:changes r)) :shape)))))
+
 (t/deftest fast-forward-main-untouched
   ;; main == base; branch changed -> all branch changes apply cleanly
   (let [base   (mkdata {:s1 {:id :s1 :name "A" :fill "red"}})

frontend/src/app/main/ui/workspace/sidebar/branch_review/compare.cljs

@@ -226,7 +226,7 @@
   "MAIN and BRANCH, side by side, each with what it holds for the change."
   {::mf/private true}
   [{:keys [kind main-value branch-value main-objects branch-objects
-           main-detail branch-detail main-missing branch-missing large]}]
+           main-detail branch-detail main-missing branch-missing large highlight]}]
   [:div {:class (stl/css :cards)}
    [:> preview-card* {:label (tr "workspace.branches.review.side.main")
                       :side "main"
@@ -235,7 +235,8 @@
     [:> brp/entity-preview* {:kind kind
                              :value main-value
                              :objects main-objects
-                             :missing main-missing}]]
+                             :missing main-missing
+                             :highlight highlight}]]
    [:> preview-card* {:label (tr "workspace.branches.review.side.branch")
                       :side "branch"
                       :detail branch-detail
@@ -243,7 +244,8 @@
     [:> brp/entity-preview* {:kind kind
                              :value branch-value
                              :objects branch-objects
-                             :missing branch-missing}]]])
+                             :missing branch-missing
+                             :highlight highlight}]]])
 
 (mf/defc view-modes*
   "Side by side or overlay for the previews of a change, as icon toggles,
@@ -282,12 +284,23 @@
                      (when objects (brp/side-objects objects page-items :main open-side)))
         branch-objs (mf/with-memo [objects page-items open-side]
                       (when objects (brp/side-objects objects page-items :branch open-side)))
-        main-v    (if (= :shape kind)
-                    (or (get main-objs (:id item)) (get item :main))
-                    (brp/side-entity item :main open-side (library-entity data item)))
-        branch-v  (if (= :shape kind)
-                    (or (get branch-objs (:id item)) (get item :branch))
-                    (brp/side-entity item :branch open-side (library-entity data item)))
+        ;; an added or deleted layer inside a board is shown in it: the
+        ;; board on both sides, the layer outlined on the side that holds
+        ;; it, so the change reads as what the board gains or loses
+        stage     (when (and (= :shape kind) (contains? #{:added :deleted} (:status item)))
+                    (let [holder (if (contains? main-objs (:id item)) main-objs branch-objs)
+                          fid    (brp/context-frame-id holder (:id item))]
+                      (when (and (some? fid) (contains? main-objs fid) (contains? branch-objs fid))
+                        fid)))
+        highlight (when stage {:id (:id item) :status (:status item)})
+        main-v    (cond
+                    stage          (get main-objs stage)
+                    (= :shape kind) (or (get main-objs (:id item)) (get item :main))
+                    :else          (brp/side-entity item :main open-side (library-entity data item)))
+        branch-v  (cond
+                    stage          (get branch-objs stage)
+                    (= :shape kind) (or (get branch-objs (:id item)) (get item :branch))
+                    :else          (brp/side-entity item :branch open-side (library-entity data item)))
         main-detail   (some-> (:main-at diff-meta) ct/inst ct/timeago)
         branch-detail (some-> (:branch-at diff-meta) ct/inst ct/timeago)]
     (when (or main-v branch-v)
@@ -300,12 +313,15 @@
           [:div {:class (stl/css :overlay-stage)}
            (when main-v
              [:div {:class (stl/css :overlay-layer)}
-              [:> brp/entity-preview* {:kind kind :value main-v :objects main-objs}]])
+              [:> brp/entity-preview* {:kind kind :value main-v :objects main-objs
+                                       :highlight highlight}]])
            (when branch-v
              [:div {:class (stl/css :overlay-layer :is-top)}
-              [:> brp/entity-preview* {:kind kind :value branch-v :objects branch-objs}]])]]
+              [:> brp/entity-preview* {:kind kind :value branch-v :objects branch-objs
+                                       :highlight highlight}]])]]
          [:> side-cards* {:kind kind
                           :large large
+                          :highlight highlight
                           :main-value main-v
                           :branch-value branch-v
                           :main-objects main-objs
@@ -402,7 +418,8 @@
 
          :shape
          (when (contains? #{:added :deleted} (:status item))
-           [:> brf/shape-facts* {:item item :data data :incoming incoming}])
+           [:> brf/shape-facts* {:item item :items items :data data
+                                 :open-side open-side :incoming incoming}])
 
          (:component :color :typography :token)
          [:> brf/usage-facts* {:item item :data data}]

frontend/src/app/main/ui/workspace/sidebar/branch_review/facts.cljs

@@ -8,7 +8,7 @@
   "What the detail pane says about an entry that has no before and after:
   a new or deleted page or layer, and the library entries other layers
   depend on. It answers what the entry is, where it lands and what it
-  affects, from the open file."
+  affects, from the open file and, for a layer it lacks, from the diff."
   (:require-macros [app.main.style :as stl])
   (:require
    [app.common.data.macros :as dm]
@@ -137,8 +137,17 @@
 (mf/defc shape-facts*
   "A new or deleted layer: where it lands (or was), what it is and, for a
   deleted main component, the copies it leaves behind."
-  [{:keys [item data incoming]}]
-  (let [objects  (get-in data [:pages-index (:page-id item) :objects])
+  [{:keys [item items data open-side incoming]}]
+  (let [open-objs (get-in data [:pages-index (:page-id item) :objects])
+        ;; a layer the open file lacks is read on the other side, where
+        ;; the changes of `items` put it and its deleted parents back
+        objects  (mf/with-memo [open-objs items item open-side]
+                   (if (or (contains? open-objs (:id item)) (nil? open-objs))
+                     open-objs
+                     (brp/side-objects open-objs
+                                       (filterv #(= (:page-id item) (:page-id %)) items)
+                                       (if (= open-side :main) :branch :main)
+                                       open-side)))
         shape    (get objects (:id item))
         page     (get-in data [:pages-index (:page-id item) :name])
         deleted? (= :deleted (:status item))

frontend/src/app/main/ui/workspace/sidebar/branch_review/previews.cljs

@@ -6,12 +6,13 @@
 
 (ns app.main.ui.workspace.sidebar.branch-review.previews
   "Visual previews of the two sides of a change. The diff carries whole
-  entities only for conflicts; for a clean change the open file holds one
-  side and the changed attrs give the other, which is enough to draw both
-  the layer and the board it sits in."
+  entities for conflicts and for added or deleted shapes; for a modified
+  entity the open file holds one side and the changed attrs give the
+  other, which is enough to draw both the layer and the board it sits in."
   (:require-macros [app.main.style :as stl])
   (:require
    [app.common.data.macros :as dm]
+   [app.common.types.shape :as cts]
    [app.common.uuid :as uuid]
    [app.main.render :as render]
    [app.main.ui.ds.foundations.assets.icon :as i]
@@ -28,6 +29,11 @@
   icon instead."
   #{:rect :circle :frame :path :bool :text :group})
 
+(def ^:private box-shape-types
+  "Shape types whose geometry is their position and size; a path or a
+  boolean takes it from its content, a group from its children."
+  #{:rect :circle :frame :text :image :svg-raw})
+
 (defn open-side
   "Which document the open file is for `branch`: `:branch` when it is the
   branch file, `:main` otherwise."
@@ -41,7 +47,8 @@
   both; a clean change carries the attrs that differ, so the side the open
   file does not hold is the open entity with those attrs swapped in. An
   entry that does not exist on a side (added on the other one, or
-  deleted) has no entity there."
+  deleted) has no entity there. An added or deleted shape the open file
+  lacks comes whole in the entry's `:shape`."
   [item side open-side open-entity]
   (cond
     (= :conflict (:status item))
@@ -51,7 +58,7 @@
     open-entity
 
     (nil? open-entity)
-    nil
+    (:shape item)
 
     (and (= :added (:status item)) (= side :main))
     nil
@@ -60,18 +67,27 @@
     nil
 
     :else
-    (reduce-kv (fn [entity attr values]
-                 (let [v (get values side)]
-                   (if (nil? v)
-                     (dissoc entity attr)
-                     (assoc entity attr v))))
-               open-entity
-               (:changed-attrs item))))
+    (let [attrs  (:changed-attrs item)
+          entity (reduce-kv (fn [entity attr values]
+                              (let [v (get values side)]
+                                (if (nil? v)
+                                  (dissoc entity attr)
+                                  (assoc entity attr v))))
+                            open-entity
+                            attrs)]
+      ;; the diff leaves out `:selrect` and `:points` (they follow from
+      ;; position and size), so a box that moved or resized gets them
+      ;; again from its own, or it would draw with the open side's
+      (if (and (contains? box-shape-types (:type entity))
+               (some #(contains? attrs %) [:x :y :width :height]))
+        (cts/setup-rect (dissoc entity :selrect :points))
+        entity))))
 
 (defn side-objects
   "The page `objects` as they stand on `side`, with every shape change of
   `items` that lives on this page put on that side. A shape missing on
-  that side is taken out of its parent, so boards draw without it."
+  that side is taken out of its parent, so boards draw without it, and a
+  shape only that side holds is put into its parent, so boards draw it."
   [objects items side open-side]
   (reduce
    (fn [objects item]
@@ -80,7 +96,12 @@
        (let [id     (:id item)
              entity (side-entity item side open-side (get objects id))]
          (if (some? entity)
-           (assoc objects id entity)
+           (let [parent-id (:parent-id entity)]
+             (cond-> (assoc objects id entity)
+               (and (contains? objects parent-id)
+                    (not (contains? objects id))
+                    (not (some #(= id %) (get-in objects [parent-id :shapes]))))
+               (update-in [parent-id :shapes] (fnil conj []) id)))
            (let [parent-id (:parent-id (get objects id))]
              (cond-> (dissoc objects id)
                (contains? objects parent-id)
@@ -100,23 +121,47 @@
       (let [sr (:selrect shape)]
         {:x (:x sr 0) :y (:y sr 0) :width (:width sr 1) :height (:height sr 1)}))))
 
+(defn context-frame-id
+  "The nearest board that holds shape `id` in `objects`, the page root
+  left out: the stage an added or deleted layer is shown on."
+  [objects id]
+  (loop [parent-id (:parent-id (get objects id))
+         depth     0]
+    (let [parent (get objects parent-id)]
+      (cond
+        (or (nil? parent) (= parent-id uuid/zero) (>= depth 64)) nil
+        (= :frame (:type parent)) parent-id
+        :else (recur (:parent-id parent) (inc depth))))))
+
 (mf/defc shape-preview*
   "Renders `shape` to a fitted SVG with the real shape renderers. `objects`
-  supplies its children (a board or group draws them)."
+  supplies its children (a board or group draws them). `highlight`
+  (`{:id :status}`) outlines the layer an added or deleted change is
+  about, when `objects` holds it."
   {::mf/private true}
-  [{:keys [shape objects]}]
+  [{:keys [shape objects highlight]}]
   (let [{:keys [x y width height]} (shape-bounds shape)
         width   (max 1 width)
         height  (max 1 height)
         objects (mf/with-memo [shape objects]
                   (assoc (or objects {}) (:id shape) shape))
+        marked  (some->> (:id highlight) (get objects) shape-bounds)
         wrapper (mf/with-memo [objects] (render/shape-wrapper-factory objects))]
     [:svg {:class (stl/css :preview-svg)
            :viewBox (dm/str x " " y " " width " " height)
            :preserveAspectRatio "xMidYMid meet"
            :xmlns "http://www.w3.org/2000/svg"
            :fill "none"}
-     [:> wrapper {:shape shape}]]))
+     [:> wrapper {:shape shape}]
+     (when marked
+       [:rect {:class (stl/css-case :preview-highlight true
+                                    :is-added (= :added (:status highlight))
+                                    :is-deleted (= :deleted (:status highlight)))
+               :x (:x marked)
+               :y (:y marked)
+               :width (max 1 (:width marked))
+               :height (max 1 (:height marked))
+               :vector-effect "non-scaling-stroke"}])]))
 
 (mf/defc page-preview*
   "The whole page `objects` drawn to fit: every top-level layer, framed
@@ -192,8 +237,8 @@
   shapes render to SVG, library colors to a swatch, typographies to a text
   sample; everything else shows a type icon. A side with nothing to draw
   says why (`missing`: `{:reason :side :open-side}`, see
-  `missing-reason`)."
-  [{:keys [kind value objects missing]}]
+  `missing-reason`). A shape `highlight` is passed on to `shape-preview*`."
+  [{:keys [kind value objects missing highlight]}]
   (cond
     (nil? value)
     [:> missing-preview* {:reason (:reason missing :unavailable)
@@ -202,7 +247,7 @@
 
     (= kind :shape)
     (if (contains? renderable-shape-types (:type value))
-      [:> shape-preview* {:shape value :objects objects}]
+      [:> shape-preview* {:shape value :objects objects :highlight highlight}]
       [:div {:class (stl/css :preview-empty)}
        [:> i/icon* {:icon-id (brc/shape-icon-id {:shape-type (:type value)})}]])
 

frontend/src/app/main/ui/workspace/sidebar/branch_review/previews.scss

@@ -13,6 +13,21 @@
   block-size: 100%;
 }
 
+// the outline of the layer an added or deleted change is about, drawn
+// over the board that holds it; the stroke keeps its width at any zoom
+.preview-highlight {
+  --preview-highlight-color: var(--color-accent-success);
+
+  fill: color-mix(in srgb, var(--preview-highlight-color) 12%, transparent);
+  stroke: var(--preview-highlight-color);
+  stroke-width: 2;
+  stroke-dasharray: 4 3;
+
+  &.is-deleted {
+    --preview-highlight-color: var(--color-accent-error);
+  }
+}
+
 .preview-swatch {
   inline-size: 100%;
   block-size: 100%;

frontend/test/frontend_tests/runner.cljs

@@ -111,6 +111,7 @@
    [frontend-tests.tokens.token-errors-test]
    [frontend-tests.tokens.workspace-tokens-remap-test]
    [frontend-tests.ui.branch-review-common-test]
+   [frontend-tests.ui.branch-review-previews-test]
    [frontend-tests.ui.branches-compare-labels-test]
    [frontend-tests.ui.check-updates-test]
    [frontend-tests.ui.color-token-position-test]
@@ -270,6 +271,7 @@
    'frontend-tests.tokens.logic.tokens-status-test
    'frontend-tests.tokens.workspace-tokens-remap-test
    'frontend-tests.ui.branch-review-common-test
+   'frontend-tests.ui.branch-review-previews-test
    'frontend-tests.ui.branches-compare-labels-test
    'frontend-tests.ui.check-updates-test
    'frontend-tests.ui.color-token-position-test

frontend/test/frontend_tests/ui/branch_review_previews_test.cljs

@@ -0,0 +1,59 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS SUBSIDIARY SL
+
+(ns frontend-tests.ui.branch-review-previews-test
+  "How the compare previews rebuild the side the open file lacks: a
+  deleted layer comes back from the diff, inside the board it was in,
+  and a resized board draws at its own size."
+  (:require
+   [app.common.types.shape :as cts]
+   [app.common.uuid :as uuid]
+   [app.main.ui.workspace.sidebar.branch-review.previews :as brp]
+   [cljs.test :as t :include-macros true]))
+
+(def ^:private board-id (uuid/next))
+(def ^:private icon-id (uuid/next))
+(def ^:private path-id (uuid/next))
+
+(def ^:private branch-objects
+  "The branch, open: the button lost its icon and shrank."
+  {uuid/zero {:id uuid/zero :type :frame :shapes [board-id]}
+   board-id  (cts/setup-rect {:id board-id :type :frame :name "Button"
+                              :parent-id uuid/zero :x 0 :y 0 :width 131 :height 48
+                              :shapes []})})
+
+(def ^:private icon
+  {:id icon-id :type :group :name "icon" :parent-id board-id
+   :x 119 :y 12 :width 24 :height 24 :shapes [path-id]})
+
+(def ^:private items
+  [{:kind :shape :status :deleted :id path-id
+    :shape {:id path-id :type :path :name "path" :parent-id icon-id}}
+   {:kind :shape :status :deleted :id icon-id :shape icon}
+   {:kind :shape :status :modified :id board-id
+    :changed-attrs {:width {:main 163 :branch 131}}}])
+
+(t/deftest a-deleted-layer-comes-back-on-the-side-that-has-it
+  (let [main (brp/side-objects branch-objects items :main :branch)]
+    (t/is (= icon (get main icon-id)))
+    (t/is (contains? main path-id))
+    (t/is (= [icon-id] (get-in main [board-id :shapes])))))
+
+(t/deftest the-side-that-deleted-it-stays-without-it
+  (let [branch (brp/side-objects branch-objects items :branch :branch)]
+    (t/is (not (contains? branch icon-id)))
+    (t/is (= [] (get-in branch [board-id :shapes])))))
+
+(t/deftest a-resized-board-draws-at-its-own-size
+  (let [main (brp/side-objects branch-objects items :main :branch)]
+    (t/is (= 163 (get-in main [board-id :selrect :width])))
+    (t/is (= 131 (get-in branch-objects [board-id :selrect :width])))))
+
+(t/deftest a-layer-is-staged-in-the-nearest-board
+  (let [main (brp/side-objects branch-objects items :main :branch)]
+    (t/is (= board-id (brp/context-frame-id main path-id)))
+    (t/is (= board-id (brp/context-frame-id main icon-id)))
+    (t/is (nil? (brp/context-frame-id main board-id)))))