mirror of
https://github.com/penpot/penpot.git
synced 2026-09-23 12:26:17 +00:00
🐛 Fix text variant changing text content on variant switch (#11815)
This commit is contained in:
parent
3c821f1b1b
commit
01363be3a8
@ -257,8 +257,12 @@
|
||||
(= a b)
|
||||
#{}
|
||||
|
||||
;; If types are different, the structure is different
|
||||
(not= (type a) (type b))
|
||||
;; If one is a map and the other isn't, the structure is different.
|
||||
;; Compare by category (map?), not by `type`: a map's underlying
|
||||
;; PersistentArrayMap/PersistentHashMap representation depends on its
|
||||
;; key count, not on the shape of the content tree, so two maps that
|
||||
;; only differ in size can otherwise report a false structure diff.
|
||||
(not= (map? a) (map? b))
|
||||
#{:text-content-structure}
|
||||
|
||||
;; If they are maps, check the keys
|
||||
|
||||
@ -125,6 +125,58 @@
|
||||
(assoc-in content-base [:children 0 :children 0 :children 0 :fills]
|
||||
[{:fill-color "#ff0000" :fill-opacity 1}]))
|
||||
|
||||
;; The newer text editor duplicates typography attrs onto the paragraph
|
||||
;; node (not just the leaf) once a non-default style is applied. That
|
||||
;; pushes the paragraph map's key count from 4 to 9, crossing the
|
||||
;; PersistentArrayMap/PersistentHashMap promotion threshold, which
|
||||
;; changes its `type` even though it's still just a map. These fixtures
|
||||
;; are built as raw content, matching real editor output exactly
|
||||
;; (content-base's paragraph already carries every default attr, so it
|
||||
;; can't reproduce the 4-key vs 9-key gap seen in practice).
|
||||
(defn- leaf-text-node
|
||||
[font-weight font-variant-id]
|
||||
{:line-height "1.2"
|
||||
:font-style "normal"
|
||||
:typography-ref-id nil
|
||||
:text-transform "none"
|
||||
:font-id "sourcesanspro"
|
||||
:font-size "14"
|
||||
:font-weight font-weight
|
||||
:typography-ref-file nil
|
||||
:font-variant-id font-variant-id
|
||||
:text-decoration "none"
|
||||
:letter-spacing "0"
|
||||
:fills [{:fill-color "#000000" :fill-opacity 1}]
|
||||
:font-family "sourcesanspro"
|
||||
:text "hello world"})
|
||||
|
||||
(defn- text-content
|
||||
[paragraph-extra-attrs font-weight font-variant-id]
|
||||
{:type "root"
|
||||
:vertical-align "top"
|
||||
:children
|
||||
[{:type "paragraph-set"
|
||||
:children
|
||||
[(merge {:type "paragraph"
|
||||
:text-align "left"
|
||||
:text-direction "ltr"
|
||||
:children [(leaf-text-node font-weight font-variant-id)]}
|
||||
paragraph-extra-attrs)]}]})
|
||||
|
||||
;; A plain, unstyled paragraph: only paragraph-specific attrs, 4 keys.
|
||||
(def content-plain-compact-paragraph
|
||||
(text-content {} "400" "regular"))
|
||||
|
||||
;; Same bold text/attrs, but the paragraph node also carries the
|
||||
;; duplicated attrs (9 keys), like the newer text editor produces.
|
||||
(def content-bold-duplicated-paragraph
|
||||
(text-content {:font-id "sourcesanspro"
|
||||
:font-weight "700"
|
||||
:font-variant-id "bold"
|
||||
:font-family "sourcesanspro"
|
||||
:font-style "normal"}
|
||||
"700" "bold"))
|
||||
|
||||
|
||||
(t/deftest test-get-diff-type
|
||||
(let [diff-text (cttx/get-diff-type content-base content-changed-text)
|
||||
@ -189,6 +241,14 @@
|
||||
(t/is (= #{:text-content-attribute} diff-typography-ref))
|
||||
(t/is (= #{:text-content-attribute} diff-fills))))
|
||||
|
||||
(t/deftest test-get-diff-type-paragraph-key-count
|
||||
;; A real attribute difference (400 vs 700), where only one side's
|
||||
;; paragraph node has the duplicated keys (9, crossing the map
|
||||
;; promotion threshold): must be reported as :text-content-attribute,
|
||||
;; not misclassified as :text-content-structure.
|
||||
(t/is (= #{:text-content-attribute} (cttx/get-diff-type content-plain-compact-paragraph
|
||||
content-bold-duplicated-paragraph))))
|
||||
|
||||
|
||||
(t/deftest test-get-diff-attrs
|
||||
(let [attrs-text (cttx/get-diff-attrs content-base content-changed-text)
|
||||
|
||||
@ -1106,7 +1106,7 @@
|
||||
(dwtp/propagate-workspace-tokens) ;; Make the new instance get the token values from the current file, not from the component's library
|
||||
(when (and (features/active-feature? state "render-wasm/v1")
|
||||
(seq new-text-ids))
|
||||
(dwwt/resize-wasm-text-all new-text-ids))
|
||||
(dwwt/resize-wasm-text-all new-text-ids {:skip-component-sync? true}))
|
||||
(ptk/data-event :layout/update {:ids update-layout-ids :undo-group undo-group})
|
||||
(dwu/commit-undo-transaction undo-id)
|
||||
(dws/select-shape (:id new-shape) false))))))
|
||||
|
||||
@ -0,0 +1,67 @@
|
||||
;; This Source Code Form is subject to the terms of the Mozilla Public
|
||||
;; License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
||||
;;
|
||||
;; Copyright (c) KALEIDOS SUBSIDIARY SL
|
||||
|
||||
(ns frontend-tests.data.workspace-variant-switch-wasm-test
|
||||
"Regression test for #10588: switching a component's variant while
|
||||
render-wasm/v1 is active must not let the derived WASM text-resize
|
||||
commit trigger a component sync that discards the switch's merged
|
||||
overrides. See commit fb22c1547c for the same class of bug."
|
||||
(:require
|
||||
[app.common.test-helpers.components :as cthc]
|
||||
[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.main.data.workspace.libraries :as dwl]
|
||||
[app.main.data.workspace.wasm-text :as dwwt]
|
||||
[cljs.test :as t :include-macros true]
|
||||
[frontend-tests.helpers.state :as ths]))
|
||||
|
||||
(def ^:private orig-resize-wasm-text-all dwwt/resize-wasm-text-all)
|
||||
|
||||
(def ^:private resize-calls (atom []))
|
||||
|
||||
(defn- mock-resize-wasm-text-all
|
||||
([ids] (mock-resize-wasm-text-all ids nil))
|
||||
([ids opts]
|
||||
(swap! resize-calls conj {:ids ids :opts opts})
|
||||
nil))
|
||||
|
||||
(t/use-fixtures :each
|
||||
{:before (fn []
|
||||
(reset! resize-calls [])
|
||||
(set! dwwt/resize-wasm-text-all mock-resize-wasm-text-all))
|
||||
:after (fn []
|
||||
(set! dwwt/resize-wasm-text-all orig-resize-wasm-text-all))})
|
||||
|
||||
(defn- setup-file
|
||||
[]
|
||||
(-> (cthf/sample-file :file1)
|
||||
(ctho/add-frame :root-a)
|
||||
(cthc/make-component :comp-a :root-a)
|
||||
(ctho/add-frame-with-text :root-b :text-b "hello")
|
||||
(cthc/make-component :comp-b :root-b)
|
||||
(cthc/instantiate-component :comp-a :copy1)))
|
||||
|
||||
(t/deftest test-variant-switch-marks-wasm-resize-as-skip-component-sync
|
||||
(t/async
|
||||
done
|
||||
(let [file (setup-file)
|
||||
store (ths/setup-store file)
|
||||
copy1 (cths/get-shape file :copy1)
|
||||
comp-b-id (cthi/id :comp-b)
|
||||
|
||||
events
|
||||
[(dwl/component-swap copy1 (:id file) comp-b-id true)]]
|
||||
|
||||
(ths/run-store
|
||||
store done events
|
||||
(fn [_new-state]
|
||||
(t/is (= 1 (count @resize-calls))
|
||||
"resize-wasm-text-all should be called once for the swapped-in text")
|
||||
(let [{:keys [opts]} (first @resize-calls)]
|
||||
(t/is (:skip-component-sync? opts)
|
||||
"the derived WASM resize commit must be tagged :skip-component-sync?, or watch-component-changes will treat it as a real edit and clobber the switch's merged overrides")))))))
|
||||
Loading…
x
Reference in New Issue
Block a user