mirror of
https://github.com/penpot/penpot.git
synced 2026-07-23 06:28:14 +00:00
🐛 Fix too much recursion when clicking shape in comments mode (#10622)
* 🐛 Fix too much recursion when clicking shape in comments mode Remove `deselect-all` from `handle-interrupt` in comments mode. The `select-shape` event emits `:interrupt` which the comments stream watcher routes to `handle-interrupt`. In comments mode, calling `deselect-all` cleared the selection and emitted a competing `rt/nav`, creating a synchronous recursion cycle in the potok store that overflowed the JS call stack. Fixes #10620 AI-assisted-by: opencode * 🐛 Add unit tests for comments handle-interrupt Make `handle-interrupt` public (defn- → defn) and add 4 tests covering each branch: draft thread, open thread, comments mode, and noop. AI-assisted-by: opencode
This commit is contained in:
parent
d6c50cc40b
commit
3400d6afbb
@ -24,7 +24,6 @@
|
||||
[app.main.data.workspace.drawing :as dwd]
|
||||
[app.main.data.workspace.edition :as dwe]
|
||||
[app.main.data.workspace.layout :as dwlo]
|
||||
[app.main.data.workspace.selection :as dws]
|
||||
[app.main.data.workspace.zoom :as dwz]
|
||||
[app.main.repo :as rp]
|
||||
[app.main.router :as rt]
|
||||
@ -71,7 +70,7 @@
|
||||
|
||||
(rx/take-until stopper-s))))))
|
||||
|
||||
(defn- handle-interrupt
|
||||
(defn handle-interrupt
|
||||
[]
|
||||
(ptk/reify ::handle-interrupt
|
||||
ptk/WatchEvent
|
||||
@ -85,8 +84,7 @@
|
||||
;; tool is active. When comments are merely visible during design,
|
||||
;; `select-shape` emits `:interrupt` and this would otherwise wipe
|
||||
;; the freshly selected shape, breaking click selection.
|
||||
comments-mode? (rx/of (dwe/clear-edition-mode)
|
||||
(dws/deselect-all true))
|
||||
comments-mode? (rx/of (dwe/clear-edition-mode))
|
||||
:else (rx/empty))))))
|
||||
|
||||
;; Event responsible of the what should be executed when user clicked
|
||||
|
||||
@ -0,0 +1,84 @@
|
||||
;; 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.data.workspace-comments-test
|
||||
(:require
|
||||
[app.main.data.comments :as dcmt]
|
||||
[app.main.data.workspace.comments :as dwcm]
|
||||
[app.main.data.workspace.edition :as dwe]
|
||||
[beicon.v2.core :as rx]
|
||||
[cljs.test :as t :include-macros true]
|
||||
[potok.v2.core :as ptk]))
|
||||
|
||||
(t/deftest test-handle-interrupt-draft
|
||||
(t/async
|
||||
done
|
||||
(let [event (dwcm/handle-interrupt)
|
||||
state {:comments-local {:draft {:id "draft-id"}}}
|
||||
result (ptk/watch event state (rx/empty))]
|
||||
(->> result
|
||||
(rx/subs!
|
||||
(fn [evt]
|
||||
(t/is (= ::dcmt/close-comment-thread (ptk/type evt))))
|
||||
(fn [err]
|
||||
(done)
|
||||
(js/console.error err)
|
||||
(t/do-report {:type :error :message "Stream error" :actual err}))
|
||||
(fn [_]
|
||||
(done)))))))
|
||||
|
||||
(t/deftest test-handle-interrupt-open
|
||||
(t/async
|
||||
done
|
||||
(let [event (dwcm/handle-interrupt)
|
||||
state {:comments-local {:open {:id "thread-id"}}}
|
||||
result (ptk/watch event state (rx/empty))]
|
||||
(->> result
|
||||
(rx/subs!
|
||||
(fn [evt]
|
||||
(t/is (= ::dcmt/close-comment-thread (ptk/type evt))))
|
||||
(fn [err]
|
||||
(done)
|
||||
(js/console.error err)
|
||||
(t/do-report {:type :error :message "Stream error" :actual err}))
|
||||
(fn [_]
|
||||
(done)))))))
|
||||
|
||||
(t/deftest test-handle-interrupt-comments-mode
|
||||
(t/async
|
||||
done
|
||||
(let [event (dwcm/handle-interrupt)
|
||||
state {:workspace-drawing {:tool :comments}}
|
||||
result (ptk/watch event state (rx/empty))]
|
||||
(->> result
|
||||
(rx/subs!
|
||||
(fn [evt]
|
||||
(t/is (= ::dwe/clear-edition-mode (ptk/type evt))))
|
||||
(fn [err]
|
||||
(done)
|
||||
(js/console.error err)
|
||||
(t/do-report {:type :error :message "Stream error" :actual err}))
|
||||
(fn [_]
|
||||
(done)))))))
|
||||
|
||||
(t/deftest test-handle-interrupt-noop
|
||||
(t/async
|
||||
done
|
||||
(let [event (dwcm/handle-interrupt)
|
||||
state {}
|
||||
result (ptk/watch event state (rx/empty))
|
||||
emitted? (atom false)]
|
||||
(->> result
|
||||
(rx/subs!
|
||||
(fn [_]
|
||||
(reset! emitted? true))
|
||||
(fn [err]
|
||||
(done)
|
||||
(js/console.error err)
|
||||
(t/do-report {:type :error :message "Stream error" :actual err}))
|
||||
(fn [_]
|
||||
(t/is (false? @emitted?) "should not emit any events")
|
||||
(done)))))))
|
||||
@ -12,6 +12,7 @@
|
||||
[frontend-tests.data.uploads-test]
|
||||
[frontend-tests.data.viewer-test]
|
||||
[frontend-tests.data.workspace-colors-test]
|
||||
[frontend-tests.data.workspace-comments-test]
|
||||
[frontend-tests.data.workspace-interactions-test]
|
||||
[frontend-tests.data.workspace-mcp-test]
|
||||
[frontend-tests.data.workspace-media-test]
|
||||
@ -85,6 +86,7 @@
|
||||
'frontend-tests.data.uploads-test
|
||||
'frontend-tests.data.viewer-test
|
||||
'frontend-tests.data.workspace-colors-test
|
||||
'frontend-tests.data.workspace-comments-test
|
||||
'frontend-tests.data.workspace-interactions-test
|
||||
'frontend-tests.data.workspace-mcp-test
|
||||
'frontend-tests.data.workspace-media-test
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user