🐛 Fix problems with comments clusters (#10543)

This commit is contained in:
Alonso Torres 2026-07-09 11:37:45 +02:00 committed by GitHub
parent 54bef496f1
commit 80d688be93
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 578 additions and 174 deletions

View File

@ -503,7 +503,7 @@
(-> state (-> state
(update :comments-local assoc :open id) (update :comments-local assoc :open id)
(update :comments-local assoc :options nil) (update :comments-local assoc :options nil)
(update :comments-local dissoc :draft))))) (update :comments-local dissoc :draft :expanded)))))
(defn close-thread (defn close-thread
[] []
@ -511,8 +511,26 @@
ptk/UpdateEvent ptk/UpdateEvent
(update [_ state] (update [_ state]
(-> state (-> state
(update :comments-local dissoc :open :draft :options :expanded)))))
(defn expand-comment-group
"Temporarily mark a proximity cluster of threads as expanded so its bubbles
can be laid out visually without altering their stored positions."
[thread-ids]
(ptk/reify ::expand-comment-group
ptk/UpdateEvent
(update [_ state]
(-> state
(update :comments-local assoc :expanded (set thread-ids))
(update :comments-local dissoc :open :draft :options))))) (update :comments-local dissoc :open :draft :options)))))
(defn collapse-comment-group
[]
(ptk/reify ::collapse-comment-group
ptk/UpdateEvent
(update [_ state]
(update state :comments-local dissoc :expanded))))
(defn update-filters (defn update-filters
[{:keys [mode show list] :as params}] [{:keys [mode show list] :as params}]
(ptk/reify ::update-filters (ptk/reify ::update-filters

View File

@ -25,6 +25,7 @@
[app.main.data.workspace.edition :as dwe] [app.main.data.workspace.edition :as dwe]
[app.main.data.workspace.layout :as dwlo] [app.main.data.workspace.layout :as dwlo]
[app.main.data.workspace.selection :as dws] [app.main.data.workspace.selection :as dws]
[app.main.data.workspace.viewport-wasm :as dwvw]
[app.main.data.workspace.zoom :as dwz] [app.main.data.workspace.zoom :as dwz]
[app.main.repo :as rp] [app.main.repo :as rp]
[app.main.router :as rt] [app.main.router :as rt]
@ -79,15 +80,16 @@
(let [local (:comments-local state) (let [local (:comments-local state)
comments-mode? (= :comments (get-in state [:workspace-drawing :tool]))] comments-mode? (= :comments (get-in state [:workspace-drawing :tool]))]
(cond (cond
(:draft local) (rx/of (dcmt/close-thread)) (:draft local) (rx/of (dcmt/close-thread))
(:open local) (rx/of (dcmt/close-thread)) (:open local) (rx/of (dcmt/close-thread))
(:expanded local) (rx/of (dcmt/collapse-comment-group))
;; Only clear edition / deselect on interrupt while the comments ;; Only clear edition / deselect on interrupt while the comments
;; tool is active. When comments are merely visible during design, ;; tool is active. When comments are merely visible during design,
;; `select-shape` emits `:interrupt` and this would otherwise wipe ;; `select-shape` emits `:interrupt` and this would otherwise wipe
;; the freshly selected shape, breaking click selection. ;; the freshly selected shape, breaking click selection.
comments-mode? (rx/of (dwe/clear-edition-mode) comments-mode? (rx/of (dwe/clear-edition-mode)
(dws/deselect-all true)) (dws/deselect-all true))
:else (rx/empty)))))) :else (rx/empty))))))
;; Event responsible of the what should be executed when user clicked ;; Event responsible of the what should be executed when user clicked
;; on the comments layer. An option can be create a new draft thread, ;; on the comments layer. An option can be create a new draft thread,
@ -98,17 +100,28 @@
(ptk/reify ::handle-comment-layer-click (ptk/reify ::handle-comment-layer-click
ptk/WatchEvent ptk/WatchEvent
(watch [_ state _] (watch [_ state _]
(if (not= :comments (get-in state [:workspace-drawing :tool])) (let [local (:comments-local state)
(rx/empty) comments-mode? (= :comments (get-in state [:workspace-drawing :tool]))]
(let [local (:comments-local state)] (cond
(if (some? (:open local)) ;; A click anywhere collapses a temporarily separated cluster,
(rx/of (dcmt/close-thread)) ;; regardless of the active tool. Opening a thread clears :expanded,
(let [page-id (:current-page-id state) ;; so this never fires while a comment is open.
file-id (:current-file-id state) (some? (:expanded local))
params {:position position (rx/of (dcmt/collapse-comment-group))
:page-id page-id
:file-id file-id}] (not comments-mode?)
(rx/of (dcmt/create-draft params))))))))) (rx/empty)
(some? (:open local))
(rx/of (dcmt/close-thread))
:else
(let [page-id (:current-page-id state)
file-id (:current-file-id state)
params {:position position
:page-id page-id
:file-id file-id}]
(rx/of (dcmt/create-draft params))))))))
(defn center-to-comment-thread (defn center-to-comment-thread
[{:keys [position] :as thread}] [{:keys [position] :as thread}]
@ -127,7 +140,11 @@
nh (- (/ (:height vbox) 2) ph) nh (- (/ (:height vbox) 2) ph)
nx (- (:x position) nw) nx (- (:x position) nw)
ny (- (:y position) nh)] ny (- (:y position) nh)]
(update local :vbox assoc :x nx :y ny))))))) (update local :vbox assoc :x nx :y ny)))))
ptk/EffectEvent
(effect [_ state _]
(dwvw/maybe-sync-workspace-local-viewport! state))))
(defn- set-comment-thread (defn- set-comment-thread
"Stores the comment thread in the workspace state so its bubble re-renders." "Stores the comment thread in the workspace state so its bubble re-renders."
@ -305,6 +322,25 @@
distance-overlap 32] distance-overlap 32]
(< distance-zoom distance-overlap))) (< distance-zoom distance-overlap)))
(defn group-bubbles
"Group bubbles into vectors by proximity: each group holds threads whose
bubbles overlap at the given `zoom`."
[zoom circles]
(letfn [(overlaps-group? [current group]
(some #(overlap-bubbles? zoom current %) group))
(find-overlapping-group [groups current]
(some #(when (overlaps-group? current %) %) groups))
(add-to-group [groups target current]
(map #(if (= % target) (cons current %) %) groups))
(assign [groups current]
(if-let [group (find-overlapping-group groups current)]
(add-to-group groups group current)
(cons [current] groups)))]
(reduce assign [] circles)))
(defn- calculate-zoom-scale-to-ungroup-current-bubble (defn- calculate-zoom-scale-to-ungroup-current-bubble
"Calculate the minimum zoom scale needed to keep the current bubble ungrouped from the rest" "Calculate the minimum zoom scale needed to keep the current bubble ungrouped from the rest"
[zoom thread threads] [zoom thread threads]
@ -373,7 +409,7 @@
(rx/empty)) (rx/empty))
(->> (rx/of (->> (rx/of
(dwd/select-for-drawing :comments) (dwd/select-for-drawing :comments)
(set-zoom-to-separate-grouped-bubbles thread) ;; Center on the comment (no zoom) and open its thread.
(center-to-comment-thread thread) (center-to-comment-thread thread)
(with-meta (dcmt/open-thread thread) {::ev/origin "workspace"})) (with-meta (dcmt/open-thread thread) {::ev/origin "workspace"}))
(rx/observe-on :async)))))) (rx/observe-on :async))))))

