mirror of
https://github.com/penpot/penpot.git
synced 2026-09-24 04:46:14 +00:00
🐛 Fix Japanese IME Enter duplication in comment input (#11768)
* 🐛 Fix Japanese IME Enter duplication in comment input Comment keydown handler treated every Enter as a Penpot line-break action, so confirming an IME composition duplicated the text with an extra newline and a zero-width space. Guard the whole custom keydown processing while the event belongs to an active IME composition, mirroring the v3 text-editor precedent. Closes #11757 Signed-off-by: Junsoo Choi <junsoo1172@gmail.com> AI-assisted-by: muse-spark-1.3-contributor * 🐛 Keep composing Escape from closing comment thread The parent floating-thread keydown handler closed the thread on every Escape, including one that cancels an active IME composition. Apply the same composition guard so composing Escape stays owned by the IME while plain Escape still closes the thread. Closes #11757 Signed-off-by: Junsoo Choi <junsoo1172@gmail.com> AI-assisted-by: muse-spark-1.3-contributor * 🐛 Test comment IME guard through key-action resolver The composition predicate test only verified the predicate itself, so a guard moved to the wrong place or a handler bypassing it would stay green. Resolve comment and thread keydowns through a pure resolve-comment-key-action seam and verify the observable behavior: composing keys yield :ime-owned with zero Penpot side effects while the same plain keys keep their existing commands. Closes #11757 Signed-off-by: Junsoo Choi <junsoo1172@gmail.com> AI-assisted-by: muse-spark-1.3-contributor * 🐛 Test comment IME handlers through direct calls The key-action resolver only verified a return value, so handler wiring regressions would stay green, and it read the mention snapshot before handle-select ran, changing the existing ordering. Remove the resolver, extract the two handler bodies as directly callable fns with the original select-first ordering, and assert the fired side effects instead. Closes #11757 Signed-off-by: Junsoo Choi <junsoo1172@gmail.com> AI-assisted-by: muse-spark-1.3-contributor * 📚 Remove text-editor v3 references from comment IME docs The comment IME guard is specific to the comment editor, so the docstrings no longer present it as following a v3 text-editor or render-engine precedent. Reviewers read that wording as tying this comment bug fix to unrelated subsystems. Only docstring text changes; handler logic and test assertions are untouched. Closes #11757 AI-assisted-by: deepseek-v4.1-flash Signed-off-by: makesomethingshit <junsoo1172@gmail.com> * 🐛 Review comments, and fix edge case --------- Signed-off-by: Junsoo Choi <junsoo1172@gmail.com> Signed-off-by: makesomethingshit <junsoo1172@gmail.com> Co-authored-by: alonso.torres <alonso.torres@kaleidos.net>
This commit is contained in:
makesomethingshit
GitHub
parent
6031f6c318
commit
d7527b63a2
@ -152,6 +152,62 @@
|
||||
(and (= (count content) 1)
|
||||
(= (first content) zero-width-space)))))
|
||||
|
||||
(defn- composing-event?
|
||||
"True when a keydown belongs to an active IME composition. keyCode 229
|
||||
covers browsers that send the confirming keydown with isComposing
|
||||
false."
|
||||
[^js event]
|
||||
(or (.. event -nativeEvent -isComposing)
|
||||
(= 229 (.-keyCode event))))
|
||||
|
||||
(defn handle-comment-input-key-down
|
||||
"Keydown handler for the comment input. Does nothing while an IME
|
||||
composition owns the key. Otherwise calls `on-select` and then, with
|
||||
the caret span from `get-span`, routes the key: mention-panel keys
|
||||
while a mention is open, Escape, mod+Enter, Enter and Backspace.
|
||||
|
||||
`cur-mention` holds the open mention text or nil and is read after
|
||||
`on-select`, which may update it. `on-newline` and `on-backspace`
|
||||
receive `{:event :node :span-node :offset}`."
|
||||
[event {:keys [node cur-mention mentions-s on-select get-span
|
||||
on-esc on-ctrl-enter on-newline on-backspace]}]
|
||||
(when-not (composing-event? event)
|
||||
(on-select event)
|
||||
(when (some? node)
|
||||
(when-let [[span-node offset] (get-span node)]
|
||||
(let [mention-cmd (when @cur-mention
|
||||
(cond
|
||||
(kbd/enter? event) :insert-selected-mention
|
||||
(kbd/down-arrow? event) :insert-next-mention
|
||||
(kbd/up-arrow? event) :insert-prev-mention
|
||||
(kbd/esc? event) :hide-mentions))
|
||||
ctx {:event event :node node :span-node span-node :offset offset}]
|
||||
(cond
|
||||
(some? mention-cmd)
|
||||
(do (dom/prevent-default event)
|
||||
(dom/stop-propagation event)
|
||||
(rx/push! mentions-s {:type mention-cmd}))
|
||||
|
||||
(and (kbd/esc? event) (fn? on-esc))
|
||||
(on-esc event)
|
||||
|
||||
(and (kbd/mod? event) (kbd/enter? event) (fn? on-ctrl-enter))
|
||||
(on-ctrl-enter event)
|
||||
|
||||
(kbd/enter? event)
|
||||
(on-newline ctx)
|
||||
|
||||
(kbd/backspace? event)
|
||||
(on-backspace ctx)))))))
|
||||
|
||||
(defn handle-thread-key-down
|
||||
"Keydown handler for the floating thread: calls `on-close` on Escape
|
||||
unless an IME composition owns the key."
|
||||
[event on-close]
|
||||
(when (and (kbd/esc? event)
|
||||
(not (composing-event? event)))
|
||||
(on-close event)))
|
||||
|
||||
;; Component that renders the component content
|
||||
(mf/defc comment-content*
|
||||
{::mf/private true}
|
||||
@ -356,59 +412,35 @@
|
||||
(mf/use-fn
|
||||
(mf/deps on-esc on-ctrl-enter handle-select handle-input)
|
||||
(fn [event]
|
||||
(handle-select event)
|
||||
(when-let [node (mf/ref-val local-ref)]
|
||||
(when-let [[span-node offset] (current-text-node node)]
|
||||
(cond
|
||||
(and @cur-mention (kbd/enter? event))
|
||||
(do (dom/prevent-default event)
|
||||
(dom/stop-propagation event)
|
||||
(rx/push! mentions-s {:type :insert-selected-mention}))
|
||||
|
||||
(and @cur-mention (kbd/down-arrow? event))
|
||||
(do (dom/prevent-default event)
|
||||
(dom/stop-propagation event)
|
||||
(rx/push! mentions-s {:type :insert-next-mention}))
|
||||
|
||||
(and @cur-mention (kbd/up-arrow? event))
|
||||
(do (dom/prevent-default event)
|
||||
(dom/stop-propagation event)
|
||||
(rx/push! mentions-s {:type :insert-prev-mention}))
|
||||
|
||||
(and @cur-mention (kbd/esc? event))
|
||||
(do (dom/prevent-default event)
|
||||
(dom/stop-propagation event)
|
||||
(rx/push! mentions-s {:type :hide-mentions}))
|
||||
|
||||
(and (kbd/esc? event) (fn? on-esc))
|
||||
(on-esc event)
|
||||
|
||||
(and (kbd/mod? event) (kbd/enter? event) (fn? on-ctrl-enter))
|
||||
(on-ctrl-enter event)
|
||||
|
||||
(kbd/enter? event)
|
||||
(let [sel (wapi/get-selection)
|
||||
range (.getRangeAt sel 0)]
|
||||
(handle-comment-input-key-down
|
||||
event
|
||||
{:node (mf/ref-val local-ref)
|
||||
:cur-mention cur-mention
|
||||
:mentions-s mentions-s
|
||||
:on-select handle-select
|
||||
:get-span current-text-node
|
||||
:on-esc on-esc
|
||||
:on-ctrl-enter on-ctrl-enter
|
||||
:on-newline
|
||||
(fn [{:keys [event span-node offset]}]
|
||||
(let [range (.getRangeAt (wapi/get-selection) 0)]
|
||||
(dom/prevent-default event)
|
||||
(dom/stop-propagation event)
|
||||
(.deleteContents range)
|
||||
(handle-input)
|
||||
(let [txt (.-textContent span-node)]
|
||||
(dom/set-html! span-node (dm/str (dom/escape-html (subs txt 0 offset)) "\n" zero-width-space (dom/escape-html (subs txt offset))))
|
||||
(wapi/set-cursor! span-node (inc offset))
|
||||
(handle-input))))
|
||||
:on-backspace
|
||||
(fn [{:keys [event node span-node offset]}]
|
||||
(let [prev-node (get-prev-node node span-node)]
|
||||
(when (and (some? prev-node)
|
||||
(= "mention" (dom/get-data prev-node "type"))
|
||||
(= offset 1))
|
||||
(dom/prevent-default event)
|
||||
(dom/stop-propagation event)
|
||||
(let [[span-node offset] (current-text-node node)]
|
||||
(.deleteContents range)
|
||||
(handle-input)
|
||||
|
||||
(when span-node
|
||||
(let [txt (.-textContent span-node)]
|
||||
(dom/set-html! span-node (dm/str (dom/escape-html (subs txt 0 offset)) "\n" zero-width-space (dom/escape-html (subs txt offset))))
|
||||
(wapi/set-cursor! span-node (inc offset))
|
||||
(handle-input)))))
|
||||
|
||||
(kbd/backspace? event)
|
||||
(let [prev-node (get-prev-node node span-node)]
|
||||
(when (and (some? prev-node)
|
||||
(= "mention" (dom/get-data prev-node "type"))
|
||||
(= offset 1))
|
||||
(dom/prevent-default event)
|
||||
(dom/stop-propagation event)
|
||||
(.remove prev-node))))))))]
|
||||
(.remove prev-node))))})))]
|
||||
|
||||
(mf/with-layout-effect [autofocus]
|
||||
(when ^boolean autofocus
|
||||
@ -1022,8 +1054,6 @@
|
||||
[thread-id]
|
||||
(l/derived (l/in [:comments thread-id]) st/state))
|
||||
|
||||
|
||||
|
||||
(mf/defc comment-floating-thread*
|
||||
{::mf/wrap [mf/memo]}
|
||||
[{:keys [thread zoom origin position-modifier viewport]}]
|
||||
@ -1066,10 +1096,12 @@
|
||||
on-key-down
|
||||
(mf/use-fn
|
||||
(fn [event]
|
||||
(when (kbd/esc? event)
|
||||
(dom/prevent-default event)
|
||||
(dom/stop-propagation event)
|
||||
(st/emit! (dcm/close-thread)))))
|
||||
(handle-thread-key-down
|
||||
event
|
||||
(fn [event]
|
||||
(dom/prevent-default event)
|
||||
(dom/stop-propagation event)
|
||||
(st/emit! (dcm/close-thread))))))
|
||||
|
||||
on-cancel
|
||||
(mf/use-fn #(st/emit! (dcm/close-thread)))]
|
||||
|
||||
@ -93,6 +93,7 @@
|
||||
[frontend-tests.tokens.workspace-tokens-remap-test]
|
||||
[frontend-tests.ui.check-updates-test]
|
||||
[frontend-tests.ui.colorpicker-token-set-order-test]
|
||||
[frontend-tests.ui.comment-input-ime-test]
|
||||
[frontend-tests.ui.comments-clustering-test]
|
||||
[frontend-tests.ui.comments-position-modifier-test]
|
||||
[frontend-tests.ui.ds-controls-numeric-input-test]
|
||||
@ -214,6 +215,7 @@
|
||||
'frontend-tests.tokens.workspace-tokens-remap-test
|
||||
'frontend-tests.ui.check-updates-test
|
||||
'frontend-tests.ui.colorpicker-token-set-order-test
|
||||
'frontend-tests.ui.comment-input-ime-test
|
||||
'frontend-tests.ui.comments-clustering-test
|
||||
'frontend-tests.ui.comments-position-modifier-test
|
||||
'frontend-tests.ui.ds-controls-numeric-input-test
|
||||
|
||||
129
frontend/test/frontend_tests/ui/comment_input_ime_test.cljs
Normal file
129
frontend/test/frontend_tests/ui/comment_input_ime_test.cljs
Normal file
@ -0,0 +1,129 @@
|
||||
;; 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 SUBSIDIARY SL
|
||||
|
||||
(ns frontend-tests.ui.comment-input-ime-test
|
||||
"Keydown handling of the comment input and floating thread: keys owned
|
||||
by an IME composition run nothing, all other keys keep their
|
||||
commands."
|
||||
(:require
|
||||
[app.main.ui.comments :as cmt]
|
||||
[beicon.v2.core :as rx]
|
||||
[cljs.test :as t :include-macros true]))
|
||||
|
||||
(defn- keydown
|
||||
"Fake keyboard event. preventDefault/stopPropagation append to `log`."
|
||||
[key {:keys [composing? key-code mod? log]}]
|
||||
#js {:key key
|
||||
:keyCode (or key-code 0)
|
||||
:ctrlKey (boolean mod?)
|
||||
:metaKey (boolean mod?)
|
||||
:preventDefault #(some-> log (swap! conj :prevent-default))
|
||||
:stopPropagation #(some-> log (swap! conj :stop-propagation))
|
||||
:nativeEvent #js {:isComposing (boolean composing?)}})
|
||||
|
||||
(def ^:private composition-modes
|
||||
{"isComposing" {:composing? true}
|
||||
"keyCode 229" {:key-code 229}})
|
||||
|
||||
(def ^:private input-keys
|
||||
["Enter" "Escape" "ArrowDown" "ArrowUp" "Backspace"])
|
||||
|
||||
(defn- run-input!
|
||||
"Runs the comment-input handler for `key` and returns the side-effect
|
||||
log. Mention-panel commands are observed on a real subject and the
|
||||
open mention lives in an atom, as in the component."
|
||||
[key {:keys [event-opts mention select-sets-mention node span on-esc? on-ctrl-enter?]
|
||||
:or {node :node span [:span 3] on-esc? true on-ctrl-enter? true}}]
|
||||
(let [log (atom [])
|
||||
cur-mention (atom mention)
|
||||
mentions-s (rx/subject)
|
||||
sub (rx/sub! mentions-s #(swap! log conj [:mention (:type %)]))
|
||||
event (keydown key (assoc event-opts :log log))]
|
||||
(cmt/handle-comment-input-key-down
|
||||
event
|
||||
{:node node
|
||||
:cur-mention cur-mention
|
||||
:mentions-s mentions-s
|
||||
:on-select (fn [e]
|
||||
(t/is (identical? event e))
|
||||
(swap! log conj :select)
|
||||
(some->> select-sets-mention (reset! cur-mention)))
|
||||
:get-span (fn [n]
|
||||
(t/is (= node n))
|
||||
span)
|
||||
:on-esc (when on-esc? #(swap! log conj :on-esc))
|
||||
:on-ctrl-enter (when on-ctrl-enter? #(swap! log conj :on-ctrl-enter))
|
||||
:on-newline #(swap! log conj [:newline (dissoc % :event)])
|
||||
:on-backspace #(swap! log conj [:backspace (dissoc % :event)])})
|
||||
(rx/dispose! sub)
|
||||
@log))
|
||||
|
||||
(defn- run-thread!
|
||||
[key event-opts]
|
||||
(let [log (atom [])]
|
||||
(cmt/handle-thread-key-down (keydown key event-opts)
|
||||
#(swap! log conj :close))
|
||||
@log))
|
||||
|
||||
(def ^:private ctx {:node :node :span-node :span :offset 3})
|
||||
|
||||
(t/deftest composing-keys-run-nothing
|
||||
(doseq [[mode opts] composition-modes
|
||||
key input-keys
|
||||
mention [nil "@bob"]]
|
||||
(t/testing (str key " via " mode ", mention " (pr-str mention))
|
||||
(t/is (= [] (run-input! key {:event-opts opts :mention mention}))))))
|
||||
|
||||
(t/deftest plain-keys-keep-their-commands
|
||||
(t/testing "Enter inserts a line break at the caret span"
|
||||
(t/is (= [:select [:newline ctx]] (run-input! "Enter" {}))))
|
||||
(t/testing "mod+Enter submits"
|
||||
(t/is (= [:select :on-ctrl-enter]
|
||||
(run-input! "Enter" {:event-opts {:mod? true}}))))
|
||||
(t/testing "mod+Enter without on-ctrl-enter falls back to a line break"
|
||||
(t/is (= [:select [:newline ctx]]
|
||||
(run-input! "Enter" {:event-opts {:mod? true} :on-ctrl-enter? false}))))
|
||||
(t/testing "Escape calls on-esc"
|
||||
(t/is (= [:select :on-esc] (run-input! "Escape" {}))))
|
||||
(t/testing "Escape without on-esc does nothing else"
|
||||
(t/is (= [:select] (run-input! "Escape" {:on-esc? false}))))
|
||||
(t/testing "Backspace runs the mention-deletion check"
|
||||
(t/is (= [:select [:backspace ctx]] (run-input! "Backspace" {}))))
|
||||
(t/testing "other keys only sync the selection"
|
||||
(t/is (= [:select] (run-input! "a" {})))))
|
||||
|
||||
(t/deftest open-mention-routes-panel-keys
|
||||
(doseq [[key cmd] {"Enter" :insert-selected-mention
|
||||
"ArrowDown" :insert-next-mention
|
||||
"ArrowUp" :insert-prev-mention
|
||||
"Escape" :hide-mentions}]
|
||||
(t/testing key
|
||||
(t/is (= [:select :prevent-default :stop-propagation [:mention cmd]]
|
||||
(run-input! key {:mention "@bob"})))))
|
||||
(t/testing "Backspace is not a panel key"
|
||||
(t/is (= [:select [:backspace ctx]]
|
||||
(run-input! "Backspace" {:mention "@bob"})))))
|
||||
|
||||
(t/deftest mention-is-read-after-select
|
||||
(t/testing "a mention opened by on-select routes the same key"
|
||||
(t/is (= [:select :prevent-default :stop-propagation
|
||||
[:mention :insert-selected-mention]]
|
||||
(run-input! "Enter" {:select-sets-mention "@new"})))))
|
||||
|
||||
(t/deftest missing-caret-target-runs-only-select
|
||||
(t/testing "no input node"
|
||||
(t/is (= [:select] (run-input! "Enter" {:node nil}))))
|
||||
(t/testing "caret outside a text span"
|
||||
(t/is (= [:select] (run-input! "Enter" {:span nil})))))
|
||||
|
||||
(t/deftest floating-thread-escape
|
||||
(doseq [[mode opts] composition-modes]
|
||||
(t/testing (str "Escape via " mode " keeps the thread open")
|
||||
(t/is (= [] (run-thread! "Escape" opts)))))
|
||||
(t/testing "plain Escape closes the thread"
|
||||
(t/is (= [:close] (run-thread! "Escape" {}))))
|
||||
(t/testing "other keys do nothing"
|
||||
(t/is (= [] (run-thread! "Enter" {})))))
|
||||
Loading…
x
Reference in New Issue
Block a user