github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 14:00:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Fix export detach and the migrations of copies

Browse Source 
Fixes in what export does to the copies of libraries it leaves out, and in
the migrations of swap slots and copy texts (Phase 3c).

- Drop library color refs in fills and strokes on detached export.

  The binfile export that detaches libraries drops each reference to another
  file. For colors it read only where the older format kept them
  (:fill-color-ref-file and :stroke-color-ref-file on the shape and on text
  nodes), so a library color ref inside :fills or :strokes, the current
  format, stayed and the exported file pointed at a library it does not
  carry (F57). Every color of the shape, its text nodes included, now goes
  through cts/process-shape-colors, which detaches the ones that refer to
  another file.

  file-test: a fill ref inside :fills is dropped.

- Detach copies by where their refs and components point on export.

  The binfile export that detaches libraries decided for each copy shape
  from one file, the :component-file of the shape or of the closest head
  above it, and then dropped its component and its :shape-ref together
  (F52). That is wrong both ways:
  - a library copy nested in a local component refers, in each copy of that
    component, to the nested copy in the local main; using the head's own
    library, its :shape-ref was dropped, and the next sync treated the shape
    as one the copy added;
  - a dropped link kept :touched, :remote-synced and :main-instance;
  - the shapes stored in deleted components were not detached at all.

  A shape's :shape-ref now points into the file of the component of the top
  head of its copy, or of the closest swapped head above it, and the link
  goes with the shape's other component attributes (ctk/detach-shape) only
  when that file is another one. A head whose own component is in another
  file stops being a head and loses :swap-group, but stays a copy while its
  main shape is in this file. Deleted components' shapes are detached the
  same way.

  file-test: a dropped link loses :touched; a library copy nested in a local
  component keeps its ref in the component's copy, and the file validates; a
  deleted component's nested library copy is detached.

- Follow a swap slot's chain through the pages of its mains.

  find-ref-id-for-swapped, which migration 0010 runs on files older than the
  swap group, walks a swapped shape's ref chain to the main child its swap
  slot names. It read each step's children from the page of the step's own
  component, not from the page the step lives on, so when the mains of a
  nesting sat on different pages it found nothing, and the migration then
  removed a valid slot: the swap was lost (F56). Each step now goes on in
  the container the ref lookup found it in.

  file-test: the original ref of a swap is found with the mains on two
  pages.

- Remove ctf/get-component-container-from-head.

  Its last caller, the swap slot chain walk of migration 0010, now takes
  each step's container from the ref lookup.

- Recompute copy texts touched with the content group alone.

  Files older than the text override groups mark a copy text whose content
  changed with :content-group alone. Sync reads that as nothing touched in
  the text and writes the main's content, so the copy lost its text and
  styles at the next sync of its main (F50). Migration 0034 converted the
  later groups but left this one.

  convert-text-overrides now also converts a copy text with :content-group
  and no text group, reading it as allowing every text group: compared with
  its near main, it takes the groups where it differs (all of them when the
  near main cannot be reached). Migration 0035 runs it on files that already
  passed 0034; a binfile import converts them with the other old text
  groups, deferred as they are.

  convert-text-overrides-test: such a copy takes the letters and styles that
  differ, nothing where nothing differs, and through the migration.
  text-sync-test: once migrated, the copy keeps its text and takes the
  main's new size.

AI-assisted-by: claude-opus-5-5
This commit is contained in:
alonso.torres 2026-10-15 15:10:09 +02:00
parent b7638d2fc4
commit b4c2240a60
6 changed files with 134 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
common/src/app/common/files/comp_processors.cljc
View File

