🐛 Fix not being able to add multiple fills to text spans (v3) (#10988)

frontend/src/app/main/data/workspace/texts.cljs

@@ -30,6 +30,7 @@
    [app.main.data.workspace.reflow :as wrf]
    [app.main.data.workspace.selection :as dws]
    [app.main.data.workspace.shapes :as dwsh]
+   [app.main.data.workspace.texts-v3 :as dwt-v3]
    [app.main.data.workspace.transforms :as dwt]
    [app.main.data.workspace.undo :as dwu]
    [app.main.data.workspace.wasm-text :as dwwt]
@@ -699,13 +700,19 @@
 
            (rx/concat (rx/of (dwsh/update-shapes shape-ids update-shape options))
                       (when (features/active-feature? state "text-editor-wasm/v1")
-                        (let [styles ((comp update-node-fn migrate-node))
+                        ;; Transform each span so add-fill preserves its existing fills.
-                              result (wasm.api/apply-styles-to-selection styles)]
+                        (let [result (wasm.api/apply-styles-to-selection
+                                      (comp update-node-fn migrate-node)
+                                      {:with-fills? true})]
                           (when result
                             (rx/of (v2-update-text-shape-content
                                     (:shape-id result)
                                     (:content result)
-                                    :update-name? true)))))))))
+                                    :update-name? true)
+                                   ;; Refresh the panel now, not only after a reselect.
+                                   (dwt-v3/v3-update-text-editor-styles
+                                    (:shape-id result)
+                                    {:fills (:fills result)})))))))))
 
      ptk/EffectEvent
      (effect [_ state _]

frontend/src/app/render_wasm/api.cljs

@@ -703,9 +703,10 @@
 
 (defn apply-styles-to-selection
   "Apply style attrs to the currently selected text spans.
-   Updates the cached content, pushes to WASM, and returns {:shape-id :content} for saving."
+   Updates the cached content, pushes to WASM, and returns {:shape-id :content} for saving.
-  [attrs]
+   `:with-fills?` also returns the selection's `:fills`."
-  (let [result (text-editor/apply-styles-to-selection attrs use-shape set-shape-text-content)]
+  [styles & [opts]]
+  (let [result (text-editor/apply-styles-to-selection styles use-shape set-shape-text-content opts)]
     (request-render "apply-styles-to-selection")
     result))
 

frontend/src/app/render_wasm/text_editor.cljs

@@ -623,10 +623,9 @@
     {:start-para focus-para :start-offset focus-offset
      :end-para anchor-para :end-offset anchor-offset}))
 
