Penpot/penpot
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-04-04 19:11:20 -05:00
Code Issues Activity

✨ Replace overlapping bubbles with a bubble group (#6059)

Browse source
This commit is contained in:
luisδμ 2025-03-12 14:37:39 +01:00 • committed by GitHub
parent 0efbebd94f
commit 86022a967c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 299 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGES.md
View file

@ -10,7 +10,8 @@
### :sparkles: New features
- "Mark All as Read" Functionality in Dashboard [Taiga #9235](https://tree.taiga.io/project/penpot/us/9235)
- [COMMENTS] "Mark All as Read" Functionality in Dashboard [Taiga #9235](https://tree.taiga.io/project/penpot/us/9235)
- [COMMENTS] Bubble Groups [Taiga #9236](https://tree.taiga.io/project/penpot/us/9236)
- Change templates carrousel [Taiga #9803](https://tree.taiga.io/project/penpot/us/9803)
### :bug: Bugs fixed

58
frontend/playwright/data/workspace/get-comment-threads-unread.json Normal file
View file

@ -0,0 +1,58 @@
[
{
"~:page-name":"Page 1",
"~:file-id":"~ud192fd06-a3e6-80d5-8004-7b7aaaea2a23",
"~:participants":{
"~#set":[
"~u0515a066-e303-8169-8004-73eb4018f4e0"
]
},
"~:content":"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Pellentesque lacus tellus, pretium id dapibus in, suscipit eu magna. Duis rhoncus, nisl quis accumsan euismod, dolor ipsum bibendum enim, et varius turpis erat ut purus.",
"~:count-unread-comments":1,
"~:count-comments":1,
"~:modified-at":"~m1718001240857",
"~:page-id":"~udd5cc0bb-91ff-81b9-8004-77df9cd3edb2",
"~:id": "~udd5cc0bb-91ff-81b9-8004-77df9cd3edb1",
"~:file-name":"New File 3",
"~:seqn":1,
"~:is-resolved":false,
"~:owner-id":"~u2e2da0fa-2d3e-81ec-8003-cb4453324510",
"~:position":{
"~#point":{
"~:x":120.0,
"~:y":120.0
}
},
"~:frame-id": "~uec508673-9e3b-80bf-8004-77dfa30a2b13",
"~:project-id": "~u0515a066-e303-8169-8004-73eb401b5d55",
"~:created-at":"~m1718001240857"
},
{
"~:page-name":"Page 1",
"~:file-id":"~ud192fd06-a3e6-80d5-8004-7b7aaaea2a23",
"~:participants":{
"~#set":[
"~u0515a066-e303-8169-8004-73eb4018f4e0"
]
},
"~:content":"Duis lobortis ultricies lectus, in tristique tortor. Praesent mauris mi, finibus vel imperdiet quis, congue vel erat. Sed pharetra et ipsum at vestibulum.",
"~:count-unread-comments":0,
"~:count-comments":1,
"~:modified-at":"~m1718001247587",
"~:page-id":"~udd5cc0bb-91ff-81b9-8004-77df9cd3edb2",
"~:id":"~ud192fd06-a3e6-80d5-8004-7b7ac25ac93a",
"~:file-name":"New File 44",
"~:seqn":2,
"~:is-resolved":false,
"~:owner-id":"~u2e2da0fa-2d3e-81ec-8003-cb4453324510",
"~:position":{
"~#point":{
"~:x":160.0,
"~:y":160.0
}
},
"~:frame-id": "~uec508673-9e3b-80bf-8004-77dfa30a2b13",
"~:project-id":"~u343837a3-0d75-808a-8004-659df7b7873e",
"~:created-at":"~m1718001247587"
}
]

25
frontend/playwright/ui/pages/WorkspacePage.js
View file

@ -171,6 +171,25 @@ export class WorkspacePage extends BaseWebSocketPage {
);
}
async setupFileWithComments() {
await this.mockRPC(
"get-comment-threads?file-id=*",
"workspace/get-comment-threads-unread.json",
);
await this.mockRPC(
"get-file-fragment?file-id=*&fragment-id=*",
"viewer/get-file-fragment-single-board.json",
);
await this.mockRPC(
"get-comments?thread-id=*",
"workspace/get-thread-comments.json",
);
await this.mockRPC(
"update-comment-thread-status",
"workspace/update-comment-thread-status.json",
);
}
async clickWithDragViewportAt(x, y, width, height) {
await this.page.waitForTimeout(100);
await this.viewport.hover({ position: { x, y } });
@ -266,4 +285,10 @@ export class WorkspacePage extends BaseWebSocketPage {
await this.tokenThemesSetsSidebar.getByText("Edit").click(clickOptions);
await expect(this.tokenThemeUpdateCreateModal).toBeVisible();
}
async showComments(clickOptions = {}) {
await this.page
.getByRole("button", { name: "Comments (C)" })
.click(clickOptions);
}
}

37
frontend/playwright/ui/specs/workspace-comments.spec.js Normal file
View file

@ -0,0 +1,37 @@
import { test, expect } from "@playwright/test";
import { WorkspacePage } from "../pages/WorkspacePage";
test.beforeEach(async ({ page }) => {
await WorkspacePage.init(page);
});
test("Group bubbles when zooming out if they overlap", async ({ page }) => {
const workspacePage = new WorkspacePage(page);
await workspacePage.setupEmptyFile();
await workspacePage.setupFileWithComments();
await workspacePage.goToWorkspace();
await workspacePage.showComments();
await expect(page.getByTestId("floating-thread-bubble-1")).toBeVisible();
await expect(page.getByTestId("floating-thread-bubble-2")).toBeVisible();
await expect(page.getByTestId("floating-thread-bubble-1-2")).toBeHidden();
const zoom = page.getByTitle("Zoom");
await zoom.click();
const zoomOut = page.getByTitle("Zoom out");
await zoomOut.click();
await zoomOut.click();
await zoomOut.click();
await zoomOut.click();
await expect(page.getByTestId("floating-thread-bubble-1")).toBeHidden();
await expect(page.getByTestId("floating-thread-bubble-2")).toBeHidden();
await expect(page.getByTestId("floating-thread-bubble-1-2")).toBeVisible();
await expect(page.getByTestId("floating-thread-bubble-1-2")).toHaveClass(
/unread/,
);
});

82
frontend/src/app/main/data/workspace/comments.cljs
View file

@ -20,6 +20,7 @@
[app.main.data.workspace.drawing :as dwd]
[app.main.data.workspace.edition :as dwe]
[app.main.data.workspace.selection :as dws]
[app.main.data.workspace.zoom :as dwz]
[app.main.repo :as rp]
[app.main.router :as rt]
[app.main.streams :as ms]
@ -102,26 +103,6 @@
ny (- (:y position) nh)]
(update local :vbox assoc :x nx :y ny)))))))
(defn navigate
[thread]
(dm/assert!
"expected valid comment thread"
(dcmt/check-comment-thread! thread))
(ptk/reify ::open-comment-thread
ptk/WatchEvent
(watch [_ _ stream]
(rx/merge
(rx/of (dcm/go-to-workspace :file-id (:file-id thread)
:page-id (:page-id thread)))
(->> stream
(rx/filter (ptk/type? ::dcmt/comment-threads-fetched))
(rx/take 1)
(rx/mapcat #(rx/of (center-to-comment-thread thread)
(dwd/select-for-drawing :comments)
(with-meta (dcmt/open-thread thread)
{::ev/origin "workspace"}))))))))
(defn update-comment-thread-position
([thread [new-x new-y]]
(update-comment-thread-position thread [new-x new-y] nil))
@ -192,6 +173,66 @@
(map build-move-event)
(rx/from))))))
(defn overlap-bubbles?
"Detect if two bubbles overlap"
[zoom thread-1 thread-2]
(let [distance (gpt/distance (:position thread-1) (:position thread-2))
distance-zoom (* distance zoom)
distance-overlap 32]
(< distance-zoom distance-overlap)))
(defn- calculate-zoom-scale-to-ungroup-current-bubble
"Calculate the minimum zoom scale needed to keep the current bubble ungrouped from the rest"
[zoom thread threads]
(let [threads-rest (filterv #(not= (:id %) (:id thread)) threads)
zoom-scale-step 1.75]
(if (some #(overlap-bubbles? zoom thread %) threads-rest)
(calculate-zoom-scale-to-ungroup-current-bubble (* zoom zoom-scale-step) thread threads)
zoom)))
(defn set-zoom-to-separate-grouped-bubbles
[thread]
(dm/assert!
"zoom-to-separate-bubbles"
(dcmt/check-comment-thread! thread))
(ptk/reify ::set-zoom-to-separate-grouped-bubbles
ptk/WatchEvent
(watch [_ state _]
(let [local (:workspace-local state)
zoom (:zoom local)
page-id (:page-id thread)
threads-map (:comment-threads state)
threads-all (vals threads-map)
threads (filterv #(= (:page-id %) page-id) threads-all)
updated-zoom (calculate-zoom-scale-to-ungroup-current-bubble zoom thread threads)
scale-zoom (/ updated-zoom zoom)]
(rx/of (dwz/set-zoom scale-zoom))))))
(defn navigate-to-comment-from-dashboard
[thread]
(dm/assert!
"expected valid comment thread"
(dcmt/check-comment-thread! thread))
(ptk/reify ::navigate-to-comment-from-dashboard
ptk/WatchEvent
(watch [_ _ stream]
(rx/merge
(rx/of (dcm/go-to-workspace :file-id (:file-id thread)
:page-id (:page-id thread)))
(->> stream
(rx/filter (ptk/type? :app.main.data.workspace/workspace-initialized))
(rx/observe-on :async)
(rx/take 1)
(rx/mapcat #(rx/of (dwd/select-for-drawing :comments)
(set-zoom-to-separate-grouped-bubbles thread)
(center-to-comment-thread thread)
(with-meta (dcmt/open-thread thread)
{::ev/origin "workspace"}))))))))
(defn navigate-to-comment
[thread]
(ptk/reify ::navigate-to-comment
@ -208,6 +249,7 @@
(rx/empty))
(->> (rx/of
(dwd/select-for-drawing :comments)
(set-zoom-to-separate-grouped-bubbles thread)
(center-to-comment-thread thread)
(with-meta (dcmt/open-thread thread) {::ev/origin "workspace"}))
(rx/observe-on :async))))))

