From e4ee585704ba94e787677c8703e3388787d223b3 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 19 Feb 2025 17:21:52 +0100 Subject: [PATCH 1/8] :bug: Fix incorrect notification assignation after update operation --- CHANGES.md | 2 + frontend/src/app/main/data/profile.cljs | 24 +++++------ frontend/src/app/main/ui/settings.cljs | 4 +- .../app/main/ui/settings/notifications.cljs | 41 +++++-------------- 4 files changed, 27 insertions(+), 44 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index e235a8f00..db483b802 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -50,6 +50,8 @@ is a number of cores) - [COMMENTS] Notifications in Backend, Profile Section, and Mention Email Notification [Taiga #9233](https://tree.taiga.io/project/penpot/us/9233) ### :bug: Bugs fixed + +- Fix missing state refresh on notifications update [Taiga #10253](https://tree.taiga.io/project/penpot/issue/10253) - Fix icon visualization on select component [Taiga #8889](https://tree.taiga.io/project/penpot/issue/8889) - Fix typo on integration tests docs [Taiga #10112](https://tree.taiga.io/project/penpot/issue/10112) - Fix problem with alt key measures being stuck [Taiga #9348](https://tree.taiga.io/project/penpot/issue/9348) diff --git a/frontend/src/app/main/data/profile.cljs b/frontend/src/app/main/data/profile.cljs index c69a3fa45..b43ef9f55 100644 --- a/frontend/src/app/main/data/profile.cljs +++ b/frontend/src/app/main/data/profile.cljs @@ -14,11 +14,12 @@ [app.config :as cf] [app.main.data.event :as ev] [app.main.data.media :as di] + [app.main.data.notifications :as ntf] [app.main.data.team :as-alias dtm] [app.main.repo :as rp] [app.main.router :as rt] [app.plugins.register :as plugins.register] - [app.util.i18n :as i18n] + [app.util.i18n :as i18n :refer [tr]] [app.util.storage :as storage] [beicon.v2.core :as rx] [potok.v2.core :as ptk])) @@ -239,25 +240,24 @@ [:email-comments [::sm/one-of #{:all :partial :none}]] [:email-invites [::sm/one-of #{:all :none}]]]) +(def ^:private check-update-notifications-params + (sm/check-fn schema:update-notifications)) + (defn update-notifications [data] - (dm/assert! - "expected valid parameters" - (sm/check schema:update-notifications data)) - + (assert (check-update-notifications-params data)) (ptk/reify ::update-notifications ev/Event (-data [_] {}) + ptk/UpdateEvent + (update [_ state] + (update-in state [:profile :props] assoc :notifications data)) + ptk/WatchEvent (watch [_ _ _] - (let [{:keys [on-error on-success] - :or {on-error identity - on-success identity}} (meta data)] - (->> (rp/cmd! :update-profile-notifications data) - (rx/tap on-success) - (rx/catch #(do (on-error %) (rx/empty))) - (rx/ignore)))))) + (->> (rp/cmd! :update-profile-notifications data) + (rx/map #(ntf/success (tr "dashboard.notifications.notifications-saved"))))))) (defn update-profile-props [props] diff --git a/frontend/src/app/main/ui/settings.cljs b/frontend/src/app/main/ui/settings.cljs index e66c3f5ad..aa30c4f73 100644 --- a/frontend/src/app/main/ui/settings.cljs +++ b/frontend/src/app/main/ui/settings.cljs @@ -17,7 +17,7 @@ [app.main.ui.settings.change-email] [app.main.ui.settings.delete-account] [app.main.ui.settings.feedback :refer [feedback-page]] - [app.main.ui.settings.notifications :refer [notifications-page]] + [app.main.ui.settings.notifications :refer [notifications-page*]] [app.main.ui.settings.options :refer [options-page]] [app.main.ui.settings.password :refer [password-page]] [app.main.ui.settings.profile :refer [profile-page]] @@ -71,4 +71,4 @@ [:& access-tokens-page] :settings-notifications - [:& notifications-page])]]]])) + [:& notifications-page* {:profile profile}])]]]])) diff --git a/frontend/src/app/main/ui/settings/notifications.cljs b/frontend/src/app/main/ui/settings/notifications.cljs index b402b3af8..5779474c7 100644 --- a/frontend/src/app/main/ui/settings/notifications.cljs +++ b/frontend/src/app/main/ui/settings/notifications.cljs @@ -9,14 +9,11 @@ (:require [app.common.data :as d] [app.common.schema :as sm] - [app.main.data.notifications :as ntf] [app.main.data.profile :as dp] - [app.main.refs :as refs] [app.main.store :as st] [app.main.ui.components.forms :as fm] [app.util.dom :as dom] [app.util.i18n :as i18n :refer [tr]] - [okulary.core :as l] [rumext.v2 :as mf])) (def default-notification-settings @@ -24,29 +21,9 @@ :email-comments :partial :email-invites :all}) -(def notification-settings-ref - (l/derived - (fn [profile] - (-> (merge default-notification-settings - (-> profile :props :notifications)) - (d/update-vals d/name))) - refs/profile)) - -(defn- on-error - [form _] - (reset! form nil) - (st/emit! (ntf/error (tr "generic.error")))) - -(defn- on-success - [_] - (st/emit! (ntf/success (tr "dashboard.notifications.notifications-saved")))) - (defn- on-submit - [form event] - (dom/prevent-default event) - (let [params (with-meta (:clean-data @form) - {:on-success (partial on-success form) - :on-error (partial on-error form)})] + [form _event] + (let [params (:clean-data @form)] (st/emit! (dp/update-notifications params)))) (def ^:private schema:notifications-form @@ -55,11 +32,15 @@ [:email-comments [::sm/one-of #{:all :partial :none}]] [:email-invites [::sm/one-of #{:all :partial :none}]]]) -(mf/defc notifications-page - [] - (let [settings (mf/deref notification-settings-ref) - form (fm/use-form :schema schema:notifications-form - :initial settings)] +(mf/defc notifications-page* + [{:keys [profile]}] + (let [settings (mf/with-memo [profile] + (-> (merge default-notification-settings + (-> profile :props :notifications)) + (update-vals d/name))) + form (fm/use-form :schema schema:notifications-form + :initial settings)] + (mf/with-effect [] (dom/set-html-title (tr "title.settings.notifications"))) From 7d0c19fcc77595b651ae23f47e3ed799f263882b Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 19 Feb 2025 11:56:28 +0100 Subject: [PATCH 2/8] :bug: Add correct feature check on manifest reading Instead on the file save operation so we can raise exception if something does not match without processing the whole file --- backend/src/app/binfile/common.clj | 11 +++++------ backend/src/app/binfile/v3.clj | 7 +++++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index a0b256660..8cd4f8cd3 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -513,12 +513,11 @@ (assoc :ignore-sync-until (dt/plus timestamp (dt/duration {:seconds 5}))) (update :features (fn [features] - (let [features (cfeat/check-supported-features! features)] - (-> (::features cfg #{}) - (set/union features) - ;; We never want to store - ;; frontend-only features on file - (set/difference cfeat/frontend-only-features))))))] + (-> (::features cfg #{}) + (set/union features) + ;; We never want to store + ;; frontend-only features on file + (set/difference cfeat/frontend-only-features)))))] (when (contains? cf/flags :file-schema-validation) (fval/validate-file-schema! file)) diff --git a/backend/src/app/binfile/v3.clj b/backend/src/app/binfile/v3.clj index 006fc5ff0..dcda35455 100644 --- a/backend/src/app/binfile/v3.clj +++ b/backend/src/app/binfile/v3.clj @@ -875,14 +875,17 @@ :manifest manifest)) ;; Check if all files referenced on manifest are present - (doseq [{file-id :id} (:files manifest)] + (doseq [{file-id :id features :features} (:files manifest)] (let [path (str "files/" file-id ".json")] + (when-not (get-zip-entry input path) (ex/raise :type :validation :code :invalid-binfile-v3 :hint "some files referenced on manifest not found" :path path - :file-id file-id)))) + :file-id file-id)) + + (cfeat/check-supported-features! features))) (events/tap :progress {:section :manifest}) From a391d71b6018ff99e36c8daa2c6b8674962c2634 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 19 Feb 2025 12:12:38 +0100 Subject: [PATCH 3/8] :bug: Add correct feature handling on binfile import --- backend/src/app/binfile/common.clj | 1 - backend/src/app/rpc/commands/binfile.clj | 54 ++++++++++-------------- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index 8cd4f8cd3..f461ab7b6 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -541,7 +541,6 @@ file) - (defn apply-pending-migrations! "Apply alredy registered pending migrations to files" [cfg] diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index 358e23ea5..49ab84193 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -10,8 +10,10 @@ [app.binfile.common :as bfc] [app.binfile.v1 :as bf.v1] [app.binfile.v3 :as bf.v3] + [app.common.features :as cfeat] [app.common.logging :as l] [app.common.schema :as sm] + [app.config :as cf] [app.db :as db] [app.http.sse :as sse] [app.loggers.audit :as-alias audit] @@ -20,6 +22,7 @@ [app.rpc :as-alias rpc] [app.rpc.commands.files :as files] [app.rpc.commands.projects :as projects] + [app.rpc.commands.teams :as teams] [app.rpc.doc :as-alias doc] [app.tasks.file-gc] [app.util.services :as sv] @@ -91,41 +94,30 @@ ;; --- Command: import-binfile -(defn- import-binfile-v1 - [{:keys [::wrk/executor] :as cfg} {:keys [project-id profile-id name file]}] - (let [cfg (-> cfg - (assoc ::bfc/project-id project-id) - (assoc ::bfc/profile-id profile-id) - (assoc ::bfc/name name) - (assoc ::bfc/input (:path file)))] - - ;; NOTE: the importation process performs some operations that are - ;; not very friendly with virtual threads, and for avoid - ;; unexpected blocking of other concurrent operations we dispatch - ;; that operation to a dedicated executor. - (px/invoke! executor (partial bf.v1/import-files! cfg)))) - -(defn- import-binfile-v3 - [{:keys [::wrk/executor] :as cfg} {:keys [project-id profile-id name file]}] - (let [cfg (-> cfg - (assoc ::bfc/project-id project-id) - (assoc ::bfc/profile-id profile-id) - (assoc ::bfc/name name) - (assoc ::bfc/input (:path file)))] - ;; NOTE: the importation process performs some operations that are - ;; not very friendly with virtual threads, and for avoid - ;; unexpected blocking of other concurrent operations we dispatch - ;; that operation to a dedicated executor. - (px/invoke! executor (partial bf.v3/import-files! cfg)))) - (defn- import-binfile - [{:keys [::db/pool] :as cfg} {:keys [project-id version] :as params}] - (let [result (case (int version) - 1 (import-binfile-v1 cfg params) - 3 (import-binfile-v3 cfg params))] + [{:keys [::db/pool ::wrk/executor] :as cfg} {:keys [profile-id project-id version name file]}] + (let [team (teams/get-team pool + :profile-id profile-id + :project-id project-id) + cfg (-> cfg + (assoc ::bfc/features (cfeat/get-team-enabled-features cf/flags team)) + (assoc ::bfc/project-id project-id) + (assoc ::bfc/profile-id profile-id) + (assoc ::bfc/name name) + (assoc ::bfc/input (:path file))) + + ;; NOTE: the importation process performs some operations that are + ;; not very friendly with virtual threads, and for avoid + ;; unexpected blocking of other concurrent operations we dispatch + ;; that operation to a dedicated executor. + result (case (int version) + 1 (px/invoke! executor (partial bf.v1/import-files! cfg)) + 3 (px/invoke! executor (partial bf.v3/import-files! cfg)))] + (db/update! pool :project {:modified-at (dt/now)} {:id project-id}) + result)) (def ^:private schema:import-binfile From ca9b5b1b8a98e27925153a93b14332d9418dca7a Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 19 Feb 2025 12:13:40 +0100 Subject: [PATCH 4/8] :paperclip: Use standard asserts on binfile common ns --- backend/src/app/binfile/common.clj | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index f461ab7b6..0db55f664 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -9,7 +9,6 @@ binfile format implementations and management rpc methods." (:require [app.common.data :as d] - [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.features :as cfeat] [app.common.files.helpers :as cfh] @@ -219,10 +218,8 @@ "Given a set of file-id's, return all matching relations with the libraries" [cfg ids] - (dm/assert! - "expected a set of uuids" - (and (set? ids) - (every? uuid? ids))) + (assert (set? ids) "expected a set of uuids") + (assert (every? uuid? ids) "expected a set of uuids") (db/run! cfg (fn [{:keys [::db/conn]}] (let [ids (db/create-array conn "uuid" ids) @@ -503,9 +500,7 @@ specific, should not be used outside of binfile domain" [{:keys [::timestamp] :as cfg} file & {:as opts}] - (dm/assert! - "expected valid timestamp" - (dt/instant? timestamp)) + (assert (dt/instant? timestamp) "expected valid timestamp") (let [file (-> file (assoc :created-at timestamp) From cb8e31e7f8d45565ec6b2e72ae9e3ee23b5be301 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 19 Feb 2025 22:32:42 +0100 Subject: [PATCH 5/8] :bug: Add correct handling of features on clone-template --- backend/src/app/rpc/commands/management.clj | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/backend/src/app/rpc/commands/management.clj b/backend/src/app/rpc/commands/management.clj index 73d832fe3..c04660f6a 100644 --- a/backend/src/app/rpc/commands/management.clj +++ b/backend/src/app/rpc/commands/management.clj @@ -406,12 +406,16 @@ :prefix "penpot.template." :suffix "" :min-age "30m") - format (bfc/parse-file-format template) + format (bfc/parse-file-format template) + team (teams/get-team conn + :profile-id profile-id + :project-id project-id) cfg (-> cfg (assoc ::bfc/project-id project-id) (assoc ::bfc/profile-id profile-id) - (assoc ::bfc/input template)) + (assoc ::bfc/input template) + (assoc ::bfc/features (cfeat/get-team-enabled-features cf/flags team))) result (if (= format :binfile-v3) (px/invoke! executor (partial bf.v3/import-files! cfg)) From dd6ae81e838b193b00dc8de0f70f626acfc6a833 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 19 Feb 2025 22:40:11 +0100 Subject: [PATCH 6/8] :bug: Add correct feature handling on dbg binfile import --- backend/src/app/http/debug.clj | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/backend/src/app/http/debug.clj b/backend/src/app/http/debug.clj index 4eed5a4d0..5c6f5cce1 100644 --- a/backend/src/app/http/debug.clj +++ b/backend/src/app/http/debug.clj @@ -12,6 +12,7 @@ [app.binfile.v3 :as bf.v3] [app.common.data :as d] [app.common.exceptions :as ex] + [app.common.features :as cfeat] [app.common.logging :as l] [app.common.pprint :as pp] [app.common.uuid :as uuid] @@ -21,6 +22,7 @@ [app.rpc.commands.auth :as auth] [app.rpc.commands.files-create :refer [create-file]] [app.rpc.commands.profile :as profile] + [app.rpc.commands.teams :as teams] [app.setup :as-alias setup] [app.srepl.helpers :as srepl] [app.storage :as-alias sto] @@ -317,7 +319,10 @@ :hint "missing upload file")) (let [profile (profile/get-profile pool profile-id) - project-id (:default-project-id profile)] + project-id (:default-project-id profile) + team (teams/get-team pool + :profile-id profile-id + :project-id project-id)] (when-not project-id (ex/raise :type :validation @@ -329,7 +334,8 @@ cfg (assoc cfg ::bfc/profile-id profile-id ::bfc/project-id project-id - ::bfc/input path)] + ::bfc/input path + ::bfc/features (cfeat/get-team-enabled-features cf/flags team))] (if (= format :binfile-v3) (bf.v3/import-files! cfg) From 0e73de17eccc1f288ac1e07806869030958383cd Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 19 Feb 2025 16:43:21 +0100 Subject: [PATCH 7/8] :bug: Fix incorrect libraries filtering on workspace --- frontend/src/app/main/data/workspace.cljs | 8 ++++-- .../app/main/data/workspace/libraries.cljs | 9 ++++-- frontend/src/app/main/refs.cljs | 28 +++++++++++++++++-- .../src/app/main/ui/workspace/libraries.cljs | 24 ++++++++-------- .../src/app/main/ui/workspace/sidebar.cljs | 3 +- .../app/main/ui/workspace/sidebar/assets.cljs | 11 ++++---- 6 files changed, 57 insertions(+), 26 deletions(-) diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 141dd25e1..d6bffce4b 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -168,11 +168,13 @@ (assoc file :data (d/removem (comp t/pointer? val) data)))))))))) (defn- libraries-fetched - [libraries] + [file-id libraries] (ptk/reify ::libraries-fetched ptk/UpdateEvent (update [_ state] - (let [libraries (d/index-by :id libraries)] + (let [libraries (->> libraries + (map (fn [l] (assoc l :library-of file-id))) + (d/index-by :id))] (update state :files merge libraries))) ptk/WatchEvent @@ -208,7 +210,7 @@ (rx/map #(assoc % :synced-at synced-at))))) (rx/merge-map resolve-file) (rx/reduce conj []) - (rx/map libraries-fetched)) + (rx/map (partial libraries-fetched file-id))) (->> (rx/from libraries) (rx/map :id) (rx/mapcat (fn [file-id] diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 34cf12d4f..d4a57d21e 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -1201,7 +1201,7 @@ (vals (get state :files))) do-more-info - #(modal/show! :libraries-dialog {:starting-tab "updates"}) + #(modal/show! :libraries-dialog {:starting-tab "updates" :file-id file-id}) do-update #(do (apply st/emit! (map (fn [library] @@ -1388,7 +1388,10 @@ (let [libraries (:workspace-shared-files state) library (d/seek #(= (:id %) library-id) libraries)] (if library - (update state :files assoc library-id (dissoc library :library-summary)) + (update state :files assoc library-id + (-> library + (dissoc :library-summary) + (assoc :library-of file-id))) state))) ptk/WatchEvent @@ -1401,6 +1404,8 @@ (->> (rp/cmd! :get-file {:id library-id :features features}) (rx/merge-map fpmap/resolve-file) ;; FIXME: this should call the libraries-fetched event instead of ad-hoc assoc event + (rx/map (fn [file] + (assoc file :library-of file-id))) (rx/map (fn [file] (fn [state] (assoc-in state [:files library-id] file))))) diff --git a/frontend/src/app/main/refs.cljs b/frontend/src/app/main/refs.cljs index deb64480c..f9e46b272 100644 --- a/frontend/src/app/main/refs.cljs +++ b/frontend/src/app/main/refs.cljs @@ -83,10 +83,32 @@ files (without the content, only summary)" (l/derived :shared-files st/state)) +(defn select-libraries + [files file-id] + (persistent! + (reduce-kv (fn [result id file] + (if (or (= id file-id) + (= (:library-of file) file-id)) + (assoc! result id file) + result)) + (transient {}) + files))) + +;; NOTE: for performance reasons, prefer derefing refs/files and then +;; use with-memo mechanism with `select-libraries` this will avoid +;; executing the select-libraries reduce-kv on each state change and +;; only execute it when files are changed. This ref exists for +;; backward compatibility with the code, but it is considered +;; DEPRECATED and all new code should not use it and old code should +;; be gradually migrated to more efficient approach (def libraries - "A derived state that contanins the currently loaded shared libraries - with all its content; including the current file" - (l/derived :files st/state)) + "A derived state that contanins the currently loaded shared + libraries with all its content; including the current file" + (l/derived (fn [state] + (let [files (get state :files) + file-id (get state :current-file-id)] + (select-libraries files file-id))) + st/state)) (defn extract-selected-files [files selected] diff --git a/frontend/src/app/main/ui/workspace/libraries.cljs b/frontend/src/app/main/ui/workspace/libraries.cljs index 963385602..90d9111b2 100644 --- a/frontend/src/app/main/ui/workspace/libraries.cljs +++ b/frontend/src/app/main/ui/workspace/libraries.cljs @@ -248,10 +248,12 @@ (mf/use-fn (mf/deps file-id) #(st/emit! (dwl/set-file-shared file-id false) - (modal/show :libraries-dialog {}))) + (modal/show :libraries-dialog {:file-id file-id}))) on-delete-cancel - (mf/use-fn #(st/emit! (modal/show :libraries-dialog {}))) + (mf/use-fn + (mf/deps file-id) + #(st/emit! (modal/show :libraries-dialog {:file-id file-id}))) publish (mf/use-fn @@ -259,7 +261,7 @@ (fn [event] (let [input-node (dom/get-target event) publish-library #(st/emit! (dwl/set-file-shared file-id true)) - cancel-publish #(st/emit! (modal/show :libraries-dialog {}))] + cancel-publish #(st/emit! (modal/show :libraries-dialog {:file-id file-id}))] (if empty-library? (st/emit! (modal/show {:type :confirm @@ -563,17 +565,15 @@ (mf/defc libraries-dialog {::mf/register modal/components ::mf/register-as :libraries-dialog} - [{:keys [starting-tab] :as props :or {starting-tab :libraries}}] - (let [;; NOTE: we don't want to react on file changes, we just want - ;; a snapshot of file on the momento of open the dialog - file (deref refs/file) - - file-id (:id file) - team-id (:team-id file) - shared? (:is-shared file) + [{:keys [starting-tab file-id] :as props :or {starting-tab :libraries}}] + (let [files (mf/deref refs/files) + file (get files file-id) + team-id (:team-id file) + shared? (:is-shared file) linked-libraries - (mf/deref refs/files) + (mf/with-memo [files file-id] + (refs/select-libraries files file-id)) linked-libraries (mf/with-memo [linked-libraries file-id] diff --git a/frontend/src/app/main/ui/workspace/sidebar.cljs b/frontend/src/app/main/ui/workspace/sidebar.cljs index 21e639f25..a556b664e 100644 --- a/frontend/src/app/main/ui/workspace/sidebar.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar.cljs @@ -56,6 +56,7 @@ [{:keys [layout file page-id] :as props}] (let [options-mode (mf/deref refs/options-mode-global) project (mf/deref refs/project) + file-id (get file :id) design-tokens? (features/use-feature "design-tokens/v1") mode-inspect? (= options-mode :inspect) @@ -116,7 +117,7 @@ assets-tab - (mf/html [:& assets-toolbox {:size (- size 58) :file-id file}]) + (mf/html [:& assets-toolbox {:size (- size 58) :file-id file-id}]) tokens-tab (when design-tokens? diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets.cljs index 7f240d734..d0d8ca8be 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets.cljs @@ -33,10 +33,10 @@ ::mf/private true} [{:keys [filters]}] (let [file-id (mf/use-ctx ctx/current-file-id) - - libraries (mf/deref refs/libraries) - libraries (mf/with-memo [libraries file-id] - (->> (vals libraries) + files (mf/deref refs/files) + libraries (mf/with-memo [files file-id] + (->> (refs/select-libraries files file-id) + (vals) (remove :is-indirect) (remove #(= file-id (:id %))) (map (fn [file] @@ -129,8 +129,9 @@ show-libraries-dialog (mf/use-fn + (mf/deps file-id) (fn [] - (modal/show! :libraries-dialog {}) + (modal/show! :libraries-dialog {:file-id file-id}) (modal/allow-click-outside!))) on-open-menu From 6e92e3b765ec7f48e4fc0bff9ac19d111884f444 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 19 Feb 2025 16:10:35 +0100 Subject: [PATCH 8/8] :bug: Fix inconsistency on naming This also a fix of passing incorrect prop :shared-libs to a component that already expectes :libraries. It also removes unnecesary use of refs/libraries ref --- .../app/main/ui/inspect/attributes/text.cljs | 4 ++- .../app/main/ui/inspect/right_sidebar.cljs | 15 ++++------ .../app/main/ui/workspace/color_palette.cljs | 6 ++-- .../ui/workspace/colorpicker/libraries.cljs | 2 +- .../ui/workspace/sidebar/assets/common.cljs | 2 +- .../workspace/sidebar/assets/components.cljs | 4 +-- .../main/ui/workspace/sidebar/options.cljs | 14 +++++----- .../options/menus/color_selection.cljs | 4 +-- .../sidebar/options/menus/component.cljs | 5 ++-- .../workspace/sidebar/options/menus/text.cljs | 28 +++++++++---------- .../sidebar/options/rows/color_row.cljs | 4 +-- .../sidebar/options/shapes/group.cljs | 4 +-- .../sidebar/options/shapes/text.cljs | 6 ++-- .../app/main/ui/workspace/text_palette.cljs | 12 +++++--- .../ui/workspace/text_palette_ctx_menu.cljs | 4 +-- 15 files changed, 56 insertions(+), 58 deletions(-) diff --git a/frontend/src/app/main/ui/inspect/attributes/text.cljs b/frontend/src/app/main/ui/inspect/attributes/text.cljs index e478b93a5..e1828710a 100644 --- a/frontend/src/app/main/ui/inspect/attributes/text.cljs +++ b/frontend/src/app/main/ui/inspect/attributes/text.cljs @@ -56,10 +56,12 @@ (make-typographies-library-ref (:typography-ref-file style))) typography-library (mf/deref typography-library-ref) + + ;; FIXME: too many duplicate operations file-typographies-viewer (mf/deref file-typographies-ref) file-typographies-workspace (mf/deref refs/workspace-file-typography) - file-library-workspace (get (mf/deref refs/libraries) (:typography-ref-file style)) + file-library-workspace (get (mf/deref refs/files) (:typography-ref-file style)) typography-external-lib (get-in file-library-workspace [:data :typographies (:typography-ref-id style)]) color-format (mf/use-state :hex) diff --git a/frontend/src/app/main/ui/inspect/right_sidebar.cljs b/frontend/src/app/main/ui/inspect/right_sidebar.cljs index 27ca93817..990b3a228 100644 --- a/frontend/src/app/main/ui/inspect/right_sidebar.cljs +++ b/frontend/src/app/main/ui/inspect/right_sidebar.cljs @@ -26,16 +26,11 @@ "Retrieve all libraries, including the local file, on workspace or viewer" [from] (if (= from :workspace) - (let [workspace-data (deref refs/workspace-data) - {:keys [id] :as local} workspace-data - libraries (deref refs/libraries)] - (-> libraries - (assoc id {:id id - :data local}))) - (let [viewer-data (deref refs/viewer-data) - local (get-in viewer-data [:file :data]) - id (get local :id) - libraries (:libraries viewer-data)] + (deref refs/libraries) + (let [viewer-data (deref refs/viewer-data) + local (get-in viewer-data [:file :data]) + id (get local :id) + libraries (:libraries viewer-data)] (-> libraries (assoc id {:id id :data local}))))) diff --git a/frontend/src/app/main/ui/workspace/color_palette.cljs b/frontend/src/app/main/ui/workspace/color_palette.cljs index 4f3e507e4..fd93b321d 100644 --- a/frontend/src/app/main/ui/workspace/color_palette.cljs +++ b/frontend/src/app/main/ui/workspace/color_palette.cljs @@ -166,9 +166,9 @@ (defn- make-library-colors-ref [file-id] - (l/derived (fn [libraries] - (dm/get-in libraries [file-id :data :colors])) - refs/libraries)) + (l/derived (fn [files] + (dm/get-in files [file-id :data :colors])) + refs/files)) (mf/defc file-color-palette* {::mf/private true} diff --git a/frontend/src/app/main/ui/workspace/colorpicker/libraries.cljs b/frontend/src/app/main/ui/workspace/colorpicker/libraries.cljs index 788d354db..6814a9a7c 100644 --- a/frontend/src/app/main/ui/workspace/colorpicker/libraries.cljs +++ b/frontend/src/app/main/ui/workspace/colorpicker/libraries.cljs @@ -48,7 +48,7 @@ {:value "file" :label (tr "workspace.libraries.colors.file-library")}]) options - (mf/with-memo [library-options file-id] + (mf/with-memo [library-options libraries file-id] (into library-options (comp (map val) diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs index 371cfc624..405ca5f26 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets/common.cljs @@ -321,7 +321,7 @@ current-file-id (mf/use-ctx ctx/current-file-id) current-page-id (mf/use-ctx ctx/current-page-id) - libraries (deref refs/libraries) + libraries (deref refs/files) current-file (get libraries current-file-id) objects (-> (dsh/get-page (:data current-file) current-page-id) diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets/components.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets/components.cljs index 15f81726c..bc4b6bc76 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets/components.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets/components.cljs @@ -44,7 +44,7 @@ (defn- get-component-root-and-container [file-id component] - (let [data (dm/get-in @refs/libraries [file-id :data]) + (let [data (dm/get-in @refs/files [file-id :data]) root-shape (ctf/get-component-root data component) container (ctf/get-component-page data component)] [root-shape container])) @@ -453,7 +453,7 @@ (fn [component event] (let [file-data - (dm/get-in @refs/libraries [file-id :data]) + (dm/get-in @refs/files [file-id :data]) shape-main (ctf/get-component-root file-data component)] diff --git a/frontend/src/app/main/ui/workspace/sidebar/options.cljs b/frontend/src/app/main/ui/workspace/sidebar/options.cljs index 56a8023a0..1d59834b6 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options.cljs @@ -44,7 +44,7 @@ (mf/defc shape-options* {::mf/wrap [#(mf/throttle % 60)]} - [{:keys [shape shapes-with-children page-id file-id shared-libs] :as props}] + [{:keys [shape shapes-with-children page-id file-id libraries] :as props}] (let [shape-type (dm/get-prop shape :type) shape-id (dm/get-prop shape :id) @@ -56,8 +56,8 @@ [:* (case shape-type :frame [:> frame/options* props] - :group [:& group/options {:shape shape :shape-with-children shapes-with-children :file-id file-id :shared-libs shared-libs}] - :text [:& text/options {:shape shape :file-id file-id :shared-libs shared-libs}] + :group [:& group/options {:shape shape :shape-with-children shapes-with-children :file-id file-id :libraries libraries}] + :text [:& text/options {:shape shape :file-id file-id :libraries libraries}] :rect [:& rect/options {:shape shape}] :circle [:& circle/options {:shape shape}] :path [:& path/options {:shape shape}] @@ -83,7 +83,7 @@ [{:keys [selected objects page-id file-id selected-shapes shapes-with-children]}] (let [sp-panel (mf/deref refs/specialized-panel) drawing (mf/deref refs/workspace-drawing) - shared-libs (mf/deref refs/libraries) + libraries (mf/deref refs/libraries) edition (mf/deref refs/selected-edition) edit-grid? (ctl/grid-layout? objects edition) grid-edition (mf/deref refs/workspace-grid-edition) @@ -113,7 +113,7 @@ {:shape (:object drawing) :page-id page-id :file-id file-id - :shared-libs shared-libs}] + :libraries libraries}] (= 0 (count selected)) [:> page/options*] @@ -123,7 +123,7 @@ {:shape (first selected-shapes) :page-id page-id :file-id file-id - :shared-libs shared-libs + :libraries libraries :shapes-with-children shapes-with-children}] :else @@ -132,7 +132,7 @@ :shapes selected-shapes :page-id page-id :file-id file-id - :shared-libs shared-libs}])])) + :libraries libraries}])])) (mf/defc options-content* {::mf/memo true diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs index f6b753ba1..bd8579138 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs @@ -19,8 +19,8 @@ [rumext.v2 :as mf])) (defn- prepare-colors - [shapes file-id shared-libs] - (let [data (into [] (remove nil? (ctc/extract-all-colors shapes file-id shared-libs))) + [shapes file-id libraries] + (let [data (into [] (remove nil? (ctc/extract-all-colors shapes file-id libraries))) groups (d/group-by :attrs #(dissoc % :attrs) data) all-colors (distinct (mapv :attrs data)) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs index 86fb956a2..56e1f1533 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs @@ -295,7 +295,8 @@ (let [single? (= 1 (count shapes)) shape (first shapes) current-file-id (mf/use-ctx ctx/current-file-id) - libraries (mf/deref refs/files) + + libraries (mf/deref refs/libraries) objects (mf/deref refs/workspace-page-objects) ^boolean @@ -513,7 +514,7 @@ [{:keys [shapes swap-opened?]}] (let [current-file-id (mf/use-ctx ctx/current-file-id) - libraries (deref refs/libraries) + libraries (deref refs/files) current-file (get libraries current-file-id) state* (mf/use-state diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/text.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/text.cljs index e2363c52a..74e645737 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/text.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/text.cljs @@ -184,7 +184,7 @@ (let [file-id (mf/use-ctx ctx/current-file-id) typographies (mf/deref refs/workspace-file-typography) - shared-libs (mf/deref refs/libraries) + libraries (mf/deref refs/files) label (case type :multiple (tr "workspace.options.text-options.title-selection") :group (tr "workspace.options.text-options.title-group") @@ -224,21 +224,19 @@ (emit-update! ids attrs))) typography - (mf/use-memo - (mf/deps values file-id shared-libs) - (fn [] - (cond - (and typography-id - (not= typography-id :multiple) - (not= typography-file-id file-id)) - (-> shared-libs - (get-in [typography-file-id :data :typographies typography-id]) - (assoc :file-id typography-file-id)) + (mf/with-memo [values file-id libraries] + (cond + (and typography-id + (not= typography-id :multiple) + (not= typography-file-id file-id)) + (-> libraries + (get-in [typography-file-id :data :typographies typography-id]) + (assoc :file-id typography-file-id)) - (and typography-id - (not= typography-id :multiple) - (= typography-file-id file-id)) - (get typographies typography-id)))) + (and typography-id + (not= typography-id :multiple) + (= typography-file-id file-id)) + (get typographies typography-id))) on-convert-to-typography (fn [_] diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs index 7e7cdb17e..b86a972df 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs @@ -48,13 +48,13 @@ [{:keys [index color disable-gradient disable-opacity disable-image disable-picker hidden on-change on-reorder on-detach on-open on-close on-remove disable-drag on-focus on-blur select-only select-on-focus]}] - (let [shared-libs (mf/deref refs/libraries) + (let [libraries (mf/deref refs/files) hover-detach (mf/use-state false) on-change (h/use-ref-callback on-change) file-id (or (:ref-file color) (:file-id color)) color-id (or (:ref-id color) (:id color)) - src-colors (dm/get-in shared-libs [file-id :data :colors]) + src-colors (dm/get-in libraries [file-id :data :colors]) color-name (dm/get-in src-colors [color-id :name]) multiple-colors? (uc/multiple? color) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/shapes/group.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/shapes/group.cljs index e39a3aff6..140174130 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/shapes/group.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/shapes/group.cljs @@ -34,7 +34,7 @@ [props] (let [shape (unchecked-get props "shape") shape-with-children (unchecked-get props "shape-with-children") - shared-libs (unchecked-get props "shared-libs") + libraries (unchecked-get props "libraries") objects (->> shape-with-children (group-by :id) (d/mapm (fn [_ v] (first v)))) file-id (unchecked-get props "file-id") layout-container-values (select-keys shape layout-container-flex-attrs) @@ -106,7 +106,7 @@ {:type type :shapes (vals objects) :file-id file-id - :libraries shared-libs}] + :libraries libraries}] (when-not (empty? shadow-ids) [:> shadow-menu* {:ids ids :values (get shape :shadow) :type type}]) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/shapes/text.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/shapes/text.cljs index 61d06a07d..5f1307632 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/shapes/text.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/shapes/text.cljs @@ -29,7 +29,7 @@ [rumext.v2 :as mf])) (mf/defc options - [{:keys [shape file-id] :as props}] + [{:keys [shape file-id libraries] :as props}] (let [ids [(:id shape)] type (:type shape) @@ -53,8 +53,6 @@ (mf/deref refs/workspace-v2-editor-state) (mf/deref refs/workspace-editor-state)) - shared-libs (mf/deref refs/libraries) - editor-state (when (not (features/active-feature? @st/state "text-editor/v2")) (get state-map (:id shape))) @@ -150,7 +148,7 @@ {:type type :shapes [shape] :file-id file-id - :libraries shared-libs}]) + :libraries libraries}]) [:> shadow-menu* {:ids ids :values (get shape :shadow)}] diff --git a/frontend/src/app/main/ui/workspace/text_palette.cljs b/frontend/src/app/main/ui/workspace/text_palette.cljs index 7c92895dc..4becd5a54 100644 --- a/frontend/src/app/main/ui/workspace/text_palette.cljs +++ b/frontend/src/app/main/ui/workspace/text_palette.cljs @@ -68,7 +68,7 @@ (str (:font-size typography) "px | " (:name variant-data))]])])) (mf/defc palette - [{:keys [selected selected-ids current-file-id file-typographies shared-libs size width]}] + [{:keys [selected selected-ids current-file-id file-typographies libraries size width]}] (let [file-id (case selected :recent nil @@ -79,7 +79,7 @@ (case selected :recent [] :file (sort-by #(str/lower (:name %)) (vals file-typographies)) - (sort-by #(str/lower (:name %)) (vals (get-in shared-libs [selected :data :typographies])))) + (sort-by #(str/lower (:name %)) (vals (get-in libraries [selected :data :typographies])))) state (mf/use-state {:offset 0}) offset-step 144 buttons-size (cond @@ -182,13 +182,17 @@ {::mf/wrap [mf/memo]} [{:keys [size width selected] :as props}] (let [selected-ids (mf/deref refs/selected-shapes) + + ;; FIXME: we have duplicate operations, if we already have the + ;; libraries, so we already have file-typographies so we don't + ;; need two separate lens/refs for that file-typographies (mf/deref refs/workspace-file-typography) - shared-libs (mf/deref refs/libraries) + libraries (mf/deref refs/files) current-file-id (mf/use-ctx ctx/current-file-id)] [:& palette {:current-file-id current-file-id :selected-ids selected-ids :file-typographies file-typographies - :shared-libs shared-libs + :libraries libraries :width width :selected selected :size size}])) diff --git a/frontend/src/app/main/ui/workspace/text_palette_ctx_menu.cljs b/frontend/src/app/main/ui/workspace/text_palette_ctx_menu.cljs index 568e6a84e..8034780e5 100644 --- a/frontend/src/app/main/ui/workspace/text_palette_ctx_menu.cljs +++ b/frontend/src/app/main/ui/workspace/text_palette_ctx_menu.cljs @@ -17,11 +17,11 @@ (mf/defc text-palette-ctx-menu [{:keys [show-menu? close-menu on-select-palette selected]}] (let [typographies (mf/deref refs/workspace-file-typography) - shared-libs (mf/deref refs/libraries)] + libraries (mf/deref refs/libraries)] [:& dropdown {:show show-menu? :on-close close-menu} [:ul {:class (stl/css :text-context-menu)} - (for [[idx cur-library] (map-indexed vector (vals shared-libs))] + (for [[idx cur-library] (map-indexed vector (vals libraries))] (let [typographies (-> cur-library (get-in [:data :typographies]) vals)] [:li {:class (stl/css-case :palette-library true