-(defn- apply-attrs-to-paragraph
+(defn apply-attrs-to-paragraph
-  "Apply attrs to spans within [sel-start, sel-end) char range of a single paragraph.
+  "Apply `styles` (attrs map, or a fn per span) within [sel-start, sel-end), splitting spans."
-   Splits spans at boundaries as needed."
+  [para sel-start sel-end styles]
-  [para sel-start sel-end attrs]
   (let [spans  (:children para)
 
         result (loop [spans spans
@@ -645,8 +644,10 @@
                        (recur (rest spans) span-end (conj acc span))
                        (let [before   (when (> ol-start pos)
                                         (assoc span :text (subs text 0 (- ol-start pos))))
-                             selected (merge span attrs
+                             selected (-> (if (fn? styles)
-                                             {:text (subs text (- ol-start pos) (- ol-end pos))})
+                                            (styles span)
+                                            (merge span styles))
+                                          (assoc :text (subs text (- ol-start pos) (- ol-end pos))))
                              after    (when (< ol-end span-end)
                                         (assoc span :text (subs text (- ol-end pos))))]
                          (recur (rest spans) span-end
@@ -658,15 +659,50 @@
   [para]
   (apply + (map (fn [span] (count (:text span))) (:children para))))
 
+(defn- paragraph-selected-spans
+  "Return the spans of `para` that overlap the [sel-start, sel-end) char range."
+  [para sel-start sel-end]
+  (loop [spans (:children para)
+         pos   0
+         acc   []]
+    (if (empty? spans)
+      acc
+      (let [span     (first spans)
+            span-end (+ pos (count (:text span)))
+            overlap? (< (max pos sel-start) (min span-end sel-end))]
+        (recur (rest spans) span-end (cond-> acc overlap? (conj span)))))))
+
+(defn selection-fills
+  "The selection's fills: shared vector if all spans match, `:multiple` if not, nil if empty."
+  [content {:keys [start-para start-offset end-para end-offset]}]
+  (let [paragraphs (:children (first (:children content)))
+        selected   (mapcat (fn [idx para]
+                             (cond
+                               (or (< idx start-para) (> idx end-para)) nil
+                               (= start-para end-para) (paragraph-selected-spans para start-offset end-offset)
+                               (= idx start-para)      (paragraph-selected-spans para start-offset (para-char-count para))
+                               (= idx end-para)        (paragraph-selected-spans para 0 end-offset)
+                               :else                   (paragraph-selected-spans para 0 (para-char-count para))))
+                           (range (count paragraphs))
+                           paragraphs)
+        fills-set  (into #{} (map :fills) selected)]
+    (cond
+      (empty? selected)       nil
+      (= 1 (count fills-set)) (first fills-set)
+      :else                   :multiple)))
+
 (defn apply-styles-to-selection
-  [attrs use-shape-fn set-shape-text-content-fn]
+  "Apply `styles` (attrs map, or a fn per span) to the selected spans; `:with-fills?` also returns `:fills`."
+  [styles use-shape-fn set-shape-text-content-fn & [{:keys [with-fills?]}]]
   (when (wasm/ready?)
     (let [;; Drop nil-valued attrs so they are never merged onto text spans.
           ;; The DOM editor path strips these in `attrs->styles`; the WASM merge
           ;; here (`apply-attrs-to-paragraph`) does not, so an unresolved attr
           ;; (e.g. nil :font-family/:font-weight/:font-style from an unloaded
           ;; font) would corrupt the span and fail the backend schema.
-          attrs     (into {} (remove (comp nil? val)) attrs)
+          styles    (if (fn? styles)
+                      styles
+                      (into {} (remove (comp nil? val)) styles))
           shape-id  (text-editor-get-active-shape-id)
           selection (text-editor-get-selection)]
 
@@ -691,19 +727,19 @@
 
                               ;; same paragraph.
                               (= start-para end-para)
-                              (apply-attrs-to-paragraph para start-offset end-offset attrs)
+                              (apply-attrs-to-paragraph para start-offset end-offset styles)
 
                               ;; first paragraph
                               (= idx start-para)
-                              (apply-attrs-to-paragraph para start-offset (para-char-count para) attrs)
+                              (apply-attrs-to-paragraph para start-offset (para-char-count para) styles)
 
                               ;; final paragraph
                               (= idx end-para)
-                              (apply-attrs-to-paragraph para 0 end-offset attrs)
+                              (apply-attrs-to-paragraph para 0 end-offset styles)
 
                               ;; any other paragraph
                               :else
-                              (apply-attrs-to-paragraph para 0 (para-char-count para) attrs)))
+                              (apply-attrs-to-paragraph para 0 (para-char-count para) styles)))
 
                           (range (count paragraphs))
                           paragraphs))
@@ -716,5 +752,7 @@
                 (update-cached-content! shape-id new-content)
                 (use-shape-fn shape-id)
                 (set-shape-text-content-fn shape-id new-content)
-                {:shape-id shape-id
+                (cond-> {:shape-id shape-id
-                 :content  new-content}))))))))
+                         :content  new-content}
+                  with-fills?
+                  (assoc :fills (selection-fills new-content normalized-selection)))))))))))

frontend/test/frontend_tests/render_wasm/text_editor_apply_styles_test.cljs

