mirror of
https://github.com/penpot/penpot.git
synced 2026-11-05 17:40:39 +00:00
🐛 Fix Creating or combining a variant set renames the component first
This commit is contained in:
Pablo Alba
Pablo Alba
parent
eeda9c1d18
commit
7666d7d500
@ -38,6 +38,24 @@
|
||||
(pcb/update-shapes changes shape-ids #(assoc % :name stored-name))
|
||||
components)))
|
||||
|
||||
(defn generate-define-variant-container
|
||||
"One change vector. `name` is the string stored on the board.
|
||||
The component path and name are `split-group-name` of that string.
|
||||
Library data is refreshed before the first property is added."
|
||||
[changes variant-id main-id component-id name]
|
||||
(let [[path item-name] (cpn/split-group-name 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
|
||||
:path path
|
||||
:name item-name)
|
||||
{:apply-changes-local-library? true})
|
||||
(clvp/generate-add-new-property variant-id {:fill-values? true}))))
|
||||
|
||||
(defn generate-add-new-variant
|
||||
[changes shape variant-id new-component-id new-shape-id prop-num]
|
||||
(let [data (pcb/get-library-data changes)
|
||||
|
||||
@ -14,10 +14,12 @@
|
||||
[app.common.logic.variants :as clv]
|
||||
[app.common.path-names :as cpn]
|
||||
[app.common.test-helpers.components :as thc]
|
||||
[app.common.test-helpers.compositions :as tho]
|
||||
[app.common.test-helpers.files :as thf]
|
||||
[app.common.test-helpers.ids-map :as thi]
|
||||
[app.common.test-helpers.shapes :as ths]
|
||||
[app.common.test-helpers.variants :as thv]
|
||||
[app.common.types.component :as ctk]
|
||||
[app.common.types.variant :as ctv]
|
||||
[clojure.test :as t]
|
||||
[cuerdas.core :as str]))
|
||||
@ -566,6 +568,48 @@
|
||||
(t/is (= (ctv/properties-to-name props') (:variant-name main')))
|
||||
(t/is (= "active, crítica" (:variant-name main')))))
|
||||
|
||||
(defn- post-artboard-alpha-file
|
||||
"A normal frame holding one main named Alpha — before the container flag."
|
||||
[]
|
||||
(-> (thf/sample-file :file1)
|
||||
(tho/add-frame :v01 {:name "Board"})
|
||||
(tho/add-simple-component :c01 :m01 :r01
|
||||
:root-params {:parent-label :v01 :name "Alpha"})))
|
||||
|
||||
(t/deftest test-define-variant-container-renames-component-in-same-vector
|
||||
(let [file (post-artboard-alpha-file)
|
||||
page (thf/current-page file)
|
||||
variant-id (thi/id :v01)
|
||||
main-id (thi/id :m01)
|
||||
component-id (:id (thc/get-component file :c01))
|
||||
changes (-> (pcb/empty-changes nil (:id page))
|
||||
(pcb/with-objects (:objects page))
|
||||
(pcb/with-library-data (:data file))
|
||||
(clv/generate-define-variant-container
|
||||
variant-id main-id component-id "Component"))
|
||||
file' (thf/apply-changes file changes)
|
||||
container' (ths/get-shape file' :v01)
|
||||
main' (ths/get-shape file' :m01)
|
||||
comp' (thc/get-component file' :c01)
|
||||
restored (thf/apply-undo-changes file' changes)
|
||||
container-rest (ths/get-shape restored :v01)
|
||||
main-rest (ths/get-shape restored :m01)
|
||||
comp-rest (thc/get-component restored :c01)]
|
||||
|
||||
(t/is (ctk/is-variant-container? container'))
|
||||
(t/is (= "Component"
|
||||
(:name container')
|
||||
(:name main')
|
||||
(cpn/merge-path-item (:path comp') (:name comp'))))
|
||||
(t/is (= variant-id (:variant-id main') (:variant-id comp')))
|
||||
(t/is (= (ctv/properties-to-name (:variant-properties comp'))
|
||||
(:variant-name main')))
|
||||
(t/is (not (ctk/is-variant-container? container-rest)))
|
||||
(t/is (= "Alpha" (:name main-rest)))
|
||||
(t/is (= "Alpha" (cpn/merge-path-item (:path comp-rest) (:name comp-rest))))
|
||||
(t/is (nil? (:variant-id main-rest)))
|
||||
(t/is (nil? (:variant-id comp-rest)))))
|
||||
|
||||
(defn- rename-variant-changes
|
||||
[file name]
|
||||
(let [page (thf/current-page file)]
|
||||
|
||||
@ -304,21 +304,6 @@
|
||||
(dch/commit-changes changes)
|
||||
(dwu/commit-undo-transaction undo-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."
|
||||
@ -332,7 +317,8 @@
|
||||
changes (-> (pcb/empty-changes it page-id)
|
||||
(pcb/with-objects objects)
|
||||
(pcb/with-library-data data)
|
||||
(generate-define-variant-container variant-id main-id component-id name))]
|
||||
(clv/generate-define-variant-container
|
||||
variant-id main-id component-id name))]
|
||||
(rx/of (dch/commit-changes changes))))))
|
||||
|
||||
(defn- focus-property
|
||||
@ -340,7 +326,10 @@
|
||||
(ptk/reify ::focus-property
|
||||
ptk/EffectEvent
|
||||
(effect [_ _ _]
|
||||
(dom/focus! (dom/get-element (str "variant-prop-" variant-id "-0"))))))
|
||||
;; Unit tests run without a DOM; skip rather than throw into the store.
|
||||
(when (exists? js/document)
|
||||
(some-> (dom/get-element (str "variant-prop-" variant-id "-0"))
|
||||
(dom/focus!))))))
|
||||
|
||||
(defn- resposition-and-resize-variant
|
||||
"Resize the variant container, and move the shape (that is a variant) to the right"
|
||||
@ -468,9 +457,6 @@
|
||||
(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})
|
||||
|
||||
@ -8,6 +8,7 @@
|
||||
(:require
|
||||
[app.common.files.changes-builder :as pcb]
|
||||
[app.common.files.variant :as cfv]
|
||||
[app.common.logic.variants :as clv]
|
||||
[app.common.path-names :as cpn]
|
||||
[app.common.test-helpers.components :as cthc]
|
||||
[app.common.test-helpers.compositions :as tho]
|
||||
@ -181,6 +182,20 @@
|
||||
(tho/add-simple-component :c02 :m02 :r02
|
||||
:root-params {:name "Beta" :x 150 :y 0 :width 100 :height 80})))
|
||||
|
||||
(defn- two-mains-shared-prefix-file
|
||||
[]
|
||||
(-> (cthf/sample-file :file1)
|
||||
(tho/add-simple-component :c01 :m01 :r01
|
||||
:root-params {:name "UI / Alpha" :x 0 :y 0 :width 100 :height 80})
|
||||
(tho/add-simple-component :c02 :m02 :r02
|
||||
:root-params {:name "UI / Beta" :x 150 :y 0 :width 100 :height 80})))
|
||||
|
||||
(defn- path-named-main-file
|
||||
[]
|
||||
(-> (cthf/sample-file :file1)
|
||||
(tho/add-simple-component :c01 :m01 :r01
|
||||
:root-params {:name "Group / Button"})))
|
||||
|
||||
(defn- two-mains-on-page1-with-page2
|
||||
"Two mains on :page1; the open page is :page2 (Assets combine while elsewhere)."
|
||||
[]
|
||||
@ -201,6 +216,10 @@
|
||||
(tho/add-simple-component :c01 :m01 :r01
|
||||
:root-params {:parent-label :v01 :name "Set"})))
|
||||
|
||||
(defn- component-full-name
|
||||
[component]
|
||||
(cpn/merge-path-item (:path component) (:name component)))
|
||||
|
||||
(defn- page-objects
|
||||
[file]
|
||||
(:objects (cthf/current-page file)))
|
||||
@ -258,7 +277,7 @@
|
||||
changes (-> (pcb/empty-changes nil (:id page))
|
||||
(pcb/with-objects (:objects page))
|
||||
(pcb/with-library-data (:data file))
|
||||
(dwv/generate-define-variant-container
|
||||
(clv/generate-define-variant-container
|
||||
variant-id main-id component-id name))
|
||||
file' (cthf/apply-changes file changes)
|
||||
container' (cths/get-shape file' :v01)
|
||||
@ -271,7 +290,7 @@
|
||||
(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'))))))
|
||||
(:variant-name main')))))
|
||||
|
||||
(t/deftest transform-in-variant-validates-after-every-commit
|
||||
(t/async
|
||||
@ -320,6 +339,54 @@
|
||||
(t/is (= variant-id (:variant-id comp')))))
|
||||
after-commits-idle))))
|
||||
|
||||
(t/deftest transform-in-variant-path-name-validates-after-every-commit
|
||||
(t/async
|
||||
done
|
||||
(let [file (path-named-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]
|
||||
(let [ffile (:file snap)
|
||||
fmain (get (page-objects ffile) main-id)
|
||||
fcomp (cthc/get-component ffile :c01)]
|
||||
(t/is (= "Group / Button" (:name fmain)))
|
||||
(t/is (= "Group / Button" (component-full-name fcomp)))))
|
||||
(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 (= "Group" (:name fcont) (:name fmain) (component-full-name fcomp)))
|
||||
(t/is (= variant-id (:variant-id fmain) (:variant-id fcomp)))
|
||||
(t/is (= (ctv/properties-to-name (:variant-properties fcomp))
|
||||
(:variant-name fmain)))))
|
||||
(t/is (ctk/is-variant-container? container'))
|
||||
(t/is (= "Group" (:name container') (:name main') (component-full-name comp')))
|
||||
(t/is (= variant-id (:variant-id main') (:variant-id comp')))))
|
||||
after-commits-idle))))
|
||||
|
||||
(t/deftest add-new-variant-on-existing-set-still-validates
|
||||
(t/async
|
||||
done
|
||||
@ -363,19 +430,37 @@
|
||||
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))]
|
||||
(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))
|
||||
before-flag (take-while
|
||||
#(not (ctk/is-variant-container?
|
||||
(get (page-objects (:file %)) variant-id)))
|
||||
snaps)
|
||||
flagged (first-flagged snaps variant-id)]
|
||||
(assert-valid-snapshots snaps)
|
||||
(doseq [snap before-flag]
|
||||
(let [ffile (:file snap)
|
||||
fmain (get (page-objects ffile) id1)
|
||||
fcomp (cthc/get-component ffile :c01)]
|
||||
(t/is (= "Alpha" (:name fmain)))
|
||||
(t/is (= "Alpha" (component-full-name fcomp)))))
|
||||
(when flagged
|
||||
(let [ffile (:file flagged)
|
||||
fobjs (page-objects ffile)
|
||||
fcont (get fobjs variant-id)
|
||||
fmain (get fobjs id1)
|
||||
fcomp (cthc/get-component ffile :c01)]
|
||||
(t/is (= "Component" (:name fcont) (:name fmain) (component-full-name fcomp)))))
|
||||
(doseq [{:keys [file]} snaps]
|
||||
(let [objs (page-objects file)
|
||||
cont (get objs variant-id)
|
||||
@ -392,6 +477,53 @@
|
||||
(t/is (= variant-id (:parent-id m02')))))
|
||||
after-commits-idle))))
|
||||
|
||||
(t/deftest combine-as-variants-shared-prefix-validates-after-every-commit
|
||||
(t/async
|
||||
done
|
||||
(let [file (two-mains-shared-prefix-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)
|
||||
flagged (first-flagged snaps 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"))))
|
||||
(when flagged
|
||||
(let [ffile (:file flagged)
|
||||
fobjs (page-objects ffile)
|
||||
fcont (get fobjs variant-id)
|
||||
fmain (get fobjs id1)
|
||||
fcomp (cthc/get-component ffile :c01)]
|
||||
(t/is (= "UI" (:name fcont) (:name fmain) (component-full-name fcomp)))))
|
||||
(t/is (ctk/is-variant-container? container))
|
||||
(t/is (= "UI" (:name container) (:name m01') (:name m02')
|
||||
(component-full-name c01') (component-full-name c02')))
|
||||
(t/is (= variant-id (:variant-id m01') (:variant-id m02')
|
||||
(:variant-id c01') (:variant-id c02')))
|
||||
(t/is (= variant-id (:parent-id m01') (:parent-id m02')))))
|
||||
after-commits-idle))))
|
||||
|
||||
(t/deftest combine-as-variants-from-another-page-validates-after-every-commit
|
||||
(t/async
|
||||
done
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user