From 4e6a5ffa6980f164bb2a88057e665c32e6347f84 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Tue, 30 Aug 2022 15:06:50 +0200 Subject: [PATCH] :bug: Fix problem with multi-user text editing --- CHANGES.md | 5 + .../main/data/workspace/notifications.cljs | 14 +- .../main/ui/workspace/shapes/text/editor.cljs | 6 +- .../workspace/shapes/text/viewport_texts.cljs | 258 ------------------ .../shapes/text/viewport_texts_html.cljs | 20 +- 5 files changed, 33 insertions(+), 270 deletions(-) delete mode 100644 frontend/src/app/main/ui/workspace/shapes/text/viewport_texts.cljs diff --git a/CHANGES.md b/CHANGES.md index 080504c2f4..d801c3758d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,10 @@ # CHANGELOG +## 1.15.2-beta + +### :bug: Bugs fixed + +- Fix problem with multi-user text editing [Taiga #3446](https://tree.taiga.io/project/penpot/issue/3446) ## 1.15.1-beta diff --git a/frontend/src/app/main/data/workspace/notifications.cljs b/frontend/src/app/main/data/workspace/notifications.cljs index a9388898c7..cdd7ca26ff 100644 --- a/frontend/src/app/main/data/workspace/notifications.cljs +++ b/frontend/src/app/main/data/workspace/notifications.cljs @@ -203,18 +203,24 @@ (fn [{:keys [type attr]}] (and (= :set type) (= attr :position-data))) - remove-update-position-data + add-origin-session-id + (fn [{:keys [] :as op}] + (cond-> op + (position-data-operation? op) + (update :val with-meta {:session-id (:session-id msg)}))) + + update-position-data (fn [change] (cond-> change (= :mod-obj (:type change)) - (update :operations #(filterv (comp not position-data-operation?) %)))) + (update :operations #(mapv add-origin-session-id %)))) process-page-changes (fn [[page-id changes]] (dch/update-indices page-id changes)) - ;; We remove `position-data` from the incomming message - changes (->> changes (mapv remove-update-position-data)) + ;; We update `position-data` from the incomming message + changes (->> changes (mapv update-position-data)) changes-by-pages (group-by :page-id changes)] (rx/merge diff --git a/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs b/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs index 1417c5d5b6..0e9791dfd3 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs @@ -268,7 +268,11 @@ text-modifier (mf/deref text-modifier-ref) - bounding-box (gsht/position-data-bounding-box (or text-modifier shape)) + shape (cond-> shape + (some? text-modifier) + (dwt/apply-text-modifier text-modifier)) + + bounding-box (gsht/position-data-bounding-box shape) x (min (:x bounding-box) (:x shape)) y (min (:y bounding-box) (:y shape)) diff --git a/frontend/src/app/main/ui/workspace/shapes/text/viewport_texts.cljs b/frontend/src/app/main/ui/workspace/shapes/text/viewport_texts.cljs deleted file mode 100644 index b0aa93524b..0000000000 --- a/frontend/src/app/main/ui/workspace/shapes/text/viewport_texts.cljs +++ /dev/null @@ -1,258 +0,0 @@ -;; 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) UXBOX Labs SL - -(ns app.main.ui.workspace.shapes.text.viewport-texts - (:require - [app.common.data :as d] - [app.common.data.macros :as dm] - [app.common.geom.shapes :as gsh] - [app.common.geom.shapes.text :as gsht] - [app.common.math :as mth] - [app.common.pages.helpers :as cph] - [app.common.text :as txt] - [app.main.data.workspace.texts :as dwt] - [app.main.fonts :as fonts] - [app.main.refs :as refs] - [app.main.store :as st] - [app.main.ui.hooks :as hooks] - [app.main.ui.shapes.text.fo-text :as fo] - [app.util.dom :as dom] - [app.util.object :as obj] - [app.util.text-editor :as ted] - [app.util.text-svg-position :as tsp] - [app.util.timers :as ts] - [promesa.core :as p] - [rumext.alpha :as mf])) - -(defn strip-position-data [shape] - (dissoc shape :position-data :transform :transform-inverse)) - -(defn strip-modifier - [modifier] - (if (or (some? (dm/get-in modifier [:modifiers :resize-vector])) - (some? (dm/get-in modifier [:modifiers :resize-vector-2]))) - modifier - (d/update-when modifier :modifiers dissoc :displacement :rotation))) - -(defn process-shape [modifiers {:keys [id] :as shape}] - (let [modifier (-> (get modifiers id) strip-modifier) - shape (cond-> shape - (not (gsh/empty-modifiers? (:modifiers modifier))) - (-> (assoc :grow-type :fixed) - (merge modifier) gsh/transform-shape))] - (-> shape - (cond-> (nil? (:position-data shape)) - (assoc :migrate true)) - strip-position-data))) - -(defn- update-with-editor-state - "Updates the shape with the current state in the editor" - [shape editor-state] - (let [content (:content shape) - editor-content - (when editor-state - (-> editor-state - (ted/get-editor-current-content) - (ted/export-content)))] - - (cond-> shape - (and (some? shape) (some? editor-content)) - (assoc :content (d/txt-merge content editor-content))))) - -(defn- update-text-shape - [{:keys [grow-type id migrate] :as shape} node] - ;; Check if we need to update the size because it's auto-width or auto-height - ;; Update the position-data of every text fragment - (p/let [position-data (tsp/calc-position-data id)] - ;; At least one paragraph needs to be inside the bounding box - (when (gsht/overlaps-position-data? shape position-data) - (st/emit! (dwt/update-position-data id position-data))) - - (when (contains? #{:auto-height :auto-width} grow-type) - (let [{:keys [width height]} - (-> (dom/query node ".paragraph-set") - (dom/get-client-size)) - width (mth/ceil width) - height (mth/ceil height)] - (when (and (not (mth/almost-zero? width)) - (not (mth/almost-zero? height)) - (not migrate)) - (st/emit! (dwt/resize-text id width height))))) - (st/emit! (dwt/clean-text-modifier id)))) - -(defn- update-text-modifier - [{:keys [grow-type id]} node] - (p/let [position-data (tsp/calc-position-data id) - props {:position-data position-data} - - props - (if (contains? #{:auto-height :auto-width} grow-type) - (let [{:keys [width height]} (-> (dom/query node ".paragraph-set") (dom/get-client-size)) - width (mth/ceil width) - height (mth/ceil height)] - (if (and (not (mth/almost-zero? width)) (not (mth/almost-zero? height))) - (assoc props :width width :height height) - props)) - props)] - - (st/emit! (dwt/update-text-modifier id props)))) - -(mf/defc text-container - {::mf/wrap-props false - ::mf/wrap [mf/memo]} - [props] - (let [shape (obj/get props "shape") - on-update (obj/get props "on-update") - - handle-update - (mf/use-callback - (mf/deps shape on-update) - (fn [node] - (when (some? node) - (on-update shape node))))] - - [:& fo/text-shape {:key (str "shape-" (:id shape)) - :ref handle-update - :shape shape - :grow-type (:grow-type shape)}])) - -(mf/defc viewport-texts-wrapper - {::mf/wrap-props false - ::mf/wrap [mf/memo #(mf/deferred % ts/idle-then-raf)]} - [props] - (let [text-shapes (obj/get props "text-shapes") - modifiers (obj/get props "modifiers") - prev-modifiers (hooks/use-previous modifiers) - prev-text-shapes (hooks/use-previous text-shapes) - - ;; A change in position-data won't be a "real" change - text-change? - (fn [id] - (let [old-shape (get prev-text-shapes id) - new-shape (get text-shapes id) - old-modifiers (-> (get prev-modifiers id) strip-modifier) - new-modifiers (-> (get modifiers id) strip-modifier)] - - (or (and (not (identical? old-shape new-shape)) - (not= (dissoc old-shape :migrate :position-data) - (dissoc new-shape :migrate :position-data))) - (not= new-modifiers old-modifiers) - - ;; When the position data is nil we force to recalculate - (:migrate new-shape)))) - - changed-texts - (mf/use-memo - (mf/deps text-shapes modifiers) - #(->> (keys text-shapes) - (filter text-change?) - (map (d/getf text-shapes)))) - - handle-update-modifier (mf/use-callback update-text-modifier) - handle-update-shape (mf/use-callback update-text-shape)] - - [:* - (for [{:keys [id] :as shape} changed-texts] - [:& text-container {:shape (gsh/transform-shape shape) - :on-update (if (some? (get modifiers (:id shape))) - handle-update-modifier - handle-update-shape) - :key (str (dm/str "text-container-" id))}])])) - -(mf/defc viewport-text-editing - {::mf/wrap-props false} - [props] - - (let [shape (obj/get props "shape") - - ;; Join current objects with the state of the editor - editor-state - (-> (mf/deref refs/workspace-editor-state) - (get (:id shape))) - - text-modifier-ref - (mf/use-memo (mf/deps (:id shape)) #(refs/workspace-text-modifier-by-id (:id shape))) - - text-modifier - (mf/deref text-modifier-ref) - - shape (cond-> shape - (some? editor-state) - (update-with-editor-state editor-state)) - - ;; When we have a text with grow-type :auto-height we need to check the correct height - ;; otherwise the center alignment will break - shape - (if (or (not= :auto-height (:grow-type shape)) (empty? text-modifier)) - shape - (let [tr-shape (dwt/apply-text-modifier shape text-modifier)] - (cond-> shape - ;; we only change the height otherwise could cause problems with the other fields - (some? text-modifier) - (assoc :height (:height tr-shape))))) - - shape (hooks/use-equal-memo shape) - - handle-update-shape (mf/use-callback update-text-modifier)] - - (mf/use-effect - (mf/deps (:id shape)) - (fn [] - #(st/emit! (dwt/remove-text-modifier (:id shape))))) - - [:& text-container {:shape shape - :on-update handle-update-shape}])) - -(defn check-props - [new-props old-props] - (and (identical? (unchecked-get new-props "objects") - (unchecked-get old-props "objects")) - (identical? (unchecked-get new-props "modifiers") - (unchecked-get old-props "modifiers")) - (= (unchecked-get new-props "edition") - (unchecked-get old-props "edition")))) - -(mf/defc viewport-texts - {::mf/wrap-props false - ::mf/wrap [#(mf/memo' % check-props)]} - [props] - (let [objects (obj/get props "objects") - edition (obj/get props "edition") - modifiers (obj/get props "modifiers") - - text-shapes - (mf/use-memo - (mf/deps objects) - #(into {} (filter (comp cph/text-shape? second)) objects)) - - text-shapes - (mf/use-memo - (mf/deps text-shapes modifiers) - #(d/update-vals text-shapes (partial process-shape modifiers))) - - editing-shape (get text-shapes edition) - - ;; This memo is necesary so the viewport-text-wrapper memoize its props correctly - text-shapes-wrapper - (mf/use-memo - (mf/deps text-shapes edition) - (fn [] - (dissoc text-shapes edition)))] - - ;; We only need the effect to run on "mount" because the next fonts will be changed when the texts are - ;; edited - (mf/use-effect - (fn [] - (let [text-nodes (->> text-shapes (vals)(mapcat #(txt/node-seq txt/is-text-node? (:content %)))) - fonts (into #{} (keep :font-id) text-nodes)] - (run! fonts/ensure-loaded! fonts)))) - - [:* - (when editing-shape - [:& viewport-text-editing {:shape editing-shape}]) - - [:& viewport-texts-wrapper {:text-shapes text-shapes-wrapper - :modifiers modifiers}]])) diff --git a/frontend/src/app/main/ui/workspace/shapes/text/viewport_texts_html.cljs b/frontend/src/app/main/ui/workspace/shapes/text/viewport_texts_html.cljs index a8892489a7..299bf74254 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/viewport_texts_html.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/viewport_texts_html.cljs @@ -28,7 +28,10 @@ [rumext.alpha :as mf])) (defn strip-position-data [shape] - (dissoc shape :position-data :transform :transform-inverse)) + (-> shape + (cond-> (some? (meta (:position-data shape))) + (with-meta (meta (:position-data shape)))) + (dissoc :position-data :transform :transform-inverse))) (defn strip-modifier [modifier] @@ -131,14 +134,17 @@ ;; A change in position-data won't be a "real" change text-change? (fn [id] - (let [old-shape (get prev-text-shapes id) - new-shape (get text-shapes id) + (let [new-shape (get text-shapes id) + old-shape (get prev-text-shapes id) old-modifiers (-> (get prev-modifiers id) strip-modifier) - new-modifiers (-> (get modifiers id) strip-modifier)] + new-modifiers (-> (get modifiers id) strip-modifier) - (or (and (not (identical? old-shape new-shape)) - (not= (dissoc old-shape :migrate :position-data) - (dissoc new-shape :migrate :position-data))) + remote? (some? (-> new-shape meta :session-id)) ] + + (or (and (not remote?) + (not (identical? old-shape new-shape)) + (not= (dissoc old-shape :migrate) + (dissoc new-shape :migrate))) (not= new-modifiers old-modifiers) ;; When the position data is nil we force to recalculate