diff --git a/common/src/app/common/files/branch_merge.cljc b/common/src/app/common/files/branch_merge.cljc index fc0d34ffe1..234dda71d6 100644 --- a/common/src/app/common/files/branch_merge.cljc +++ b/common/src/app/common/files/branch_merge.cljc @@ -783,11 +783,22 @@ (sub-of ours pid :guides) {:kind :page-guide :page-id pid})) common) + ;; an added or deleted flow carries itself whole as `:flow`, as + ;; for shapes: the compare panel names the board it starts on, + ;; which the open file may lack flows-diffs (map (fn [pid] - (three-way-entities (sub-of base pid :flows) - (sub-of theirs pid :flows) - (sub-of ours pid :flows) - {:kind :page-flow :page-id pid})) + (let [bf (sub-of base pid :flows) + of (sub-of ours pid :flows)] + (update (three-way-entities bf + (sub-of theirs pid :flows) + of + {:kind :page-flow :page-id pid}) + :changes + (partial mapv (fn [e] + (case (:status e) + :added (assoc e :flow (get of (:id e))) + :deleted (assoc e :flow (get bf (:id e))) + e)))))) common) ;; default-grids per common page (mergeable: :page-grid -> :set-default-grid) grids-diffs (map (fn [pid] diff --git a/common/test/common_tests/files_branch_merge_test.cljc b/common/test/common_tests/files_branch_merge_test.cljc index cbb430f593..d3bd965f1a 100644 --- a/common/test/common_tests/files_branch_merge_test.cljc +++ b/common/test/common_tests/files_branch_merge_test.cljc @@ -170,6 +170,20 @@ (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 a-deleted-flow-carries-the-board-it-starts-on + (let [flow {:id :f1 :name "Onboarding" :starting-frame :b1} + base (assoc-in (mkdata {:b1 {:id :b1 :name "Home"}}) + [:pages-index :p1 :flows] {:f1 flow}) + branch (assoc-in base [:pages-index :p1 :flows] {}) + merge (first (filter #(= :page-flow (:kind %)) + (:changes (bm/compute-merge base base branch :branch->main)))) + update (first (filter #(= :page-flow (:kind %)) + (:changes (bm/compute-merge base branch base :main->branch))))] + (t/is (= :deleted (:status merge))) + (t/is (= flow (:flow merge))) + (t/is (= :deleted (:status update))) + (t/is (= flow (:flow update))))) + (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"}}) diff --git a/frontend/src/app/main/ui/workspace/sidebar/branch_review/common.cljs b/frontend/src/app/main/ui/workspace/sidebar/branch_review/common.cljs index e3c0b021cf..b3b5b67538 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/branch_review/common.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/branch_review/common.cljs @@ -420,6 +420,100 @@ {} indexed))) +;; --- What goes with an added or deleted layer + +(defn- lost-interactions + "The interactions of modified entry `entry` that lead into `ids` on the + `from` side and are gone on the `to` side." + [entry ids from to] + (let [values (get-in entry [:changed-attrs :interactions]) + kept (set (get values to))] + (filterv #(and (contains? ids (:destination %)) (not (contains? kept %))) + (get values from)))) + +(defn- size-of + "Width and height of `shape` on `side`, taking the changed ones from + diff entry `entry`." + [shape entry side] + (let [attrs (:changed-attrs entry)] + [(get-in attrs [:width side] (:width shape)) + (get-in attrs [:height side] (:height shape))])) + +(defn shape-effects + "What else changes with the added or deleted layer `item`, read from + the other entries of the diff (`items`): + + `:interactions` the layers whose prototype interactions led into it + and go with it (`{:id :label :count}`); + `:flows` the flows that started on it and go with it (names); + `:layouts` the boards with a layout around it that resize + (`{:id :name :layout :from [w h] :to [w h]}`) or + whose other layers move (`{... :moved n}`). + + Penpot makes these changes itself when a layer goes, so the diff lists + them as entries of their own; this ties them to their cause. `objects` + is the page as the side that holds the layer has it, `inner` the + layers folded under it (`fold-cascades`), and `incoming` says the + changes are main's, going to the branch, rather than the branch's + going to main." + [item items objects inner incoming] + (let [from (if incoming :branch :main) + to (if incoming :main :branch) + deleted? (= :deleted (:status item)) + ids (into #{(:id item)} (map (comp :id second)) inner) + page-id (:page-id item) + on-page (filterv #(= page-id (:page-id %)) items) + modified (into {} + (comp (filter #(and (= :shape (:kind %)) (= :modified (:status %)))) + (map (juxt :id identity))) + on-page) + ancestors (loop [id (:parent-id (get objects (:id item))) out [] depth 0] + (let [shape (get objects id)] + (if (and shape (not= id uuid/zero) (< depth 64)) + (recur (:parent-id shape) (conj out shape) (inc depth)) + out)))] + {:interactions + (if deleted? + (into [] + (keep (fn [[id entry]] + (when-not (contains? ids id) + (let [lost (lost-interactions entry ids from to)] + (when (seq lost) + {:id id :label (:label entry) :count (count lost)}))))) + modified) + []) + + :flows + (if deleted? + (into [] + (keep (fn [entry] + (when (and (= :page-flow (:kind entry)) + (= :deleted (:status entry)) + (contains? ids (get-in entry [:flow :starting-frame]))) + (or (get-in entry [:flow :name]) (:label entry))))) + on-page) + []) + + :layouts + (into [] + (keep (fn [{:keys [id layout] :as shape}] + (when (some? layout) + (let [entry (get modified id) + before (size-of shape entry from) + after (size-of shape entry to) + moved (count (filter (fn [[sid e]] + (and (= id (:parent-id (get objects sid))) + (not (contains? ids sid)) + (some #(contains? (:changed-attrs e) %) [:x :y]))) + modified))] + (cond + (not= before after) + {:id id :name (:name shape) :layout layout :from before :to after} + + (pos? moved) + {:id id :name (:name shape) :layout layout :moved moved}))))) + ancestors)})) + (defn group-by-place "Group the clean changes (`[index item]` pairs) by page, and inside a page by the top-level board that holds them, reading names from the diff --git a/frontend/src/app/main/ui/workspace/sidebar/branch_review/facts.cljs b/frontend/src/app/main/ui/workspace/sidebar/branch_review/facts.cljs index 1cf322ddfe..035f2473a8 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/branch_review/facts.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/branch_review/facts.cljs @@ -32,13 +32,22 @@ "workspace.branches.review.side.main" "workspace.branches.review.side.branch"))) +(defn- names-line + "`names` joined, the first `max-names` of them and a count of the rest." + [names] + (dm/str (str/join ", " (take max-names names)) + (when (> (count names) max-names) + (dm/str " " (tr "workspace.branches.review.summary.more" + (i18n/c (- (count names) max-names))))))) + (mf/defc fact-list* "Label and value rows, one per fact." {::mf/private true} [{:keys [rows]}] [:div {:class (stl/css :facts)} - (for [[label value] (remove nil? rows)] - [:div {:class (stl/css :fact) :key label} + ;; keyed by place: a label may repeat (a layout row per board) + (for [[idx [label value]] (map-indexed vector (remove nil? rows))] + [:div {:class (stl/css :fact) :key idx} [:> text* {:as "span" :typography t/body-small :class (stl/css :fact-label)} label] [:div {:class (stl/css :fact-value)} value]])]) @@ -74,10 +83,7 @@ (tr "workspace.branches.review.facts.layers" (i18n/c layers))))] (when (seq names) [(tr "workspace.branches.review.facts.board-names") - (dm/str (str/join ", " (take max-names names)) - (when (> (count names) max-names) - (dm/str " " (tr "workspace.branches.review.summary.more" - (i18n/c (- (count names) max-names))))))])]}] + (names-line names)])]}] (when (and deleted? (pos? layers)) [:> brl/notice* {:level :warning :title (tr "workspace.branches.review.facts.page-goes")} @@ -134,6 +140,57 @@ component (conj [(tr "workspace.branches.compare.attr.component") component])))) +;; --- What goes with a layer + +(defn- size-line + [[w h]] + (dm/str (brc/display-val w) " × " (brc/display-val h))) + +(defn- layout-row + "A board with a layout around the layer: how it resizes, or how many + of its other layers move." + [{:keys [name layout from to moved]}] + [(tr "workspace.branches.review.category.layout") + (dm/str name " (" (tr (if (= :grid layout) "labels.grid-layout" "labels.flex-layout")) ") - " + (if moved + (tr "workspace.branches.review.effects.moved" (i18n/c moved)) + (dm/str (size-line from) " → " (size-line to))))]) + +(mf/defc effects* + "What else changes with an added or deleted layer (`brc/shape-effects`): + the prototype links and flows that go with it first, as warnings, then + the layouts around it that adapt and the layers it holds. Nothing + shows when none of it applies." + {::mf/private true} + [{:keys [item items objects inner incoming]}] + (let [{:keys [interactions flows layouts]} + (mf/with-memo [item items objects inner incoming] + (brc/shape-effects item items objects inner incoming)) + names (mapv (comp :label second) inner) + deleted? (= :deleted (:status item)) + rows (cond-> (mapv layout-row layouts) + (seq names) + (conj [(tr "workspace.branches.review.facts.contents") (names-line names)]))] + (when (or (seq interactions) (seq flows) (seq rows)) + [:div {:class (stl/css :section)} + [:> section-title* {} + (tr (if deleted? + "workspace.branches.review.effects.title-deleted" + "workspace.branches.review.effects.title-added"))] + (when (seq interactions) + [:> brl/notice* {:level :warning + :title (tr "workspace.branches.review.effects.interactions" + (i18n/c (reduce + (map :count interactions))))} + (tr "workspace.branches.review.effects.interactions-from" + (names-line (map :label interactions)))]) + (when (seq flows) + [:> brl/notice* {:level :warning + :title (tr "workspace.branches.review.effects.flows" + (i18n/c (count flows)))} + (names-line flows)]) + (when (seq rows) + [:> fact-list* {:rows rows}])]))) + (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. `inner` are the @@ -162,18 +219,16 @@ "workspace.branches.review.facts.where-was" "workspace.branches.review.facts.where-lands")) (str/join " › " (cons page (brc/shape-path objects (:id item))))]] - (cond-> (shape-rows shape data type-lbl) - (seq inner) - (conj [(tr "workspace.branches.review.facts.contents") - (tr "workspace.branches.review.facts.layers" - (i18n/c (count inner)))])))}] + (shape-rows shape data type-lbl))}] (when (and copies (pos? copies)) [:> brl/notice* {:level :warning :title (tr "workspace.branches.review.facts.copies-orphaned-title")} (tr (if incoming "workspace.branches.review.facts.copies-orphaned-update" "workspace.branches.review.facts.copies-orphaned-merge") - (i18n/c copies))])]))) + (i18n/c copies))]) + [:> effects* {:item item :items items :objects objects + :inner inner :incoming incoming}]]))) ;; --- Library entries other layers depend on diff --git a/frontend/test/frontend_tests/ui/branch_review_common_test.cljs b/frontend/test/frontend_tests/ui/branch_review_common_test.cljs index 710280750d..fbfeb60a9c 100644 --- a/frontend/test/frontend_tests/ui/branch_review_common_test.cljs +++ b/frontend/test/frontend_tests/ui/branch_review_common_test.cljs @@ -168,3 +168,74 @@ [3 (assoc (deleted :other :icon) :page-id :elsewhere)]]] (t/is (= {} (brc/fold-cascades rows)) "an addition, a layer whose parent stays and one on another page stay apart"))) + +;; --- What goes with an added or deleted layer + +(def ^:private home-id (uuid/next)) +(def ^:private screen-id (uuid/next)) +(def ^:private cta-id (uuid/next)) +(def ^:private button-id (uuid/next)) +(def ^:private icon-id (uuid/next)) +(def ^:private label-id (uuid/next)) + +(def ^:private proto-objects + "Main's page: a button with a flex layout on Home, and a Screen the + CTA leads to." + {uuid/zero {:id uuid/zero :type :frame :shapes [home-id screen-id]} + home-id {:id home-id :type :frame :name "Home" :parent-id uuid/zero} + screen-id {:id screen-id :type :frame :name "Screen" :parent-id uuid/zero} + cta-id {:id cta-id :type :rect :name "CTA" :parent-id home-id} + button-id {:id button-id :type :frame :name "Button" :parent-id home-id + :layout :flex :width 163 :height 48} + icon-id {:id icon-id :type :group :name "icon" :parent-id button-id} + label-id {:id label-id :type :text :name "label" :parent-id button-id}}) + +(defn- gone + [id] + {:kind :shape :status :deleted :id id :page-id page-id}) + +(defn- edit + [id attrs] + {:kind :shape :status :modified :id id :page-id page-id :label (name id) + :changed-attrs attrs}) + +(t/deftest a-board-with-a-layout-resizes-with-the-layer + (let [items [(gone icon-id) + (edit button-id {:width {:main 163 :branch 131}})] + fx (brc/shape-effects (gone icon-id) items proto-objects [] false)] + (t/is (= [{:id button-id :name "Button" :layout :flex :from [163 48] :to [131 48]}] + (:layouts fx))) + (t/is (= [] (:interactions fx))) + (t/is (= [] (:flows fx))))) + +(t/deftest an-update-reads-the-sides-the-other-way-round + (let [items [(gone icon-id) + (edit button-id {:width {:main 131 :branch 163}})] + fx (brc/shape-effects (gone icon-id) items proto-objects [] true)] + (t/is (= [[163 48] [131 48]] ((juxt :from :to) (first (:layouts fx))))))) + +(t/deftest a-fixed-board-moves-its-other-layers + (let [items [(gone icon-id) + (edit label-id {:x {:main 40 :branch 20}})] + fx (brc/shape-effects (gone icon-id) items proto-objects [] false)] + (t/is (= [{:id button-id :name "Button" :layout :flex :moved 1}] (:layouts fx))))) + +(t/deftest prototype-links-into-a-deleted-board-go-with-it + (let [link {:event-type :click :action-type :navigate :destination screen-id} + items [(gone screen-id) + (assoc (edit cta-id {:interactions {:main [link] :branch []}}) :label "CTA") + {:kind :page-flow :status :deleted :id :f1 :page-id page-id :label "f1" + :flow {:id :f1 :name "Onboarding" :starting-frame screen-id}} + {:kind :page-flow :status :deleted :id :f2 :page-id page-id :label "f2" + :flow {:id :f2 :name "Other" :starting-frame home-id}}] + fx (brc/shape-effects (gone screen-id) items proto-objects [] false)] + (t/is (= [{:id cta-id :label "CTA" :count 1}] (:interactions fx))) + (t/is (= ["Onboarding"] (:flows fx))) + (t/is (= [] (:layouts fx)) "the page root is not a layout"))) + +(t/deftest an-added-layer-breaks-no-links + (let [link {:event-type :click :action-type :navigate :destination screen-id} + added (assoc (gone screen-id) :status :added) + items [added (edit cta-id {:interactions {:main [] :branch [link]}})] + fx (brc/shape-effects added items proto-objects [] false)] + (t/is (= [] (:interactions fx))))) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 6aae6ad1c5..f8281f58c9 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -11536,6 +11536,36 @@ msgid_plural "workspace.branches.review.facts.deleted-used" msgstr[0] "1 layer in this file loses its link to it." msgstr[1] "%s layers in this file lose their link to it." +#: src/app/main/ui/workspace/sidebar/branch_review/facts.cljs +msgid "workspace.branches.review.effects.title-deleted" +msgstr "What goes with it" + +#: src/app/main/ui/workspace/sidebar/branch_review/facts.cljs +msgid "workspace.branches.review.effects.title-added" +msgstr "What comes with it" + +#: src/app/main/ui/workspace/sidebar/branch_review/facts.cljs +msgid "workspace.branches.review.effects.interactions" +msgid_plural "workspace.branches.review.effects.interactions" +msgstr[0] "1 prototype interaction leads here and goes too" +msgstr[1] "%s prototype interactions lead here and go too" + +#: src/app/main/ui/workspace/sidebar/branch_review/facts.cljs +msgid "workspace.branches.review.effects.interactions-from" +msgstr "On %s" + +#: src/app/main/ui/workspace/sidebar/branch_review/facts.cljs +msgid "workspace.branches.review.effects.flows" +msgid_plural "workspace.branches.review.effects.flows" +msgstr[0] "1 prototype flow starts here and goes too" +msgstr[1] "%s prototype flows start here and go too" + +#: src/app/main/ui/workspace/sidebar/branch_review/facts.cljs +msgid "workspace.branches.review.effects.moved" +msgid_plural "workspace.branches.review.effects.moved" +msgstr[0] "1 other layer moves" +msgstr[1] "%s other layers move" + #: src/app/main/ui/workspace/sidebar/branch_review/common.cljs msgid "workspace.branches.review.group.pages" msgstr "Pages"