🐛 Fix Creating or combining a variant set saves half-finished steps

This commit is contained in:
Pablo Alba 2026-10-29 22:46:49 +01:00 committed by Pablo Alba
parent 69fa682217
commit d47f366c70
2 changed files with 392 additions and 106 deletions

View File

@ -304,22 +304,36 @@
(dch/commit-changes changes)
(dwu/commit-undo-transaction undo-id)))))))))
(defn- set-variant-id
"Sets the variant-id on a component"
[component-id variant-id]
(ptk/reify ::set-variant-id
(defn generate-define-variant-container
"One commit that turns an existing frame into a valid variant set:
container flag and name, the main's variant-id, the component's
variant-id (refreshing library data), then the first property."
[changes variant-id main-id component-id name]
(-> changes
(pcb/update-shapes [variant-id]
#(assoc % :is-variant-container true :name name))
(pcb/update-shapes [main-id]
#(assoc % :name name :variant-id variant-id))
(pcb/update-component component-id
#(assoc % :variant-id variant-id)
{:apply-changes-local-library? true})
(clvp/generate-add-new-property variant-id {:fill-values? true})))
(defn- define-variant-container
"Read page objects after the artboard exists, then commit the defining
variant fields in one change."
[variant-id main-id component-id name]
(ptk/reify ::define-variant-container
ptk/WatchEvent
(watch [it state _]
(let [page-id (:current-page-id state)
data (dsh/lookup-file-data state)
objects (dsh/lookup-page-objects state page-id)
changes (-> (pcb/empty-changes it page-id)
(pcb/with-objects objects)
(pcb/with-library-data data)
(pcb/update-component component-id #(assoc % :variant-id variant-id)))
undo-id (js/Symbol)]
(rx/of
(dwu/start-undo-transaction undo-id)
(dch/commit-changes changes)
(dwu/commit-undo-transaction undo-id))))))
(generate-define-variant-container variant-id main-id component-id name))]
(rx/of (dch/commit-changes changes))))))
(defn- focus-property
[variant-id]
@ -397,6 +411,95 @@
(->> (rx/of (focus-property (:id variant-container)))
(rx/delay 250))))))))))
(defn- transform-in-variant-stream
"Event stream that turns `main-instance-id` into a variant set.
The defining fields share one commit (`define-variant-container`).
Combine concatenates this stream before relocating the other mains.
`page-id` is the page that holds the main; nil means the open page."
[state main-instance-id variant-id delta prefix add-wrapper? duplicate? flex? page-id]
(let [variant-id (or variant-id (uuid/next))
variant-vec [variant-id]
file-id (:current-file-id state)
page-id (or page-id (:current-page-id state))
objects (dsh/lookup-page-objects state file-id page-id)
main (get objects main-instance-id)
parent (get objects (:parent-id main))
component-id (:component-id main)
name (if add-wrapper?
(str "Component/" (:name main))
(:name main))
;; If there is a prefix, set is as first item of path
cpath (-> name
cpn/split-path
(cond->
(seq prefix)
(->> (drop (count prefix))
(cons (cpn/join-path prefix))
vec)))
name (first cpath)
num-props (max 1 (dec (count cpath)))
style-props (cond-> {:r1 20
:r2 20
:r3 20
:r4 20
:layout-item-absolute true}
flex?
(into {:layout-item-h-sizing :auto
:layout-item-v-sizing :auto
:layout-padding {:p1 30 :p2 30 :p3 30 :p4 30}
:layout-gap {:row-gap 0 :column-gap 20}}))
stroke-props {:stroke-alignment :inner
:stroke-style :solid
:stroke-color "#bb97d8" ;; todo use color var?
:stroke-opacity 1
:stroke-width 2}
;; Move the position of the variant container so the main shape doesn't
;; change its position
delta (or delta
(if (ctsl/any-layout? parent)
(gpt/point 0 0)
(gpt/point -30 -30)))
undo-id (js/Symbol)]
(rx/concat
(rx/of
(dwu/start-undo-transaction undo-id)
(when (not= name (:name main))
(dwl/rename-component component-id name))
;; Create the frame while it is still a normal board
(dwsh/create-artboard-from-shapes [main-instance-id] variant-id nil nil nil delta flex?)
(cl/remove-all-fills variant-vec {:color clr/black :opacity 1})
(when flex? (dwsl/create-layout-from-id variant-id :flex))
(dwsh/update-shapes variant-vec #(merge % style-props))
(cl/add-stroke variant-vec stroke-props))
(rx/of (define-variant-container variant-id main-instance-id component-id name))
;; First property is in the defining commit
(when (> num-props 1)
(rx/from
(repeatedly (dec num-props)
#(add-new-property variant-id {:fill-values? true}))))
;; When the component has path, set the path items as properties values
(when (> (count cpath) 1)
(rx/from
(map
#(update-property-value component-id % (nth cpath (inc %)))
(range num-props))))
(rx/of
(when duplicate? (add-new-variant main-instance-id))
(dwsh/update-shapes variant-vec #(dissoc % :layout-item-absolute))
(dwu/commit-undo-transaction undo-id)
(when flex?
(ptk/data-event :layout/update {:ids [variant-id]}))))))
(defn transform-in-variant
"Given the id of a main shape of a component, creates a variant structure for
that component"
@ -406,98 +509,7 @@
(ptk/reify ::transform-in-variant
ptk/WatchEvent
(watch [_ state _]
(let [variant-id (or variant-id (uuid/next))
variant-vec [variant-id]
file-id (:current-file-id state)
page-id (:current-page-id state)
objects (dsh/lookup-page-objects state file-id page-id)
main (get objects main-instance-id)
parent (get objects (:parent-id main))
component-id (:component-id main)
name (if add-wrapper?
(str "Component/" (:name main))
(:name main))
;; If there is a prefix, set is as first item of path
cpath (-> name
cpn/split-path
(cond->
(seq prefix)
(->> (drop (count prefix))
(cons (cpn/join-path prefix))
vec)))
name (first cpath)
num-props (max 1 (dec (count cpath)))
base-props {:is-variant-container true
:name name
:r1 20
:r2 20
:r3 20
:r4 20
:layout-item-absolute true}
flex-props {:layout-item-h-sizing :auto
:layout-item-v-sizing :auto
:layout-padding {:p1 30 :p2 30 :p3 30 :p4 30}
:layout-gap {:row-gap 0 :column-gap 20}}
cont-props (if flex?
(into base-props flex-props)
base-props)
main-props {:name name
:variant-id variant-id}
stroke-props {:stroke-alignment :inner
:stroke-style :solid
:stroke-color "#bb97d8" ;; todo use color var?
:stroke-opacity 1
:stroke-width 2}
;; Move the position of the variant container so the main shape doesn't
;; change its position
delta (or delta
(if (ctsl/any-layout? parent)
(gpt/point 0 0)
(gpt/point -30 -30)))
undo-id (js/Symbol)]
;;TODO Refactor all called methods in order to be able to
;;generate changes instead of call the events
(rx/concat
(rx/of
(dwu/start-undo-transaction undo-id)
(when (not= name (:name main))
(dwl/rename-component component-id name))
;; Create variant container
(dwsh/create-artboard-from-shapes [main-instance-id] variant-id nil nil nil delta flex?)
(cl/remove-all-fills variant-vec {:color clr/black :opacity 1})
(when flex? (dwsl/create-layout-from-id variant-id :flex))
(dwsh/update-shapes variant-vec #(merge % cont-props))
(dwsh/update-shapes [main-instance-id] #(merge % main-props))
(cl/add-stroke variant-vec stroke-props)
(set-variant-id component-id variant-id))
;; Add the necessary number of new properties, with default values
(rx/from
(repeatedly num-props
#(add-new-property variant-id {:fill-values? true})))
;; When the component has path, set the path items as properties values
(when (> (count cpath) 1)
(rx/from
(map
#(update-property-value component-id % (nth cpath (inc %)))
(range num-props))))
(rx/of
(when duplicate? (add-new-variant main-instance-id))
(dwsh/update-shapes variant-vec #(dissoc % :layout-item-absolute))
(dwu/commit-undo-transaction undo-id)
(when flex?
(ptk/data-event :layout/update {:ids [variant-id]})))))))))
(transform-in-variant-stream state main-instance-id variant-id delta prefix add-wrapper? duplicate? flex? nil)))))
(defn add-component-or-variant
"Manage the shared shortcut, and do the pertinent action"
@ -696,9 +708,11 @@
(rx/of (dcm/go-to-workspace :page-id page-id))
(rx/empty))
(rx/of (dwu/start-undo-transaction undo-id)
(transform-in-variant (first ids) variant-id delta prefix add-wrapper? false false)
(dwsh/relocate-shapes (into #{} (rest ids)) variant-id 0)
(rx/of (dwu/start-undo-transaction undo-id))
(transform-in-variant-stream state (first ids) variant-id delta prefix add-wrapper? false false current-page)
(rx/of (dwsh/relocate-shapes (into #{} (rest ids)) variant-id 0)
;; relocate-shapes inserts the shapes in layer-tree
;; order, so restore the intended order afterwards
(order-variant-children variant-id ids)

View File

@ -6,16 +6,24 @@
(ns frontend-tests.data.workspace-variants-test
(:require
[app.common.files.changes-builder :as pcb]
[app.common.files.variant :as cfv]
[app.common.path-names :as cpn]
[app.common.test-helpers.components :as cthc]
[app.common.test-helpers.compositions :as tho]
[app.common.test-helpers.files :as cthf]
[app.common.test-helpers.ids-map :as cthi]
[app.common.test-helpers.shapes :as cths]
[app.common.test-helpers.variants :as thv]
[app.common.types.component :as ctk]
[app.common.types.variant :as ctv]
[app.common.uuid :as uuid]
[app.main.data.workspace :as dw]
[app.main.data.workspace.variants :as dwv]
[beicon.v2.core :as rx]
[cljs.test :as t :include-macros true]
[frontend-tests.helpers.state :as ths]))
[frontend-tests.helpers.state :as ths]
[potok.v2.core :as ptk]))
(t/use-fixtures :each
{:before cthi/reset-idmap!})
@ -152,3 +160,267 @@
(t/is (= clean-name (:name main2)))
(t/is (= clean-name (cpn/merge-path-item (:path comp1) (:name comp1))))
(t/is (= clean-name (cpn/merge-path-item (:path comp2) (:name comp2))))))))))
;; ---------------------------------------------------------------------------
;; Create-set / combine: every commit must be a valid file
;; ---------------------------------------------------------------------------
(def ^:private apply-local?
(ptk/type? :app.main.data.changes/apply-changes-localy))
(defn- simple-main-file
[]
(-> (cthf/sample-file :file1)
(tho/add-simple-component :c01 :m01 :r01)))
(defn- two-mains-file
[]
(-> (cthf/sample-file :file1)
(tho/add-simple-component :c01 :m01 :r01
:root-params {:name "Alpha" :x 0 :y 0 :width 100 :height 80})
(tho/add-simple-component :c02 :m02 :r02
:root-params {:name "Beta" :x 150 :y 0 :width 100 :height 80})))
(defn- two-mains-on-page1-with-page2
"Two mains on :page1; the open page is :page2 (Assets combine while elsewhere)."
[]
(-> (cthf/sample-file :file1 :page-label :page1)
(tho/add-simple-component :c01 :m01 :r01
:root-params {:name "Alpha" :x 0 :y 0 :width 100 :height 80})
(tho/add-simple-component :c02 :m02 :r02
:root-params {:name "Beta" :x 150 :y 0 :width 100 :height 80})
(cthf/add-sample-page :page2)
(cthf/switch-to-page :page2)))
(defn- post-artboard-file
"A normal frame holding one main — the state after create-artboard-from-shapes
and before the container flag."
[]
(-> (cthf/sample-file :file1)
(tho/add-frame :v01 {:name "Set"})
(tho/add-simple-component :c01 :m01 :r01
:root-params {:parent-label :v01 :name "Set"})))
(defn- page-objects
[file]
(:objects (cthf/current-page file)))
(defn- page-objects-on
[file page-label]
(:objects (cthf/get-page file page-label)))
(defn- commit-error
[file]
(try
(cthf/validate-file! file)
nil
(catch :default cause
(or (ex-message cause) (str cause)))))
(defn- watch-applied-commits
"Record the file after every local apply. Keep the subscription for the
whole run-store; assertions run in the completed callback."
[store snapshots]
(->> (ptk/input-stream store)
(rx/filter apply-local?)
(rx/subs! (fn [_]
(let [file (ths/get-file-from-state @store)]
(swap! snapshots conj {:file file
:error (commit-error file)}))))))
(defn- after-commits-idle
"Stop once applied commits have been idle. Combine schedules an async
undo commit after the main stream."
[stream]
(->> stream
(rx/filter apply-local?)
(rx/debounce 100)))
(defn- assert-valid-snapshots
[snapshots]
(doseq [{:keys [error]} snapshots]
(t/is (nil? error) error)))
(defn- first-flagged
[snapshots container-id]
(first (filter #(ctk/is-variant-container?
(get (page-objects (:file %)) container-id))
snapshots)))
(t/deftest define-variant-container-commit-validates
(let [file (post-artboard-file)
page (cthf/current-page file)
variant-id (cthi/id :v01)
main-id (cthi/id :m01)
component (cthc/get-component file :c01)
component-id (:id component)
name (:name (cths/get-shape file :v01))
changes (-> (pcb/empty-changes nil (:id page))
(pcb/with-objects (:objects page))
(pcb/with-library-data (:data file))
(dwv/generate-define-variant-container
variant-id main-id component-id name))
file' (cthf/apply-changes file changes)
container' (cths/get-shape file' :v01)
main' (cths/get-shape file' :m01)
comp' (cthc/get-component file' :c01)]
(t/is (ctk/is-variant-container? container'))
(t/is (= [main-id] (:shapes container')))
(t/is (= variant-id (:variant-id main')))
(t/is (= variant-id (:variant-id comp')))
(t/is (some? (:variant-id comp')))
(t/is (= (ctv/properties-to-name (:variant-properties comp'))
(:variant-name main'))))))
(t/deftest transform-in-variant-validates-after-every-commit
(t/async
done
(let [file (simple-main-file)
main-id (cthi/id :m01)
variant-id (uuid/next)
snapshots (atom [])
store (ths/setup-store file)]
(watch-applied-commits store snapshots)
(ths/run-store
store
done
[(dwv/transform-in-variant main-id variant-id nil [] false true true)]
(fn [state]
(let [snaps @snapshots
flagged (first-flagged snaps variant-id)
file' (ths/get-file-from-state state)
objects' (page-objects file')
container' (get objects' variant-id)
main' (get objects' main-id)
comp' (cthc/get-component file' :c01)
before-flag (take-while
#(not (ctk/is-variant-container?
(get (page-objects (:file %)) variant-id)))
snaps)]
(assert-valid-snapshots snaps)
(t/is (some? flagged) "a commit must set :is-variant-container")
(doseq [snap before-flag]
(t/is (not (ctk/is-variant-container?
(get (page-objects (:file snap)) variant-id)))))
(when flagged
(let [ffile (:file flagged)
fobjs (page-objects ffile)
fcont (get fobjs variant-id)
fmain (get fobjs main-id)
fcomp (cthc/get-component ffile :c01)]
(t/is (= [main-id] (:shapes fcont)))
(t/is (ctk/main-instance? fmain))
(t/is (= variant-id (:variant-id fmain)))
(t/is (= variant-id (:variant-id fcomp)))
(t/is (= (ctv/properties-to-name (:variant-properties fcomp))
(:variant-name fmain)))))
(t/is (ctk/is-variant-container? container'))
(t/is (= variant-id (:variant-id main')))
(t/is (= variant-id (:variant-id comp')))))
after-commits-idle))))
(t/deftest add-new-variant-on-existing-set-still-validates
(t/async
done
(let [file (-> (two-property-file-on-other-page)
(cthf/switch-to-page :page1))
main-id (cthi/id :m01)
c01-before (cthc/get-component file :c01)
c02-before (cthc/get-component file :c02)
last-c01 (-> c01-before :variant-properties last)
last-c02 (-> c02-before :variant-properties last)]
(run file
[(dwv/add-new-variant main-id)]
done
(fn [state]
(let [file' (ths/get-file-from-state state)
container' (cths/get-shape file' :v01 :page-label :page1)
c01' (cthc/get-component file' :c01)
c02' (cthc/get-component file' :c02)
new-id (first (:shapes container'))
new-shape (cths/get-shape-by-id file' new-id :page-label :page1)
new-comp (cthc/get-component-by-id file' (:component-id new-shape))]
(t/is (= 3 (count (:shapes container'))))
(t/is (= last-c01 (-> c01' :variant-properties last)))
(t/is (= last-c02 (-> c02' :variant-properties last)))
(t/is (not= last-c01 (-> new-comp :variant-properties last)))
(t/is (= (ctv/properties-to-name (:variant-properties new-comp))
(:variant-name new-shape)))))))))
(t/deftest combine-as-variants-validates-after-every-commit
(t/async
done
(let [file (two-mains-file)
id1 (cthi/id :m01)
id2 (cthi/id :m02)
variant-id (uuid/next)
snapshots (atom [])
store (ths/setup-store file)]
(watch-applied-commits store snapshots)
(ths/run-store
store
done
[(dwv/combine-as-variants [id1 id2] {:variant-id variant-id :trigger "test"})]
(fn [state]
(let [snaps @snapshots
file' (ths/get-file-from-state state)
objects (page-objects file')
container (get objects variant-id)
m01' (get objects id1)
m02' (get objects id2)
c01' (cthc/get-component file' :c01)
c02' (cthc/get-component file' :c02)
names1 (mapv :name (:variant-properties c01'))
names2 (mapv :name (:variant-properties c02'))
ordered (mapv :main-instance-id
(cfv/find-variant-components (:data file') objects variant-id))]
(assert-valid-snapshots snaps)
(doseq [{:keys [file]} snaps]
(let [objs (page-objects file)
cont (get objs variant-id)
kids (set (:shapes cont))]
(when (contains? kids id2)
(t/is (ctk/is-variant-container? cont)
"other mains must not enter the frame before it is a variant container"))))
(t/is (ctk/is-variant-container? container))
(t/is (= names1 names2))
(t/is (= [id1 id2] ordered))
(t/is (= variant-id (:variant-id m01')))
(t/is (= variant-id (:variant-id m02')))
(t/is (= variant-id (:parent-id m01')))
(t/is (= variant-id (:parent-id m02')))))
after-commits-idle))))
(t/deftest combine-as-variants-from-another-page-validates-after-every-commit
(t/async
done
(let [file (two-mains-on-page1-with-page2)
id1 (cthi/id :m01)
id2 (cthi/id :m02)
page1 (cthi/id :page1)
variant-id (uuid/next)
snapshots (atom [])
store (ths/setup-store file)]
(watch-applied-commits store snapshots)
(ths/run-store
store
done
[(dwv/combine-as-variants [id1 id2] {:page-id page1 :variant-id variant-id :trigger "test"})
(dw/initialize-page (:id file) page1)]
(fn [state]
(let [snaps @snapshots
file' (ths/get-file-from-state state)
objects (page-objects-on file' :page1)
container (get objects variant-id)
m01' (get objects id1)
m02' (get objects id2)
c01' (cthc/get-component file' :c01)
c02' (cthc/get-component file' :c02)]
(assert-valid-snapshots snaps)
(t/is (ctk/is-variant-container? container))
(t/is (= variant-id (:variant-id m01')))
(t/is (= variant-id (:variant-id m02')))
(t/is (= variant-id (:variant-id c01')))
(t/is (= variant-id (:variant-id c02')))))
after-commits-idle))))