From ab623cc02dcf16a33d76a6ac7d05701899785daa Mon Sep 17 00:00:00 2001 From: mvanhorn Date: Wed, 17 Jun 2026 10:29:51 -0700 Subject: [PATCH] :bug: Make overlay position optional in plugin open-overlay/toggle-overlay interactions --- .../app/common/types/shape/interactions.cljc | 8 +- frontend/src/app/plugins/parser.cljs | 4 +- .../frontend_tests/plugins/parser_test.cljs | 77 ++++++++++++++++--- 3 files changed, 71 insertions(+), 18 deletions(-) diff --git a/common/src/app/common/types/shape/interactions.cljc b/common/src/app/common/types/shape/interactions.cljc index 4554d02511..d207f337ca 100644 --- a/common/src/app/common/types/shape/interactions.cljc +++ b/common/src/app/common/types/shape/interactions.cljc @@ -136,8 +136,8 @@ [:map {:title "OpenOverlayInteraction"} [:action-type [:= :open-overlay]] [:event-type [::sm/one-of event-types]] - [:overlay-position ::gpt/point] - [:overlay-pos-type [::sm/one-of overlay-positioning-types]] + [:overlay-position {:optional true} ::gpt/point] + [:overlay-pos-type {:optional true} [::sm/one-of overlay-positioning-types]] [:destination {:optional true} [:maybe ::sm/uuid]] [:close-click-outside {:optional true} :boolean] [:background-overlay {:optional true} :boolean] @@ -148,8 +148,8 @@ [:map {:title "ToggleOverlayInteraction"} [:action-type [:= :toggle-overlay]] [:event-type [::sm/one-of event-types]] - [:overlay-position ::gpt/point] - [:overlay-pos-type [::sm/one-of overlay-positioning-types]] + [:overlay-position {:optional true} ::gpt/point] + [:overlay-pos-type {:optional true} [::sm/one-of overlay-positioning-types]] [:destination {:optional true} [:maybe ::sm/uuid]] [:close-click-outside {:optional true} :boolean] [:background-overlay {:optional true} :boolean] diff --git a/frontend/src/app/plugins/parser.cljs b/frontend/src/app/plugins/parser.cljs index d6ec255ab8..1b9f844c48 100644 --- a/frontend/src/app/plugins/parser.cljs +++ b/frontend/src/app/plugins/parser.cljs @@ -488,8 +488,8 @@ {:action-type action-type :destination (-> (obj/get action "destination") (obj/get "$id")) :relative-to (-> (obj/get action "relativeTo") (obj/get "$id")) - :overlay-pos-type (-> (obj/get action "position") parse-keyword) - :overlay-position (-> (obj/get action "manualPositionLocation") parse-point (d/nilv (gpt/point 0 0))) + :overlay-pos-type (or (-> (obj/get action "position") parse-keyword) :center) + :overlay-position (-> (obj/get action "manualPositionLocation") parse-point) :close-click-outside (obj/get action "closeWhenClickOutside") :background-overlay (obj/get action "addBackgroundOverlay") :animation (-> (obj/get action "animation") parse-animation)} diff --git a/frontend/test/frontend_tests/plugins/parser_test.cljs b/frontend/test/frontend_tests/plugins/parser_test.cljs index 425c51d3fa..99b248e205 100644 --- a/frontend/test/frontend_tests/plugins/parser_test.cljs +++ b/frontend/test/frontend_tests/plugins/parser_test.cljs @@ -9,9 +9,29 @@ [app.common.geom.point :as gpt] [app.common.schema :as sm] [app.common.types.grid :as ctg] + [app.common.types.shape.interactions :as ctsi] + [app.common.uuid :as uuid] [app.plugins.parser :as parser] [cljs.test :as t :include-macros true])) +(defn- overlay-action + [{:keys [type destination position manual-position-location]}] + (let [action (js-obj "type" type + "destination" (js-obj "$id" destination))] + (when (some? position) + (unchecked-set action "position" position)) + (when (some? manual-position-location) + (unchecked-set action "manualPositionLocation" manual-position-location)) + action)) + +(defn- parse-overlay-interaction + [action] + (parser/parse-interaction "click" (overlay-action action) nil)) + +(defn- valid-interaction? + [interaction] + (sm/validate ctsi/schema:interaction interaction)) + (t/deftest test-parse-point-returns-gpt-point-record ;; Regression test for issue #8409. ;; @@ -34,18 +54,6 @@ (t/is (= 0 (:x result))) (t/is (= 0 (:y result)))))) -(t/deftest test-parse-overlay-action-defaults-manual-position - (let [destination #js {"$id" (random-uuid)} - action (parser/parse-action - #js {:type "open-overlay" - :destination destination - :position "center"})] - (t/is (= :open-overlay (:action-type action))) - (t/is (= :center (:overlay-pos-type action))) - (t/is (gpt/point? (:overlay-position action))) - (t/is (= 0 (:x (:overlay-position action)))) - (t/is (= 0 (:y (:overlay-position action)))))) - (t/deftest test-parse-frame-guide-calls-guide-parser (let [column (parser/parse-frame-guide #js {:type "column" @@ -85,3 +93,48 @@ (t/testing "clearing guides with an empty vector validates" (t/is (sm/validate [:vector ctg/schema:grid] (parser/parse-frame-guides #js []))))) + +(t/deftest test-parse-overlay-action-position-is-optional + (t/testing "open-overlay defaults omitted position to center" + (let [destination (uuid/next) + result (parse-overlay-interaction {:type "open-overlay" + :destination destination})] + (t/is (= :open-overlay (:action-type result))) + (t/is (= :click (:event-type result))) + (t/is (= destination (:destination result))) + (t/is (= :center (:overlay-pos-type result))) + (t/is (not (contains? result :overlay-position))) + (t/is (valid-interaction? result)))) + + (t/testing "toggle-overlay preserves manualPositionLocation" + (let [destination (uuid/next) + result (parse-overlay-interaction + {:type "toggle-overlay" + :destination destination + :position "manual" + :manual-position-location #js {:x 10 :y 20}}) + position (:overlay-position result)] + (t/is (= :toggle-overlay (:action-type result))) + (t/is (= :manual (:overlay-pos-type result))) + (t/is (gpt/point? position)) + (t/is (= 10 (:x position))) + (t/is (= 20 (:y position))) + (t/is (valid-interaction? result)))) + + (t/testing "explicit center position does not require manualPositionLocation" + (let [destination (uuid/next) + result (parse-overlay-interaction {:type "open-overlay" + :destination destination + :position "center"})] + (t/is (= :center (:overlay-pos-type result))) + (t/is (not (contains? result :overlay-position))) + (t/is (valid-interaction? result)))) + + (t/testing "manual position without manualPositionLocation still parses" + (let [destination (uuid/next) + result (parse-overlay-interaction {:type "open-overlay" + :destination destination + :position "manual"})] + (t/is (= :manual (:overlay-pos-type result))) + (t/is (not (contains? result :overlay-position))) + (t/is (valid-interaction? result)))))