@@ -0,0 +1,102 @@
+;; 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 INC Sucursal en España SL
+
+(ns frontend-tests.render-wasm.text-editor-apply-styles-test
+  "Unit tests for applying styles to a selection of text spans.
+
+   `apply-attrs-to-paragraph` splits the affected spans at the selection
+   boundaries and either merges a map of attrs onto the selected spans or, when
+   given a function, transforms each selected span. The function form is what
+   fill operations (add, remove, reorder...) rely on to preserve each span's
+   existing fills instead of overwriting them."
+  (:require
+   [app.render-wasm.text-editor :as text-editor]
+   [cljs.test :as t :include-macros true]))
+
+(def ^:private apply-attrs-to-paragraph text-editor/apply-attrs-to-paragraph)
+
+(defn- span [text fills]
+  {:text text :fills fills})
+
+(def ^:private red {:fill-color "#ff0000" :fill-opacity 1})
+(def ^:private green {:fill-color "#00ff00" :fill-opacity 1})
+
+(defn- prepend-fill
+  "Mirrors the `add-fill` node transform: prepend a fill to the span's fills."
+  [fill]
+  (fn [node] (update node :fills #(into [fill] %))))
+
+(t/deftest apply-map-attrs
+  (t/testing "a map of attrs is merged onto the selected span"
+    (let [para   {:children [(span "hello world" [red])]}
+          result (apply-attrs-to-paragraph para 0 5 {:font-size "20"})]
+      (t/is (= [(assoc (span "hello" [red]) :font-size "20")
+                (span " world" [red])]
+               (:children result))))))
+
+(t/deftest apply-fn-preserves-existing-fills
+  (t/testing "the fn form prepends to the selected span's existing fills"
+    (let [para   {:children [(span "hello world" [red])]}
+          result (apply-attrs-to-paragraph para 0 5 (prepend-fill green))]
+      (t/is (= [(span "hello" [green red])
+                (span " world" [red])]
+               (:children result)))))
+
+  (t/testing "each selected span keeps its own fills across multiple spans"
+    (let [para   {:children [(span "foo" [red])
+                             (span "bar" [green])]}
+          ;; select the whole paragraph (6 chars) and prepend green
+          result (apply-attrs-to-paragraph para 0 6 (prepend-fill green))]
+      (t/is (= [(span "foo" [green red])
+                (span "bar" [green green])]
+               (:children result)))))
+
+  (t/testing "a span outside the selection is left untouched"
+    (let [para   {:children [(span "abcdef" [red])]}
+          ;; select only "cd"
+          result (apply-attrs-to-paragraph para 2 4 (prepend-fill green))]
+      (t/is (= [(span "ab" [red])
+                (span "cd" [green red])
+                (span "ef" [red])]
+               (:children result))))))
+
+(defn- content [paras]
+  {:children [{:children paras}]})
+
+(defn- para [spans]
+  {:children spans})
+
+(defn- selection [start-para start-offset end-para end-offset]
+  {:start-para start-para :start-offset start-offset
+   :end-para end-para :end-offset end-offset})
+
+(t/deftest selection-fills
+  (t/testing "a selection where every span shares the same fills returns that vector"
+    (let [c (content [(para [(span "hello world" [red])])])]
+      (t/is (= [red] (text-editor/selection-fills c (selection 0 0 0 5))))))
+
+  (t/testing "a selection within a single span returns that span's fills"
+    (let [c (content [(para [(span "abcdef" [red])])])]
+      (t/is (= [red] (text-editor/selection-fills c (selection 0 2 0 4))))))
+
+  (t/testing "a selection spanning spans with different fills is :multiple"
+    (let [c (content [(para [(span "foo" [red])
+                             (span "bar" [green])])])]
+      (t/is (= :multiple (text-editor/selection-fills c (selection 0 0 0 6))))))
+
+  (t/testing "a selection restricted to one uniform span is not :multiple"
+    (let [c (content [(para [(span "foo" [red])
+                             (span "bar" [green])])])]
+      (t/is (= [green] (text-editor/selection-fills c (selection 0 3 0 6))))))
+
+  (t/testing "a selection across paragraphs with the same fills returns that vector"
+    (let [c (content [(para [(span "foo" [red])])
+                      (para [(span "bar" [red])])])]
+      (t/is (= [red] (text-editor/selection-fills c (selection 0 0 1 3))))))
+
+  (t/testing "a collapsed selection has no selected spans"
+    (let [c (content [(para [(span "hello" [red])])])]
+      (t/is (nil? (text-editor/selection-fills c (selection 0 2 0 2)))))))

frontend/test/frontend_tests/runner.cljs

@@ -52,6 +52,7 @@
    [frontend-tests.plugins.utils-test]
    [frontend-tests.plugins.value-objects-test]
    [frontend-tests.render-wasm.process-objects-test]
+   [frontend-tests.render-wasm.text-editor-apply-styles-test]
    [frontend-tests.render-wasm.text-editor-caret-color-test]
    [frontend-tests.svg-fills-test]
    [frontend-tests.text-editor-paste-guard-test]
@@ -143,6 +144,7 @@
    'frontend-tests.plugins.utils-test
    'frontend-tests.plugins.value-objects-test
    'frontend-tests.render-wasm.process-objects-test
+   'frontend-tests.render-wasm.text-editor-apply-styles-test
    'frontend-tests.render-wasm.text-editor-caret-color-test
    'frontend-tests.svg-fills-test
    'frontend-tests.tokens.copy-paste-props-test