@ -74,12 +74,10 @@
(defn- find-next-related-swap-shape-id
"Go up from the chain of references shapes that will eventually lead to the shape
with swap-slot-id as id. Returns the next shape on the chain"
[parent swap-slot-id libraries]
(let [container (ctf/get-component-container-from-head parent libraries)
objects (:objects container)
children (cfh/get-children objects (:id parent))
with swap-slot-id as id. Returns the next shape on the chain. `container`
holds `parent`."
[parent container swap-slot-id libraries]
(let [children (cfh/get-children (:objects container) (:id parent))
original-shape-id (->> children
(filter #(= swap-slot-id (:id %)))
first
@ -88,11 +86,14 @@
;; Return the children which id is the swap-slot-id
original-shape-id
;; No children with swap-slot-id as id, go up
(let [referenced-shape (ctf/find-ref-shape nil container libraries parent)
(let [referenced-shape (ctf/find-ref-shape nil container libraries parent :with-context? true)
;; Recursive call that will get the id of the next shape on
;; the chain that ends on a shape with swap-slot-id as id
next-shape-id (when referenced-shape
(find-next-related-swap-shape-id referenced-shape swap-slot-id libraries))]
(find-next-related-swap-shape-id referenced-shape
(:container (meta referenced-shape))
swap-slot-id
libraries))]
;; Return the children which shape-ref points to the next-shape-id
(->> children
(filter #(= next-shape-id (:shape-ref %)))
@ -108,10 +109,10 @@
parent (get objects (:parent-id shape))
parent-head (ctn/get-head-shape objects parent)
parent-ref (ctf/find-ref-shape nil container libraries parent-head)]
parent-ref (ctf/find-ref-shape nil container libraries parent-head :with-context? true)]
(when (and swap-slot parent-ref)
(find-next-related-swap-shape-id parent-ref swap-slot libraries))))
(find-next-related-swap-shape-id parent-ref (:container (meta parent-ref)) swap-slot libraries))))
;; ---- Repairs
@ -416,6 +417,18 @@
[shape]
(boolean (some legacy-text-groups (:touched shape))))
(defn- content-group-alone?
"Whether the copy text `shape` has `:content-group` touched with no text
override group, as files older than the text override groups mark a copy
text whose content changed in any way."
[shape]
(let [touched (:touched shape)]
(and (cfh/text-shape? shape)
(ctk/in-component-copy? shape)
(contains? touched :content-group)
(not-any? cttx/text-groups touched)
(not-any? legacy-text-groups touched))))
(defn recompute-text-touched
"The touched groups of a copy text `shape` that holds the text override
groups of files older than the feature components/text-overrides, given
@ -424,9 +437,10 @@
when `ref` is nil. A shape that is not a copy keeps none."
[shape ref]
(let [touched (:touched shape)
allowed (if (ctk/in-component-copy? shape)
(cttx/text-overrides touched)
#{})
allowed (cond
(not (ctk/in-component-copy? shape)) #{}
(content-group-alone? shape) cttx/text-groups
:else (cttx/text-overrides touched))
groups (if (some? ref)
(set/intersection allowed (cttx/override-groups (:content ref) (:content shape)))
allowed)]
@ -439,9 +453,10 @@
Files older than the feature components/text-overrides mark a copy text
with `:text-content-attribute` (any style) and `:text-content-structure`
(the number of paragraphs or runs); now it holds `:text-content-text` and
one group per style unit (`cttx/style-units`). Each copy text with the
old groups is compared with its near main, and takes the groups the old
ones allowed where it differs. When the near main cannot be reached it
one group per style unit (`cttx/style-units`). Older files still mark it
with `:content-group` alone, which allows every group. Each copy text with
the old groups is compared with its near main, and takes the groups the
old ones allowed where it differs. When the near main cannot be reached it
takes every group they allowed, so that it loses no override; with
`*defer-unreachable*` it is left, and the data carries
`::deferred-texts` in its metadata."
@ -452,7 +467,7 @@
convert-container
(fn [container]
(reduce (fn [container shape]
(if-not (legacy-text-touched? shape)
(if-not (or (legacy-text-touched? shape) (content-group-alone? shape))
container
(let [ref (when (ctk/in-component-copy? shape)
(ctf/find-ref-shape file container libraries shape

12
common/src/app/common/files/migrations.cljc
View File

@ -2234,6 +2234,15 @@
{})]
(cfcp/convert-text-overrides data libraries)))
(defmethod migrate-data "0035-recompute-content-group-alone"
[data _]
;; copy texts that older files mark with `:content-group` alone, which
;; 0034 left; it converts nothing else again
(let [libraries (if (:libs data)
(deref (:libs data))
{})]
(cfcp/convert-text-overrides data libraries)))
(def available-migrations
(into (d/ordered-set)
["legacy-2"
@ -2325,4 +2334,5 @@
"0031-migrate-stroke-width-token-attr"
"0032-fix-children-not-in-parent"
"0033-convert-swap-slots-to-swap-groups"
"0034-recompute-text-overrides"]))
"0034-recompute-text-overrides"
"0035-recompute-content-group-alone"]))

60
common/src/app/common/types/file.cljc
View File

@ -366,13 +366,6 @@
(cfh/make-container component-page :page))
(cfh/make-container component :component)))
(defn get-component-container-from-head
[instance-head libraries & {:keys [include-deleted?] :or {include-deleted? true}}]
(let [library-data (-> (get-component-library libraries instance-head)
:data)
component (ctkl/get-component library-data (:component-id instance-head) include-deleted?)]
(get-component-container library-data component)))
(defn get-component-root
"Retrieve the root shape of the component."
[file-data component]
@ -1161,20 +1154,21 @@
;; Export
(defn- get-component-ref-file
(defn- get-shape-ref-file
"The file of the main shape the copy shape `shape` refers to: the file of
the component of the top head of its copy, or of the closest swapped head
above it, whose children refer into its own component."
[objects shape]
(cond
(contains? shape :component-file)
(get shape :component-file)
(contains? shape :shape-ref)
(recur objects (get objects (:parent-id shape)))
:else
nil))
(let [parent (get objects (:parent-id shape))]
(cond
(not (ctk/in-component-copy? parent)) (:component-file shape)
(ctk/swapped? parent) (:component-file parent)
:else (recur objects parent))))
(defn detach-external-references
"Remove from `file` every reference to another file: colors,
typographies, components and the copy links to their mains, in the pages
and in the shapes of deleted components."
[file file-id]
(let [detach-text
(fn [content]
@ -1187,17 +1181,31 @@
(not= file-id (:typography-ref-file %))
(dissoc :typography-ref-id :typography-ref-file)))))
detach-color
(fn [shape position {:keys [ref-file]} _ _ detach-fn]
(if (and (some? ref-file) (not= file-id ref-file))
(detach-fn shape position)
shape))
;; a head whose component is in another file stops being a head; it
;; stays a copy while the main shape it refers to is in this file
detach-shape
(fn [objects shape]
(let [shape' (cond-> shape
(let [shape' (cond-> (cts/process-shape-colors shape detach-color)
(not= file-id (:fill-color-ref-file shape))
(dissoc :fill-color-ref-id :fill-color-ref-file)
(not= file-id (:stroke-color-ref-file shape))
(dissoc :stroke-color-ref-id :stroke-color-ref-file)
(not= file-id (get-component-ref-file objects shape))
(dissoc :component-id :component-file :shape-ref :component-root)
(and (ctk/instance-head? shape)
(not= file-id (:component-file shape)))
(-> (ctk/unhead-shape)
(ctk/remove-swap-group))
(and (ctk/in-component-copy? shape)
(not= file-id (get-shape-ref-file objects shape)))
(ctk/detach-shape)
(= :text (:type shape))
(update :content detach-text))]
@ -1215,9 +1223,15 @@
detach-pages
(fn [pages-index]
(d/update-vals pages-index #(update % :objects detach-objects)))]
(d/update-vals pages-index #(update % :objects detach-objects)))
(update-in file [:data :pages-index] detach-pages)))
detach-components
(fn [components]
(d/update-vals components #(cond-> % (:deleted %) (update :objects detach-objects))))]
(-> file
(update-in [:data :pages-index] detach-pages)
(d/update-in-when [:data :components] detach-components))))
;; Base font size

21
common/test/common_tests/files/convert_text_overrides_test.cljc
View File

@ -73,6 +73,20 @@
(let [file (old-copy [[["Price: "] ["$20" {:weight 700}]]] #{:content-group :text-content-text})]
(t/is (= (touched file :copy-child) (touched (convert file) :copy-child)))))
(t/deftest a-content-group-alone-becomes-the-letters-and-styles-that-differ
;; older files mark a copy text whose content changed with
;; `:content-group` alone
(t/is (= #{:content-group :text-content-text :text-content-fills}
(-> (old-copy [[["Cost: " {:color :red}] ["$10" {:weight 700 :color :red}]]]
#{:content-group})
(convert)
(touched :copy-child))))
(t/testing "where nothing differs it sets nothing"
(t/is (nil? (-> (old-copy main #{:content-group})
(convert)
(touched :copy-child)
(not-empty))))))
(defn- library-and-file
"A library with the text component :component1, and a file with a copy of
it whose text is red and holds the old style flag."
@ -116,3 +130,10 @@
(t/is (contains? (:features file) "components/text-overrides"))
(t/is (contains? cfeat/default-features "components/text-overrides"))
(t/is (= #{:content-group :text-content-fills} (touched file :copy-child)))))
(t/deftest migration-converts-a-content-group-alone
(let [old (-> (old-copy [[["Cost: "] ["$10" {:weight 700}]]] #{:content-group})
(assoc :migrations (disj fmg/available-migrations "0035-recompute-content-group-alone")))
file (fmg/migrate-file old (delay {}))]
(t/is (= #{:content-group :text-content-text} (touched file :copy-child)))))

14
common/test/common_tests/logic/text_sync_test.cljc
View File

@ -7,10 +7,10 @@
(ns common-tests.logic.text-sync-test
(:require
[app.common.files.changes-builder :as pcb]
[app.common.files.comp-processors :as cfcp]
[app.common.logic.libraries :as cll]
[app.common.logic.shapes :as cls]
[app.common.test-helpers.components :as thc]
[app.common.test-helpers.composable.core :as tm]
[app.common.test-helpers.compositions :as tho]
[app.common.test-helpers.files :as thf]
[app.common.test-helpers.ids-map :as thi]
@ -890,7 +890,7 @@
(t/deftest sync-copy-touched-only-in-content-group
;; A copy text whose `:touched` has `:content-group` but no text subgroup,
;; as files from before the subgroups hold it, keeps its own content
;; when the main changes.
;; when the main changes, once the file is migrated.
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-frame-with-text :main-root :main-child "hello world")
@ -902,7 +902,8 @@
(assoc-in (:content copy-child)
[:children 0 :children 0 :children 0 :text]
"copy text"))
(ths/update-shape :copy-child :touched #{:content-group}))
(ths/update-shape :copy-child :touched #{:content-group})
(update :data cfcp/convert-text-overrides {}))
page (thf/current-page file)
;; ==== Action
@ -929,7 +930,6 @@
;; ==== Get
copy-child' (ths/get-shape file' :copy-child)
line (get-in copy-child' [:content :children 0 :children 0 :children 0])]
;; with no subgroup, sync takes the "nothing touched" branch and writes
;; the main's content
(tm/known-failure "F50" "copy text" "hello world" (:text line))
(t/is (= #{:content-group} (:touched copy-child')))))
(t/is (= "copy text" (:text line)))
(t/is (= "32" (:font-size line)))
(t/is (= #{:content-group :text-content-text} (:touched copy-child')))))

36
common/test/common_tests/types/file_test.cljc
View File

@ -11,16 +11,18 @@
components."
(:require
[app.common.data :as d]
[app.common.files.changes-builder :as pcb]
[app.common.files.comp-processors :as cfcp]
[app.common.files.swap-slots :as cfss]
[app.common.files.validate :as cfv]
[app.common.logic.shapes :as cls]
[app.common.test-helpers.components :as thc]
[app.common.test-helpers.composable.core :as tm]
[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.types.component :as ctk]
[app.common.types.components-list :as ctkl]
[app.common.types.container :as ctn]
[app.common.types.file :as ctf]
[app.common.types.pages-list :as ctpl]
@ -61,7 +63,7 @@
(t/testing "a copy of a library component becomes plain shapes"
(t/is (= {} (links file' :lib-copy)))
(t/is (= {} (links file' :lib-copy-child)))
(tm/known-failure "F52" nil #{:fill-group} (:touched (ths/get-shape file' :lib-copy-child))))
(t/is (nil? (:touched (ths/get-shape file' :lib-copy-child)))))
(t/testing "a copy of a local component keeps its links"
(t/is (= (links file :copy) (links file' :copy)))
(t/is (= (links file :copy-child) (links file' :copy-child))))
@ -81,12 +83,26 @@
(t/testing "the nested library copy in the main becomes plain shapes"
(t/is (= {} (links file' :nested-head))))
(t/testing "its copy keeps the ref to the main's shape, which is local"
(tm/known-failure "F52"
{:shape-ref (thi/id :nested-head)}
{}
(links file' :copy-nested-head)))
(t/testing "the validation does not see the lost ref"
(t/is (= #{} (validation-errors file'))))))
(t/is (= {:shape-ref (thi/id :nested-head)} (links file' :copy-nested-head))))
(t/is (= #{} (validation-errors file')))))
(t/deftest detach-external-references-in-a-deleted-component
(let [library (library-with-component)
file (-> (thf/sample-file :file)
(tho/add-frame :main {:name "Frame2"})
(thc/instantiate-component :lib-component :nested-head
:library library
:parent-label :main)
(thc/make-component :component :main))
page (thf/current-page file)
[_ del] (cls/generate-delete-shapes (pcb/empty-changes nil (:id page))
file page (:objects page) #{(thi/id :main)} {})
file (thf/apply-changes file del :validate? false)
file' (ctf/detach-external-references file (:id file))
stored #(get-in (ctkl/get-component (:data %) (thi/id :component) true)
[:objects (thi/id :nested-head)])]
(t/is (= (thi/id :lib-component) (:component-id (stored file))))
(t/is (= {} (select-keys (stored file') link-attrs)))))
;; Fill and typography refs on the shape and on the text nodes themselves
;; (the older format), next to a fill ref inside `:fills`.
@ -141,7 +157,7 @@
[:fill-color-ref-file :stroke-color-ref-file]))))
(t/is (= [{} {:typography-ref-file (:id file)}] (text-leaves file')))
(t/testing "a fill ref inside `:fills`"
(tm/known-failure "F57" nil library-id (-> rect' :fills first :fill-color-ref-file)))))
(t/is (nil? (-> rect' :fills first :fill-color-ref-file))))))
;; ----- Asset usage
@ -230,7 +246,7 @@
(thc/instantiate-component :c3 :copy3)
(swap-at-second-level))]
(t/is (= (thi/id :nested-head1) (cfss/get-swap-slot (ths/get-shape file :swapped))))
(tm/known-failure "F56" (thi/id :nested-subhead2) nil (ref-id-for-swapped file))))
(t/is (= (thi/id :nested-subhead2) (ref-id-for-swapped file)))))
;; ----- Deleted components