View File

@ -18,8 +18,6 @@
[app.main.data.comments :as dcm] [app.main.data.comments :as dcm]
[app.main.data.modal :as modal] [app.main.data.modal :as modal]
[app.main.data.workspace.comments :as dwcm] [app.main.data.workspace.comments :as dwcm]
[app.main.data.workspace.viewport :as dwv]
[app.main.data.workspace.zoom :as dwz]
[app.main.refs :as refs] [app.main.refs :as refs]
[app.main.store :as st] [app.main.store :as st]
[app.main.ui.components.dropdown :refer [dropdown]] [app.main.ui.components.dropdown :refer [dropdown]]
@ -704,15 +702,21 @@
(mf/defc comment-reply-form* (mf/defc comment-reply-form*
{::mf/private true} {::mf/private true}
[{:keys [on-submit]}] [{:keys [on-submit on-cancel]}]
(let [content (mf/use-state "") (let [content (mf/use-state "")
disabled? (or (blank-content? @content) disabled? (or (blank-content? @content)
(exceeds-length? @content)) (exceeds-length? @content))
;; Contexts without a global interrupt handler (e.g. the viewer) pass an
;; explicit cancel; otherwise fall back to the interrupt cycle.
on-cancel on-cancel
(mf/use-fn (mf/use-fn
#(st/emit! :interrupt)) (mf/deps on-cancel)
(fn []
(if (fn? on-cancel)
(on-cancel)
(st/emit! :interrupt))))
on-change on-change
(mf/use-fn (mf/use-fn
@ -799,8 +803,10 @@
(some? position-modifier) (some? position-modifier)
(gpt/transform position-modifier)) (gpt/transform position-modifier))
content (:content draft) content (:content draft)
bubble-margin (gpt/point 0 0)
;; Keep the draft bubble centered on the comment position (matching a
;; created bubble) while the input box is offset to the side.
bubble-margin (gpt/point 24 24)
pos (offset-position position viewport zoom bubble-margin) pos (offset-position position viewport zoom bubble-margin)
margin-x (* (:x bubble-margin) (if (= (:h-dir pos) :left) -1 1)) margin-x (* (:x bubble-margin) (if (= (:h-dir pos) :left) -1 1))
@ -808,6 +814,9 @@
pos-x (+ (* (:x pos) zoom) margin-x) pos-x (+ (* (:x pos) zoom) margin-x)
pos-y (- (* (:y pos) zoom) margin-y) pos-y (- (* (:y pos) zoom) margin-y)
bubble-x (floor (* (:x position) zoom))
bubble-y (floor (* (:y position) zoom))
disabled? (or (blank-content? content) disabled? (or (blank-content? content)
(exceeds-length? content)) (exceeds-length? content))
@ -833,39 +842,36 @@
(on-submit draft)))] (on-submit draft)))]
[:> (mf/provider mentions-context) {:value mentions-s} [:> (mf/provider mentions-context) {:value mentions-s}
[:div {:class (stl/css-case :floating-thread-draft-wrapper true [:div {:class (stl/css :floating-preview-wrapper :floating-preview-bubble)
:data-testid "floating-thread-bubble"
:style {:top (dm/str bubble-y "px")
:left (dm/str bubble-x "px")}
:on-click dom/stop-propagation}
[:> comment-avatar* {:class (stl/css :avatar-lg)
:image (cfg/resolve-profile-photo-url profile)}]]
[:div {:class (stl/css-case :floating-thread-draft-inner-wrapper true
:cursor-auto true
:left (= (:h-dir pos) :left) :left (= (:h-dir pos) :left)
:top (= (:v-dir pos) :top)) :top (= (:v-dir pos) :top))
:style {:top (str pos-y "px") :style {:top (str pos-y "px")
:left (str pos-x "px")}} :left (str pos-x "px")}
[:div :on-click dom/stop-propagation}
{:data-testid "floating-thread-bubble" [:div {:class (stl/css :form)}
:style {:top (str pos-y "px") [:> comment-input*
:left (str pos-x "px")} {:placeholder (tr "labels.write-new-comment")
:on-click dom/stop-propagation} :value (or content "")
[:> comment-avatar* {:class (stl/css :avatar-lg) :autofocus true
:image (cfg/resolve-profile-photo-url profile)}]] :on-esc on-esc
[:div {:class (stl/css :floating-thread-draft-inner-wrapper :on-change on-change
:cursor-auto) :on-ctrl-enter on-submit*}]
:style {:top (str (- pos-y 24) "px") (when (exceeds-length? content)
:left (str (+ pos-x 28) "px")} [:div {:class (stl/css :error-text)}
(tr "errors.character-limit-exceeded")])
:on-click dom/stop-propagation} [:> comment-form-buttons* {:on-submit on-submit*
[:div {:class (stl/css :form)} :on-cancel on-esc
[:> comment-input* :is-disabled disabled?}]]
{:placeholder (tr "labels.write-new-comment") [:> mentions-panel*]]]))
:value (or content "")
:autofocus true
:on-esc on-esc
:on-change on-change
:on-ctrl-enter on-submit*}]
(when (exceeds-length? content)
[:div {:class (stl/css :error-text)}
(tr "errors.character-limit-exceeded")])
[:> comment-form-buttons* {:on-submit on-submit*
:on-cancel on-esc
:is-disabled disabled?}]]
[:> mentions-panel*]]]]))
(mf/defc comment-floating-thread-header* (mf/defc comment-floating-thread-header*
{::mf/private true} {::mf/private true}
@ -1057,7 +1063,10 @@
(mf/use-fn (mf/use-fn
(mf/deps thread) (mf/deps thread)
(fn [content] (fn [content]
(st/emit! (dcm/add-comment thread content))))] (st/emit! (dcm/add-comment thread content))))
on-cancel
(mf/use-fn #(st/emit! (dcm/close-thread)))]
(mf/with-effect [thread-id] (mf/with-effect [thread-id]
(st/emit! (dcm/retrieve-comments thread-id))) (st/emit! (dcm/retrieve-comments thread-id)))
@ -1092,60 +1101,65 @@
[:* {:key (dm/str (:id item))} [:* {:key (dm/str (:id item))}
[:> comment-floating-thread-item* {:comment item}]])] [:> comment-floating-thread-item* {:comment item}]])]
[:> comment-reply-form* {:on-submit on-submit}] [:> comment-reply-form* {:on-submit on-submit
:on-cancel (when (= origin :viewer) on-cancel)}]
[:> mentions-panel*]])])) [:> mentions-panel*]])]))
(defn group-bubbles ;; Screen-space gap (px) between concentric rings of an expanded cluster.
"Group bubbles in different vectors by proximity" (def ^:private expanded-ring-gap 44)
([zoom circles]
(group-bubbles zoom circles [] []))
([zoom circles visited groups] ;; Number of bubbles that fit in the innermost ring; each further ring
(if (empty? circles) ;; grows its capacity proportionally to its circumference.
groups (def ^:private expanded-ring-base 6)
(let [current (first circles)
remaining (rest circles)
overlapping-group (some (fn [group]
(when (some (partial dwcm/overlap-bubbles? zoom current) group) group))
groups)]
(if overlapping-group
(group-bubbles zoom remaining visited (map (fn [group]
(if (= group overlapping-group)
(cons current group)
group))
groups))
(group-bubbles zoom remaining visited (cons [current] groups)))))))
(defn- inside-vbox? (defn- expanded-ring-slot
"Checks if a bubble or a bubble group is inside a viewbox" "Return [ring slot capacity] placing the bubble at index `i` (0-based) into
[thread-group wl] concentric rings, filling the innermost ring first."
(let [vbox (:vbox wl) [i]
positions (mapv :position thread-group) (loop [ring 1
position (gpt/center-points positions) i i]
pos-x (:x position) (let [capacity (* expanded-ring-base ring)]
pos-y (:y position) (if (< i capacity)
x1 (:x vbox) [ring i capacity]
y1 (:y vbox) (recur (inc ring) (- i capacity))))))
x2 (+ x1 (:width vbox))
y2 (+ y1 (:height vbox))]
(and (> x2 pos-x x1) (> y2 pos-y y1))))
(defn- calculate-zoom-scale (defn- expanded-ring-vector
"Calculates the zoom level needed to ungroup the largest number of bubbles while "Pure ring displacement (px) around the cluster center for the bubble at index
keeping them all visible in the viewbox." `i` (0-based), laid out into concentric rings, innermost first."
[position zoom threads wl] [i]
(let [num-threads (count threads) (let [[ring slot capacity] (expanded-ring-slot i)
grouped-threads (group-bubbles zoom threads) ;; Start each ring at the top and stagger alternate rings by half a
num-grouped-threads (count grouped-threads) ;; slot so bubbles don't line up radially.
zoom-scale-step 1.75 angle (+ (* (/ slot capacity) 2 mth/PI)
scaled-zoom (* zoom zoom-scale-step) (- (/ mth/PI 2))
zoomed-wl (dwz/impl-update-zoom wl position scaled-zoom) (if (even? ring) (/ mth/PI capacity) 0))
outside-vbox? (complement inside-vbox?)] radius (* ring expanded-ring-gap)]
(if (or (= num-threads num-grouped-threads) (gpt/point (* radius (mth/cos angle))
(some #(outside-vbox? % zoomed-wl) grouped-threads)) (* radius (mth/sin angle)))))
zoom
(calculate-zoom-scale position scaled-zoom threads zoomed-wl)))) (defn expanded-group-center
"Cluster center point (stored coordinates) shared by all bubbles of a group."
[thread-group]
(gpt/center-points (mapv :position thread-group)))
(defn expanded-group-offsets
"Return a seq of [thread offset ring] triples laying each thread of
`thread-group` into a centered concentric-ring layout, leaving stored
positions untouched. `offset` is the total screen-space displacement from the
bubble's own position; `ring` is just the displacement from the cluster
center (used to animate the fan-out)."
[zoom thread-group]
(let [threads (sort-by :seqn thread-group)
center (expanded-group-center threads)]
(map-indexed
(fn [i thread]
(let [position (:position thread)
ring (expanded-ring-vector i)
base (gpt/point (* (- (:x center) (:x position)) zoom)
(* (- (:y center) (:y position)) zoom))]
[thread (gpt/add base ring) ring]))
threads)))
(mf/defc comment-floating-group* (mf/defc comment-floating-group*
{::mf/wrap [mf/memo]} {::mf/wrap [mf/memo]}
@ -1167,16 +1181,15 @@
;; Click-through while transforming a shape, so it doesn't capture the drag ;; Click-through while transforming a shape, so it doesn't capture the drag
dragging? (some? (mf/deref refs/current-transform)) dragging? (some? (mf/deref refs/current-transform))
thread-ids (mf/with-memo [thread-group]
(into #{} (map :id) thread-group))
on-click on-click
(mf/use-fn (mf/use-fn
(mf/deps thread-group position zoom) (mf/deps thread-ids)
(fn [] (fn [event]
(let [wl (deref refs/workspace-local) (dom/stop-propagation event)
centered-wl (dwv/calculate-centered-viewbox wl position) (st/emit! (dcm/expand-comment-group thread-ids))))]
updated-zoom (calculate-zoom-scale position zoom thread-group centered-wl)
scale-zoom (/ updated-zoom zoom)]
(st/emit! (dwv/update-viewport-position-center position)
(dwz/set-zoom position scale-zoom)))))]
[:div {:style {:top (dm/str pos-y "px") [:div {:style {:top (dm/str pos-y "px")
:left (dm/str pos-x "px") :left (dm/str pos-x "px")
@ -1189,9 +1202,31 @@
:data-testid (dm/str "floating-thread-bubble-" test-id)} :data-testid (dm/str "floating-thread-bubble-" test-id)}
num-threads]])) num-threads]]))
(mf/defc comment-floating-ghost*
"Dashed, semi-transparent placeholder shown at the cluster center while its
bubbles are fanned out into the expanded ring."
{::mf/wrap [mf/memo]
::mf/private true}
[{:keys [thread-group zoom position-modifier]}]
(let [center (expanded-group-center thread-group)
center (cond-> center
(some? position-modifier)
(gpt/transform position-modifier))
pos-x (floor (* (:x center) zoom))
pos-y (floor (* (:y center) zoom))
num-threads (str (count thread-group))]
[:div {:style {:top (dm/str pos-y "px")
:left (dm/str pos-x "px")
:pointer-events "none"}
:class (stl/css :floating-preview-wrapper :floating-preview-bubble :floating-preview-ghost)}
[:> comment-avatar*
{:class (stl/css :avatar-lg)
:variant "read"}
num-threads]]))
(mf/defc comment-floating-bubble* (mf/defc comment-floating-bubble*
{::mf/wrap [mf/memo]} {::mf/wrap [mf/memo]}
[{:keys [thread zoom is-open on-click origin position-modifier]}] [{:keys [thread zoom is-open on-click origin position-modifier offset ring]}]
(let [owner (mf/with-memo [thread] (let [owner (mf/with-memo [thread]
(dcm/get-owner thread)) (dcm/get-owner thread))
@ -1202,6 +1237,10 @@
frame-id (:frame-id thread) frame-id (:frame-id thread)
;; A bubble with `offset` is one of the fanned-out members of an
;; expanded cluster.
expanded? (some? offset)
;; Click-through while transforming a shape, so it doesn't capture the drag ;; Click-through while transforming a shape, so it doesn't capture the drag
dragging? (some? (mf/deref refs/current-transform)) dragging? (some? (mf/deref refs/current-transform))
@ -1212,8 +1251,31 @@
:new-position-y nil :new-position-y nil
:new-frame-id frame-id})) :new-frame-id frame-id}))
pos-x (floor (* (or (:new-position-x @state) (:x position)) zoom)) new-x (:new-position-x @state)
pos-y (floor (* (or (:new-position-y @state) (:y position)) zoom)) new-y (:new-position-y @state)
;; While dragging, the new position already accounts for the ring offset;
;; otherwise an expanded bubble sits at its stored position plus the
;; screen-space ring offset.
pos-x (if (some? new-x)
(floor (* new-x zoom))
(+ (floor (* (:x position) zoom))
(if expanded? (:x offset) 0)))
pos-y (if (some? new-y)
(floor (* new-y zoom))
(+ (floor (* (:y position) zoom))
(if expanded? (:y offset) 0)))
;; World-space anchor a drag starts from: an expanded bubble is shown at
;; its ring position, so dragging must begin there, not at its stored
;; position.
drag-base-x (if expanded? (+ (:x position) (/ (:x offset) zoom)) (:x position))
drag-base-y (if expanded? (+ (:y position) (/ (:y offset) zoom)) (:y position))
;; CSS custom properties fed to the fan-out animation; nil for regular
;; (non-expanded) bubbles.
ring-x (when (and expanded? (some? ring)) (dm/str (- (:x ring)) "px"))
ring-y (when (and expanded? (some? ring)) (dm/str (- (:y ring)) "px"))
drag? (mf/use-ref nil) drag? (mf/use-ref nil)
was-open? (mf/use-ref nil) was-open? (mf/use-ref nil)
@ -1237,7 +1299,7 @@
on-pointer-up on-pointer-up
(mf/use-fn (mf/use-fn
(mf/deps origin thread (select-keys @state [:new-position-x :new-position-y :new-frame-id])) (mf/deps origin thread expanded? (select-keys @state [:new-position-x :new-position-y :new-frame-id]))
(fn [event] (fn [event]
(when (not= origin :viewer) (when (not= origin :viewer)
(swap! state assoc :is-grabbing false) (swap! state assoc :is-grabbing false)
@ -1250,13 +1312,16 @@
(some? (:new-position-y @state))) (some? (:new-position-y @state)))
(st/emit! (dwcm/update-comment-thread-position thread [(:new-position-x @state) (st/emit! (dwcm/update-comment-thread-position thread [(:new-position-x @state)
(:new-position-y @state)])) (:new-position-y @state)]))
;; Dropping a fanned-out bubble commits its new spot and closes
;; the temporary cluster expansion.
(when expanded? (st/emit! (dcm/collapse-comment-group)))
(swap! state assoc (swap! state assoc
:new-position-x nil :new-position-x nil
:new-position-y nil))))) :new-position-y nil)))))
on-pointer-move on-pointer-move
(mf/use-fn (mf/use-fn
(mf/deps origin drag? position zoom) (mf/deps origin drag? drag-base-x drag-base-y zoom)
(fn [event] (fn [event]
(when (not= origin :viewer) (when (not= origin :viewer)
(mf/set-ref-val! drag? true) (mf/set-ref-val! drag? true)
@ -1267,8 +1332,8 @@
delta-x (/ (- (:x current-pt) (:x start-pt)) zoom) delta-x (/ (- (:x current-pt) (:x start-pt)) zoom)
delta-y (/ (- (:y current-pt) (:y start-pt)) zoom)] delta-y (/ (- (:y current-pt) (:y start-pt)) zoom)]
(swap! state assoc (swap! state assoc
:new-position-x (+ (:x position) delta-x) :new-position-x (+ drag-base-x delta-x)
:new-position-y (+ (:y position) delta-y))))))) :new-position-y (+ drag-base-y delta-y)))))))
on-pointer-enter on-pointer-enter
(mf/use-fn (mf/use-fn
@ -1286,7 +1351,7 @@
on-click* on-click*
(mf/use-fn (mf/use-fn
(mf/deps origin thread on-click was-open? drag? (select-keys @state [:is-hover])) (mf/deps origin thread on-click was-open? drag?)
(fn [event] (fn [event]
(dom/stop-propagation event) (dom/stop-propagation event)
(when (or (and (mf/ref-val was-open?) (mf/ref-val drag?)) (when (or (and (mf/ref-val was-open?) (mf/ref-val drag?))
@ -1298,34 +1363,42 @@
[:div {:style {:top (dm/str pos-y "px") [:div {:style {:top (dm/str pos-y "px")
:left (dm/str pos-x "px") :left (dm/str pos-x "px")
:pointer-events (when dragging? "none")} :pointer-events (when dragging? "none")
:on-pointer-down on-pointer-down "--comment-ring-x" ring-x
:on-pointer-up on-pointer-up "--comment-ring-y" ring-y}
:on-pointer-move on-pointer-move
:on-pointer-enter on-pointer-enter
:on-pointer-leave on-pointer-leave
:on-click on-click*
:class (stl/css-case :floating-preview-wrapper true :class (stl/css-case :floating-preview-wrapper true
:floating-preview-bubble (false? (:is-hover @state)))} :floating-preview-bubble (false? (:is-hover @state))
:floating-preview-expanded expanded?
:floating-preview-hovered (:is-hover @state))}
(if (:is-hover @state) ;; The avatar circle is the only pointer target: it drives hover, drag and
[:div {:class (stl/css-case :floating-thread-wrapper true ;; click, so hovering the preview card below never keeps the tooltip open.
:floating-preview-displacement true [:div {:on-pointer-down on-pointer-down
:cursor-pointer (false? (:is-grabbing @state)) :on-pointer-up on-pointer-up
:cursor-grabbing (true? (:is-grabbing @state)))} :on-pointer-move on-pointer-move
:on-pointer-enter on-pointer-enter
:on-pointer-leave on-pointer-leave
:on-click on-click*
:class (stl/css-case :floating-preview-avatar true
:cursor-pointer (false? (:is-grabbing @state))
:cursor-grabbing (true? (:is-grabbing @state)))}
[:> comment-avatar*
{:image (cfg/resolve-profile-photo-url owner)
:class (stl/css :avatar-lg)
:data-testid (dm/str "floating-thread-bubble-" (:seqn thread))
:variant (cond
(:is-resolved thread) "solved"
(pos? (:count-unread-comments thread)) "unread"
:else "read")}]]
(when (:is-hover @state)
[:div {:class (stl/css :floating-thread-wrapper
:floating-preview-displacement
:floating-preview-hover-card)}
[:div {:class (stl/css :floating-thread-item-wrapper)} [:div {:class (stl/css :floating-thread-item-wrapper)}
[:div {:class (stl/css :floating-thread-item)} [:div {:class (stl/css :floating-thread-item)}
[:> comment-info* {:item thread [:> comment-info* {:item thread
:profile owner}]]]] :profile owner}]]]])]))
[:> comment-avatar*
{:image (cfg/resolve-profile-photo-url owner)
:class (stl/css :avatar-lg)
:data-testid (dm/str "floating-thread-bubble-" (:seqn thread))
:variant (cond
(:is-resolved thread) "solved"
(pos? (:count-unread-comments thread)) "unread"
:else "read")}])]))
(mf/defc comment-sidebar-thread-item* (mf/defc comment-sidebar-thread-item*
{::mf/private true} {::mf/private true}

View File

@ -5,6 +5,7 @@
// Copyright (c) KALEIDOS INC Sucursal en España SL // Copyright (c) KALEIDOS INC Sucursal en España SL
@use "refactor/common-refactor.scss" as deprecated; @use "refactor/common-refactor.scss" as deprecated;
@use "ds/_sizes.scss" as *;
.cursor-grabbing { .cursor-grabbing {
cursor: grabbing; cursor: grabbing;
@ -104,6 +105,7 @@
justify-content: center; justify-content: center;
font-size: deprecated.$fs-12; font-size: deprecated.$fs-12;
background-color: var(--color-background-quaternary); background-color: var(--color-background-quaternary);
color: var(--color-foreground-quaternary);
} }
.avatar-mask { .avatar-mask {
@ -167,31 +169,68 @@
z-index: initial; z-index: initial;
} }
.floating-thread-draft-wrapper { // Sole pointer target of a floating bubble.
position: absolute; .floating-preview-avatar {
display: flex; display: flex;
flex-direction: row; height: $sz-32;
column-gap: deprecated.$s-12; width: $sz-32;
}
--translate-x: 0%; // Expanded-cluster bubble: sits above regular bubbles and fans out on appearance.
--translate-y: 0%; .floating-preview-expanded {
z-index: 2;
animation: comment-bubble-fan-out 0.18s ease-out;
}
transform: translate(var(--translate-x), var(--translate-y)); @keyframes comment-bubble-fan-out {
from {
&.left { translate: var(--comment-ring-x, 0) var(--comment-ring-y, 0);
--translate-x: -100%; opacity: 0;
flex-direction: row-reverse;
} }
&.top { to {
--translate-y: -100%; translate: 0 0;
opacity: 1;
align-items: flex-end;
} }
} }
// Hovered bubble's preview card floats above every other bubble.
.floating-preview-hovered {
z-index: 10;
}
// Placeholder shown at the cluster center while its bubbles are fanned out.
.floating-preview-ghost {
z-index: 1;
opacity: 0.6;
pointer-events: none;
animation: comment-ghost-appear 0.18s ease-out;
.avatar {
border-style: dashed;
background-color: var(--comment-modal-background-color);
}
}
@keyframes comment-ghost-appear {
from {
opacity: 0;
}
to {
opacity: 0.6;
}
}
// Anchored to the wrapper origin and click-through, so hover stays bound to the avatar.
.floating-thread-wrapper.floating-preview-hover-card {
top: 0;
left: 0;
pointer-events: none;
}
.floating-thread-draft-inner-wrapper { .floating-thread-draft-inner-wrapper {
position: absolute;
display: flex; display: flex;
flex-direction: column; flex-direction: column;
gap: deprecated.$s-12; gap: deprecated.$s-12;
@ -202,6 +241,19 @@
border: deprecated.$s-2 solid var(--modal-border-color); border: deprecated.$s-2 solid var(--modal-border-color);
background-color: var(--comment-modal-background-color); background-color: var(--comment-modal-background-color);
max-height: var(--comment-height); max-height: var(--comment-height);
--translate-x: 0%;
--translate-y: 0%;
transform: translate(var(--translate-x), var(--translate-y));
&.left {
--translate-x: -100%;
}
&.top {
--translate-y: -100%;
}
} }
.floating-preview-displacement { .floating-preview-displacement {

View File

@ -15,6 +15,7 @@
[app.common.geom.shapes :as gsh] [app.common.geom.shapes :as gsh]
[app.main.data.comments :as dcm] [app.main.data.comments :as dcm]
[app.main.data.event :as ev] [app.main.data.event :as ev]
[app.main.data.workspace.comments :as dwcm]
[app.main.refs :as refs] [app.main.refs :as refs]
[app.main.store :as st] [app.main.store :as st]
[app.main.ui.comments :as cmt] [app.main.ui.comments :as cmt]
@ -176,13 +177,22 @@
(-> (dcm/open-thread thread) (-> (dcm/open-thread thread)
(with-meta {::ev/origin "viewer"})))))) (with-meta {::ev/origin "viewer"}))))))
expanded (:expanded local)
on-click on-click
(mf/use-fn (mf/use-fn
(mf/deps open-thread-id zoom page-id file-id modifier2) (mf/deps open-thread-id expanded zoom page-id file-id modifier2)
(fn [event] (fn [event]
(dom/stop-propagation event) (dom/stop-propagation event)
(if (some? open-thread-id) (cond
;; A click anywhere first collapses a temporarily expanded cluster.
(some? expanded)
(st/emit! (dcm/collapse-comment-group))
(some? open-thread-id)
(st/emit! (dcm/close-thread)) (st/emit! (dcm/close-thread))
:else
(let [event (dom/event->native-event event) (let [event (dom/event->native-event event)
position (-> (dom/get-offset-position event) position (-> (dom/get-offset-position event)
(update :x #(/ % zoom)) (update :x #(/ % zoom))
@ -204,24 +214,60 @@
(st/emit! (dcm/create-thread-on-viewer params) (st/emit! (dcm/create-thread-on-viewer params)
(dcm/close-thread)))))] (dcm/close-thread)))))]
;; Any zoom change collapses an expanded cluster back, matching the workspace.
(mf/with-effect [zoom]
(st/emit! (dcm/collapse-comment-group)))
[:div {:class (stl/css :comments-section) [:div {:class (stl/css :comments-section)
:on-click on-click} :on-click on-click}
[:div {:class (dm/str cursor " " (stl/css :viewer-comments-container))} [:div {:class (dm/str cursor " " (stl/css :viewer-comments-container))}
[:div {:class (stl/css :threads)} [:div {:class (stl/css :threads)}
(for [item threads] (for [thread-group (dwcm/group-bubbles zoom threads)]
[:> cmt/comment-floating-bubble* (let [group? (> (count thread-group) 1)
{:thread item thread (first thread-group)
:position-modifier modifier1 expanded? (and group?
:zoom zoom (= expanded (into #{} (map :id) thread-group)))]
:on-click on-bubble-click (cond
:is-open (= (:id item) (:open local)) expanded?
:key (:seqn item) [:* {:key (:seqn thread)}
:origin :viewer}]) [:> cmt/comment-floating-ghost*
{:thread-group thread-group
:zoom zoom
:position-modifier modifier1}]
(for [[thread offset ring] (cmt/expanded-group-offsets zoom thread-group)]
[:> cmt/comment-floating-bubble*
{:thread thread
:position-modifier modifier1
:zoom zoom
:offset offset
:ring ring
:on-click on-bubble-click
:is-open false
:origin :viewer
:key (:seqn thread)}])]
group?
[:> cmt/comment-floating-group*
{:thread-group thread-group
:zoom zoom
:position-modifier modifier1
:key (:seqn thread)}]
:else
[:> cmt/comment-floating-bubble*
{:thread thread
:position-modifier modifier1
:zoom zoom
:on-click on-bubble-click
:is-open (= (:id thread) (:open local))
:origin :viewer
:key (:seqn thread)}])))
(when-let [thread (get threads-map open-thread-id)] (when-let [thread (get threads-map open-thread-id)]
[:> cmt/comment-floating-thread* [:> cmt/comment-floating-thread*
{:thread thread {:thread thread
:position-modifier modifier1 :position-modifier modifier1
:origin :viewer
:viewport {:offset-x 0 :offset-y 0 :width (:width vsize) :height (:height vsize)} :viewport {:offset-x 0 :offset-y 0 :width (:width vsize) :height (:height vsize)}
:zoom zoom}]) :zoom zoom}])

View File

@ -28,12 +28,14 @@
(mf/defc comment-floating-bubble-wrapper* (mf/defc comment-floating-bubble-wrapper*
{::mf/private true} {::mf/private true}
[{:keys [thread zoom is-open]}] [{:keys [thread zoom is-open offset ring]}]
(let [position-modifier (use-frame-position-modifier (:frame-id thread))] (let [position-modifier (use-frame-position-modifier (:frame-id thread))]
[:> cmt/comment-floating-bubble* [:> cmt/comment-floating-bubble*
{:thread thread {:thread thread
:zoom zoom :zoom zoom
:position-modifier position-modifier :position-modifier position-modifier
:offset offset
:ring ring
:is-open is-open}])) :is-open is-open}]))
(mf/defc comment-floating-group-wrapper* (mf/defc comment-floating-group-wrapper*
@ -46,6 +48,16 @@
:zoom zoom :zoom zoom
:position-modifier position-modifier}])) :position-modifier position-modifier}]))
(mf/defc comment-floating-ghost-wrapper*
{::mf/private true}
[{:keys [thread-group zoom]}]
(let [thread (first thread-group)
position-modifier (use-frame-position-modifier (:frame-id thread))]
[:> cmt/comment-floating-ghost*
{:thread-group thread-group
:zoom zoom
:position-modifier position-modifier}]))
(mf/defc comment-floating-thread-wrapper* (mf/defc comment-floating-thread-wrapper*
{::mf/private true} {::mf/private true}
[{:keys [thread viewport zoom]}] [{:keys [thread viewport zoom]}]
@ -93,6 +105,10 @@
(st/emit! (dwcm/initialize-comments file-id)) (st/emit! (dwcm/initialize-comments file-id))
(fn [] (st/emit! ::dwcm/finalize))) (fn [] (st/emit! ::dwcm/finalize)))
;; Any viewport change (pan/zoom) collapses an expanded cluster back.
(mf/with-effect [vbox zoom]
(st/emit! (dcm/collapse-comment-group)))
[:div {:class (stl/css :comments-section)} [:div {:class (stl/css :comments-section)}
[:div [:div
{:id "comments" {:id "comments"
@ -102,13 +118,30 @@
[:div {:class (stl/css :threads) [:div {:class (stl/css :threads)
:style {:transform (dm/fmt "translate(%px, %px)" pos-x pos-y)}} :style {:transform (dm/fmt "translate(%px, %px)" pos-x pos-y)}}
(for [thread-group (cmt/group-bubbles zoom threads)] (for [thread-group (dwcm/group-bubbles zoom threads)]
(let [group? (> (count thread-group) 1) (let [group? (> (count thread-group) 1)
thread (first thread-group)] thread (first thread-group)
(if group? expanded? (and group?
(= (:expanded local) (into #{} (map :id) thread-group)))]
(cond
expanded?
[:* {:key (:seqn thread)}
[:> comment-floating-ghost-wrapper* {:thread-group thread-group
:zoom zoom}]
(for [[thread offset ring] (cmt/expanded-group-offsets zoom thread-group)]
[:> comment-floating-bubble-wrapper* {:thread thread
:zoom zoom
:offset offset
:ring ring
:is-open false
:key (:seqn thread)}])]
group?
[:> comment-floating-group-wrapper* {:thread-group thread-group [:> comment-floating-group-wrapper* {:thread-group thread-group
:zoom zoom :zoom zoom
:key (:seqn thread)}] :key (:seqn thread)}]
:else
[:> comment-floating-bubble-wrapper* {:thread thread [:> comment-floating-bubble-wrapper* {:thread thread
:zoom zoom :zoom zoom
:is-open (= (:id thread) (:open local)) :is-open (= (:id thread) (:open local))

View File

@ -52,6 +52,7 @@
[frontend-tests.tokens.style-dictionary-test] [frontend-tests.tokens.style-dictionary-test]
[frontend-tests.tokens.token-errors-test] [frontend-tests.tokens.token-errors-test]
[frontend-tests.tokens.workspace-tokens-remap-test] [frontend-tests.tokens.workspace-tokens-remap-test]
[frontend-tests.ui.comments-clustering-test]
[frontend-tests.ui.comments-position-modifier-test] [frontend-tests.ui.comments-position-modifier-test]
[frontend-tests.ui.ds-controls-numeric-input-test] [frontend-tests.ui.ds-controls-numeric-input-test]
[frontend-tests.ui.measures-menu-props-test] [frontend-tests.ui.measures-menu-props-test]
@ -122,6 +123,7 @@
'frontend-tests.tokens.style-dictionary-test 'frontend-tests.tokens.style-dictionary-test
'frontend-tests.tokens.token-errors-test 'frontend-tests.tokens.token-errors-test
'frontend-tests.tokens.workspace-tokens-remap-test 'frontend-tests.tokens.workspace-tokens-remap-test
'frontend-tests.ui.comments-clustering-test
'frontend-tests.ui.comments-position-modifier-test 'frontend-tests.ui.comments-position-modifier-test
'frontend-tests.ui.ds-controls-numeric-input-test 'frontend-tests.ui.ds-controls-numeric-input-test
'frontend-tests.ui.measures-menu-props-test 'frontend-tests.ui.measures-menu-props-test

View File

@ -0,0 +1,144 @@
;; 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.ui.comments-clustering-test
(:require
[app.common.geom.point :as gpt]
[app.common.math :as mth]
[app.main.data.comments :as dcm]
[app.main.data.workspace.comments :as dwcm]
[app.main.ui.comments :as cmt]
[cljs.test :as t :include-macros true]
[potok.v2.core :as ptk]))
(defn- thread
[seqn x y]
{:id seqn :seqn seqn :position (gpt/point x y)})
(defn- magnitude
[p]
(gpt/distance p (gpt/point 0 0)))
(defn- find-group
"Return the group (from a group-bubbles result) that contains the thread with
the given seqn."
[groups seqn]
(some (fn [group]
(when (some #(= (:seqn %) seqn) group) group))
groups))
;; --- overlap-bubbles? ------------------------------------------------------
(t/deftest overlap-bubbles-close
(t/testing "bubbles within the overlap distance overlap"
(t/is (true? (dwcm/overlap-bubbles? 1 (thread 1 100 100) (thread 2 110 100))))))
(t/deftest overlap-bubbles-far
(t/testing "distant bubbles do not overlap"
(t/is (false? (dwcm/overlap-bubbles? 1 (thread 1 100 100) (thread 2 200 100))))))
(t/deftest overlap-bubbles-zoom
(t/testing "zoom scales the screen-space distance, separating close bubbles"
;; 10px apart in canvas space is 40px on screen at zoom 4, above the 32px
;; overlap threshold.
(t/is (false? (dwcm/overlap-bubbles? 4 (thread 1 100 100) (thread 2 110 100))))))
;; --- group-bubbles ---------------------------------------------------------
(t/deftest group-bubbles-separate-clusters
(let [threads [(thread 1 100 100)
(thread 2 105 100)
(thread 3 500 500)]
groups (dwcm/group-bubbles 1 threads)]
(t/testing "produces one group per proximity cluster"
(t/is (= 2 (count groups))))
(t/testing "overlapping bubbles share a group"
(t/is (= 2 (count (find-group groups 1))))
(t/is (some #(= (:seqn %) 2) (find-group groups 1))))
(t/testing "a distant bubble stays alone"
(t/is (= 1 (count (find-group groups 3)))))))
(t/deftest group-bubbles-transitive-chain
;; A-B and B-C overlap but A-C do not; the shared neighbour B keeps them in a
;; single group.
(let [threads [(thread 1 100 100)
(thread 2 120 100)
(thread 3 140 100)]
groups (dwcm/group-bubbles 1 threads)]
(t/testing "a chain of overlaps forms a single group"
(t/is (= 1 (count groups)))
(t/is (= 3 (count (first groups)))))))
;; --- expanded-group-center -------------------------------------------------
(t/deftest expanded-group-center-centroid
(t/testing "the cluster center is the centroid of the bubble positions"
(t/is (= (gpt/point 150 150)
(cmt/expanded-group-center [(thread 1 100 100)
(thread 2 200 200)])))))
;; --- expanded-group-offsets ------------------------------------------------
(t/deftest expanded-group-offsets-single-ring
(let [threads (mapv #(thread % 100 100) (range 6))
offsets (cmt/expanded-group-offsets 1 threads)]
(t/testing "lays out every bubble of the cluster"
(t/is (= 6 (count offsets))))
(t/testing "the innermost ring keeps a constant radius"
(t/is (every? #(mth/close? 44 (magnitude (nth % 2))) offsets)))
(t/testing "the first bubble is placed at the top of the ring"
(let [ring (nth (first offsets) 2)]
(t/is (mth/close? 0 (:x ring)))
(t/is (mth/close? -44 (:y ring)))))))
(t/deftest expanded-group-offsets-concentric-rings
(let [threads (mapv #(thread % 100 100) (range 8))
rings (mapv #(magnitude (nth % 2))
(cmt/expanded-group-offsets 1 threads))]
(t/testing "the first six bubbles fill the inner ring"
(t/is (every? #(mth/close? 44 %) (take 6 rings))))
(t/testing "the overflow spills onto a second, wider ring"
(t/is (every? #(mth/close? 88 %) (drop 6 rings))))))
(t/deftest expanded-group-offsets-total-offset
(t/testing "the total offset moves each bubble from its position to the ring"
;; With all bubbles sharing a position, the base displacement is zero, so the
;; total offset equals the pure ring vector.
(let [threads (mapv #(thread % 100 100) (range 3))]
(doseq [[_ offset ring] (cmt/expanded-group-offsets 1 threads)]
(t/is (mth/close? (:x offset) (:x ring)))
(t/is (mth/close? (:y offset) (:y ring)))))))
;; --- cluster state transitions ---------------------------------------------
(t/deftest expand-comment-group-replaces-open
(let [state {:comments-local {:open 5 :draft {} :options 9}}
result (ptk/update (dcm/expand-comment-group #{1 2 3}) state)
local (:comments-local result)]
(t/testing "expanding a cluster records its member ids"
(t/is (= #{1 2 3} (:expanded local))))
(t/testing "expanding a cluster clears any open thread or draft"
(t/is (nil? (:open local)))
(t/is (nil? (:draft local))))))
(t/deftest collapse-comment-group-preserves-open
(let [state {:comments-local {:expanded #{1 2} :open 5}}
result (ptk/update (dcm/collapse-comment-group) state)
local (:comments-local result)]
(t/testing "collapsing clears the expansion"
(t/is (nil? (:expanded local))))
(t/testing "collapsing leaves an open thread untouched"
(t/is (= 5 (:open local))))))
(t/deftest close-thread-clears-everything
(let [state {:comments-local {:open 5 :draft {} :expanded #{1} :options 9}}
result (ptk/update (dcm/close-thread) state)
local (:comments-local result)]
(t/testing "closing a thread clears open, draft, expanded and options"
(t/is (nil? (:open local)))
(t/is (nil? (:draft local)))
(t/is (nil? (:expanded local)))
(t/is (nil? (:options local))))))