mirror of
https://github.com/penpot/penpot.git
synced 2026-05-30 12:18:13 +00:00
🐛 Token remap preserves child component sync after renaming a token group (#9566)
* 🐛 Token remap preserves child component sync after renaming a token group * 📚 Do not update CHANGES.md We are changing the procedures to not update the changelog on each PR. Instead, we use github tracking to check what issues come in a release, and update the changelog automatically in a batch. Signed-off-by: Andrés Moya <andres.moya@kaleidos.net> --------- Signed-off-by: Andrés Moya <andres.moya@kaleidos.net> Co-authored-by: Andrés Moya <andres.moya@kaleidos.net>
This commit is contained in:
Renzo
GitHub
parent
04d4abc766
commit
02ab41f420
@ -92,62 +92,66 @@
|
||||
;; Token Remapping Core Logic
|
||||
;; ==========================
|
||||
|
||||
(defn build-remap-changes
|
||||
"Build the pending changes required to rename a token from `old-token-name`
|
||||
to `new-token-name`, covering applied-token references on shapes and alias
|
||||
references in other tokens. Updates to copy shapes are committed with
|
||||
`:ignore-touched true` so the rename does not flip sync groups into the
|
||||
:touched set and silently break later main→copy propagation."
|
||||
[file-data old-token-name new-token-name]
|
||||
(let [scan-results (scan-workspace-token-references file-data old-token-name)
|
||||
tokens-lib (:tokens-lib file-data)
|
||||
sets (ctob/get-sets tokens-lib)
|
||||
tokens-with-sets (mapcat (fn [set]
|
||||
(map (fn [token]
|
||||
{:token token :set set})
|
||||
(vals (ctob/get-tokens tokens-lib (ctob/get-id set)))))
|
||||
sets)
|
||||
|
||||
refs-by-container (group-by :container (:applied-tokens scan-results))
|
||||
|
||||
shape-changes
|
||||
(reduce-kv
|
||||
(fn [changes container refs]
|
||||
(let [shape-ids (map :shape-id refs)
|
||||
token (or (some #(when (= (:name (:token %)) new-token-name) %) tokens-with-sets)
|
||||
(some #(when (= (:name (:token %)) old-token-name) %) tokens-with-sets))
|
||||
attributes (set (map :attribute refs))]
|
||||
(if token
|
||||
(-> (pcb/with-container changes container)
|
||||
(pcb/update-shapes shape-ids
|
||||
(fn [shape]
|
||||
(update shape :applied-tokens
|
||||
#(merge % (cft/attributes-map attributes (:token token)))))
|
||||
{:ignore-touched true}))
|
||||
changes)))
|
||||
(-> (pcb/empty-changes)
|
||||
(pcb/with-file-data file-data)
|
||||
(pcb/with-library-data file-data))
|
||||
refs-by-container)]
|
||||
|
||||
(reduce
|
||||
(fn [changes ref]
|
||||
(let [source-token-id (:source-token-id ref)]
|
||||
(when-let [{:keys [token set]} (some #(when (= (:id (:token %)) source-token-id) %) tokens-with-sets)]
|
||||
(let [old-value (:value token)
|
||||
new-value (cto/update-token-value-references old-value old-token-name new-token-name)]
|
||||
(pcb/set-token changes (ctob/get-id set) (:id token)
|
||||
(assoc token :value new-value))))))
|
||||
shape-changes
|
||||
(:token-aliases scan-results))))
|
||||
|
||||
(defn remap-tokens
|
||||
"Main function to remap all token references when a token name changes"
|
||||
[old-token-name new-token-name]
|
||||
(ptk/reify ::remap-tokens
|
||||
ptk/WatchEvent
|
||||
(watch [_ state _]
|
||||
(let [file-data (dh/lookup-file-data state)
|
||||
scan-results (scan-workspace-token-references file-data old-token-name)
|
||||
tokens-lib (:tokens-lib file-data)
|
||||
sets (ctob/get-sets tokens-lib)
|
||||
tokens-with-sets (mapcat (fn [set]
|
||||
(map (fn [token]
|
||||
{:token token :set set})
|
||||
(vals (ctob/get-tokens tokens-lib (ctob/get-id set)))))
|
||||
sets)
|
||||
|
||||
;; Group applied token references by container
|
||||
refs-by-container (group-by :container (:applied-tokens scan-results))
|
||||
|
||||
;; Use apply-token logic to update shapes for both direct and alias references
|
||||
shape-changes (reduce-kv
|
||||
(fn [changes container refs]
|
||||
(let [shape-ids (map :shape-id refs)
|
||||
;; Find the correct token to apply (new or alias)
|
||||
token (or (some #(when (= (:name (:token %)) new-token-name) %) tokens-with-sets)
|
||||
(some #(when (= (:name (:token %)) old-token-name) %) tokens-with-sets))
|
||||
attributes (set (map :attribute refs))]
|
||||
(if token
|
||||
(-> (pcb/with-container changes container)
|
||||
(pcb/update-shapes shape-ids
|
||||
(fn [shape]
|
||||
(update shape :applied-tokens
|
||||
#(merge % (cft/attributes-map attributes (:token token)))))))
|
||||
changes)))
|
||||
(-> (pcb/empty-changes)
|
||||
(pcb/with-file-data file-data)
|
||||
(pcb/with-library-data file-data))
|
||||
refs-by-container)
|
||||
|
||||
;; Create changes for updating token alias references
|
||||
token-changes (reduce
|
||||
(fn [changes ref]
|
||||
(let [source-token-id (:source-token-id ref)]
|
||||
(when-let [{:keys [token set]} (some #(when (= (:id (:token %)) source-token-id) %) tokens-with-sets)]
|
||||
(let [old-value (:value token)
|
||||
new-value (cto/update-token-value-references old-value old-token-name new-token-name)]
|
||||
(pcb/set-token changes (ctob/get-id set) (:id token)
|
||||
(assoc token :value new-value))))))
|
||||
shape-changes
|
||||
(:token-aliases scan-results))]
|
||||
|
||||
(let [file-data (dh/lookup-file-data state)
|
||||
token-changes (build-remap-changes file-data old-token-name new-token-name)]
|
||||
(log/info :hint "token-remapping"
|
||||
:old-name old-token-name
|
||||
:new-name new-token-name
|
||||
:references-count (:total-references scan-results))
|
||||
|
||||
:new-name new-token-name)
|
||||
(rx/of (dch/commit-changes token-changes))))))
|
||||
|
||||
(defn bulk-remap-tokens
|
||||
|
||||
@ -9,6 +9,7 @@
|
||||
[app.common.test-helpers.compositions :as ctho]
|
||||
[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.tokens :as ctht]
|
||||
[app.common.types.token :as cto]
|
||||
[app.common.types.tokens-lib :as ctob]
|
||||
@ -116,3 +117,111 @@
|
||||
(t/testing "valid new name should be valid"
|
||||
(let [result (dwtr/validate-token-remapping "color.primary" "brand.primary")]
|
||||
(t/is (true? (:valid? result)))))))
|
||||
|
||||
(defn- setup-file-with-component-copy-and-token
|
||||
"Create a file containing a component, an instance copy of it, and a single
|
||||
`color.primary` token already applied to both the main and the copy shape.
|
||||
Mirrors the in-app state right after a user applies a token to a main and
|
||||
component sync has propagated the applied-tokens map to the copy."
|
||||
[]
|
||||
(let [color-primary {:id (cthi/new-id! :color-primary)
|
||||
:name "color.primary"
|
||||
:value "#FF0000"
|
||||
:type :color}
|
||||
color-secondary {:id (cthi/new-id! :color-secondary)
|
||||
:name "color.secondary"
|
||||
:value "#00FF00"
|
||||
:type :color}]
|
||||
(-> (cthf/sample-file :file-1 :page-label :page-1)
|
||||
(ctho/add-simple-component-with-copy :component1
|
||||
:main-root
|
||||
:main-child
|
||||
:copy-root
|
||||
:copy-root-params {:children-labels [:copy-child]})
|
||||
(assoc-in [:data :tokens-lib]
|
||||
(-> (ctob/make-tokens-lib)
|
||||
(ctob/add-theme (ctob/make-token-theme :name "Theme A" :sets #{"Set A"}))
|
||||
(ctob/set-active-themes #{"/Theme A"})
|
||||
(ctob/add-set (ctob/make-token-set :id (cthi/new-id! :set-a)
|
||||
:name "Set A"))
|
||||
(ctob/add-token (cthi/id :set-a)
|
||||
(ctob/make-token color-primary))
|
||||
(ctob/add-token (cthi/id :set-a)
|
||||
(ctob/make-token color-secondary))))
|
||||
(ctht/apply-token-to-shape :main-child "color.primary" [:fill] [:fill] "#FF0000")
|
||||
(ctht/apply-token-to-shape :copy-child "color.primary" [:fill] [:fill] "#FF0000"))))
|
||||
|
||||
(t/deftest test-remap-tokens-finds-both-main-and-copy
|
||||
(t/testing "scan should find applied-token references on both main and copy shapes"
|
||||
(let [file (setup-file-with-component-copy-and-token)
|
||||
scan-results (dwtr/scan-workspace-token-references (:data file) "color.primary")
|
||||
shape-ids (set (map :shape-id (:applied-tokens scan-results)))]
|
||||
(t/is (= 2 (count (:applied-tokens scan-results))))
|
||||
(t/is (contains? shape-ids (cthi/id :main-child)))
|
||||
(t/is (contains? shape-ids (cthi/id :copy-child))))))
|
||||
|
||||
(t/deftest test-remap-tokens-does-not-touch-copy
|
||||
(t/testing "renaming a token must not flip the copy's sync group to :touched"
|
||||
(let [;; The production flow first renames the token in the tokens-lib,
|
||||
;; then dispatches remap-tokens to update applied-token references
|
||||
;; on shapes. Mirror that order here.
|
||||
file (-> (setup-file-with-component-copy-and-token)
|
||||
(update-in [:data :tokens-lib]
|
||||
(fn [lib]
|
||||
(ctob/update-token lib
|
||||
(cthi/id :set-a)
|
||||
(cthi/id :color-primary)
|
||||
#(assoc % :name "colors.primary")))))
|
||||
changes (dwtr/build-remap-changes (:data file)
|
||||
"color.primary"
|
||||
"colors.primary")
|
||||
file' (cthf/apply-changes file changes)
|
||||
main-child' (cths/get-shape file' :main-child)
|
||||
copy-child' (cths/get-shape file' :copy-child)]
|
||||
|
||||
(t/is (= "colors.primary" (get-in main-child' [:applied-tokens :fill])))
|
||||
(t/is (= "colors.primary" (get-in copy-child' [:applied-tokens :fill])))
|
||||
|
||||
;; If the rename marks :fill-group as touched on the copy, future
|
||||
;; main→copy propagation will skip it — that is the #9495 regression.
|
||||
(t/is (not (contains? (set (:touched copy-child')) :fill-group))))))
|
||||
|
||||
(t/deftest test-remap-preserves-copy-sync-of-later-token-apply
|
||||
(t/testing "End-to-end #9495 scenario: after a token group rename + REMAP,
|
||||
applying a new token to the main must still propagate to the copy."
|
||||
(let [;; 1. Build file with two tokens, a component, and color.primary
|
||||
;; already applied to both main-child and copy-child.
|
||||
file (setup-file-with-component-copy-and-token)
|
||||
|
||||
;; 2. Rename the token group: color.primary -> colors.primary,
|
||||
;; color.secondary -> colors.secondary (mimics user editing the
|
||||
;; group name in the tokens panel).
|
||||
file (update-in file [:data :tokens-lib]
|
||||
(fn [lib]
|
||||
(-> lib
|
||||
(ctob/update-token (cthi/id :set-a) (cthi/id :color-primary)
|
||||
#(assoc % :name "colors.primary"))
|
||||
(ctob/update-token (cthi/id :set-a) (cthi/id :color-secondary)
|
||||
#(assoc % :name "colors.secondary")))))
|
||||
|
||||
;; 3. REMAP runs to update applied-token names on shapes.
|
||||
file (cthf/apply-changes
|
||||
file
|
||||
(dwtr/build-remap-changes (:data file)
|
||||
"color.primary"
|
||||
"colors.primary"))
|
||||
|
||||
;; 4. User then applies colors.secondary to the main (Step 7).
|
||||
file (ctht/apply-token-to-shape file :main-child "colors.secondary"
|
||||
[:fill] [:fill] "#00FF00")
|
||||
|
||||
;; 5. Component sync propagates the main's change to the copy.
|
||||
file (ctho/propagate-component-changes file :component1)
|
||||
|
||||
copy-child' (cths/get-shape file :copy-child)]
|
||||
|
||||
;; Step 8 of the issue: the copy must reflect the newly applied token.
|
||||
;; Pre-fix, REMAP marks :fill-group as touched on the copy, so the
|
||||
;; subsequent sync silently skips :applied-tokens and the copy is left
|
||||
;; pointing at colors.primary.
|
||||
(t/is (= "colors.secondary" (get-in copy-child' [:applied-tokens :fill]))))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user