109
frontend/src/app/main/ui/comments.cljs
View file

@ -17,6 +17,7 @@
[app.main.data.comments :as dcm]
[app.main.data.modal :as modal]
[app.main.data.workspace.comments :as dwcm]
[app.main.data.workspace.zoom :as dwz]
[app.main.refs :as refs]
[app.main.store :as st]
[app.main.ui.components.dropdown :refer [dropdown]]
@ -589,22 +590,25 @@
(def ^:private schema:comment-avatar
[:map
[:class {:optional true} :string]
[:image :string]
[:image {:optional true} :string]
[:variant {:optional true}
[:maybe [:enum "read" "unread" "solved"]]]])
(mf/defc comment-avatar*
{::mf/schema schema:comment-avatar}
[{:keys [image variant class] :rest props}]
[{:keys [image variant class children] :rest props}]
(let [variant (or variant "read")
class (dm/str class " " (stl/css-case :avatar true
:avatar-read (= variant "read")
:avatar-unread (= variant "unread")
:avatar-solved (= variant "solved")))
props (mf/spread-props props {:class class})]
class (dm/str class " " (stl/css-case :avatar true
:avatar-read (= variant "read")
:avatar-unread (= variant "unread")
:avatar-solved (= variant "solved")))
props (mf/spread-props props {:class class})]
[:> :div props
[:img {:src image
:class (stl/css :avatar-image)}]
(if image
[:img {:src image
:class (stl/css :avatar-image)}]
[:div {:class (stl/css :avatar-text)} children])
[:div {:class (stl/css-case :avatar-mask true
:avatar-darken (= variant "solved"))}]]))
@ -1072,22 +1076,83 @@
[:> mentions-panel*]])]))
(defn group-bubbles
"Group bubbles in different vectors by proximity"
([zoom circles]
(group-bubbles zoom circles [] []))
([zoom circles visited groups]
(if (empty? circles)
groups
(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- calculate-zoom-scale-to-ungroup-bubbles
"Calculate the minimum zoom scale needed for a group of bubbles to avoid overlap among them"
[zoom threads]
(let [num-threads (count threads)
num-grouped-threads (count (group-bubbles zoom threads))
zoom-scale-step 1.75]
(if (= num-threads num-grouped-threads)
zoom
(calculate-zoom-scale-to-ungroup-bubbles (* zoom zoom-scale-step) threads))))
(mf/defc comment-floating-group*
{::mf/wrap [mf/memo]}
[{:keys [thread-group zoom position-modifier]}]
(let [positions (mapv :position thread-group)
position (gpt/center-points positions)
position (cond-> position
(some? position-modifier)
(gpt/transform position-modifier))
pos-x (* (:x position) zoom)
pos-y (* (:y position) zoom)
unread? (some #(pos? (:count-unread-comments %)) thread-group)
num-threads (str (count thread-group))
test-id (str/join "-" (map :seqn (sort-by :seqn thread-group)))
on-click
(mf/use-fn
(mf/deps thread-group position)
(fn []
(let [updated-zoom (calculate-zoom-scale-to-ungroup-bubbles zoom thread-group)
scale-zoom (/ updated-zoom zoom)]
(st/emit! (dwz/set-zoom position scale-zoom)))))]
[:div {:style {:top (dm/str pos-y "px")
:left (dm/str pos-x "px")}
:on-click on-click
:class (stl/css :floating-preview-wrapper :floating-preview-bubble)}
[:> comment-avatar*
{:class (stl/css :avatar-lg)
:variant (if unread? "unread" "read")
:data-testid (dm/str "floating-thread-bubble-" test-id)}
num-threads]]))
(mf/defc comment-floating-bubble*
{::mf/wrap [mf/memo]}
[{:keys [thread zoom is-open on-click origin position-modifier]}]
(let [owner (mf/with-memo [thread]
(dcm/get-owner thread))
base-pos (cond-> (:position thread)
position (:position thread)
position (cond-> position
(some? position-modifier)
(gpt/transform position-modifier))
drag? (mf/use-ref nil)
was-open? (mf/use-ref nil)
dragging-ref (mf/use-ref false)
start-ref (mf/use-ref nil)
position (:position thread)
frame-id (:frame-id thread)
state (mf/use-state
@ -1097,8 +1162,14 @@
:new-position-y nil
:new-frame-id frame-id}))
pos-x (floor (* (or (:new-position-x @state) (:x base-pos)) zoom))
pos-y (floor (* (or (:new-position-y @state) (:y base-pos)) zoom))
pos-x (floor (* (or (:new-position-x @state) (:x position)) zoom))
pos-y (floor (* (or (:new-position-y @state) (:y position)) zoom))
drag? (mf/use-ref nil)
was-open? (mf/use-ref nil)
dragging-ref (mf/use-ref false)
start-ref (mf/use-ref nil)
on-pointer-down
(mf/use-fn

11
frontend/src/app/main/ui/comments.scss
View file

@ -91,6 +91,17 @@
border-radius: $br-circle;
}
.avatar-text {
border-radius: $br-circle;
height: 100%;
width: 100%;
display: flex;
align-items: center;
justify-content: center;
font-size: $fs-12;
background-color: var(--color-background-quaternary);
}
.avatar-mask {
border-radius: $br-circle;
position: absolute;

2
frontend/src/app/main/ui/dashboard/comments.cljs
View file

@ -62,7 +62,7 @@
on-navigate
(mf/use-callback
(fn [thread]
(st/emit! (-> (dwcm/navigate thread)
(st/emit! (-> (dwcm/navigate-to-comment-from-dashboard thread)
(with-meta {::ev/origin "dashboard"})))))
on-read-all

18
frontend/src/app/main/ui/workspace/viewport/comments.cljs
View file

@ -16,6 +16,7 @@
[rumext.v2 :as mf]))
(mf/defc comments-layer*
{::mf/wrap [mf/memo]}
[{:keys [vbox vport zoom file-id page-id]}]
(let [vbox-x (dm/get-prop vbox :x)
vbox-y (dm/get-prop vbox :y)
@ -59,11 +60,18 @@
:height (dm/str vport-h "px")}}
[:div {:class (stl/css :threads)
:style {:transform (dm/fmt "translate(%px, %px)" pos-x pos-y)}}
(for [item threads]
[:> cmt/comment-floating-bubble* {:thread item
:zoom zoom
:is-open (= (:id item) (:open local))
:key (:seqn item)}])
(for [thread-group (cmt/group-bubbles zoom threads)]
(let [group? (> (count thread-group) 1)
thread (first thread-group)]
(if group?
[:> cmt/comment-floating-group* {:thread-group thread-group
:zoom zoom
:key (:seqn thread)}]
[:> cmt/comment-floating-bubble* {:thread thread
:zoom zoom
:is-open (= (:id thread) (:open local))
:key (:seqn thread)}])))
(when-let [id (:open local)]
(when-let [thread (get threads-map id)]