feat(collab): nth-in-room colors, comment toolbar + drag + global hide, luminance chip text
- presence.ts: colors claimed by ARRIVAL ORDER — each client takes the lowest free palette slot (no birthday-problem hash collisions; the best palette colors go first; N ≤ palette-size editors always distinct). Coordination- free: claims ride awareness; simultaneous-join collisions converge (lower clientID keeps, the other re-claims), same-user tabs adopt one color, claims are sticky across eeschema sheet rebinds (skeletons reuse them). colorOf(userId) resolves live colors; comment pins/popovers follow it (offline authors fall back to the hash). +2 unit tests (verified live: alice=slot0, bob=slot1, roster/pins consistent). - CommentLayer: single comment icon expanding into a horizontal bar — new comment · list · show/hide all (eye empties the GAL pin set + DOM targets; re-shown on new-comment/panel-jump). Pins are DRAGGABLE: live LWW anchor writes while dragging (peers + the GAL dot follow), nearest-item re-snap on drop, click-vs-drag by 4px threshold (verified live: drag synced to the peer tab exactly). Shared setThreadAnchor + controller moveThread/ setPinsVisible/colorFor. - collab_presence_style.h: chip text color by background luminance (dark on light chips, white on dark — BitmapText draws with the stroke color). - comments/presence-roster e2e updated for the bar + passing; collab units 56/56; shared 122/122; shared pointer bump. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CvqUd4QsJSGHN28aunJRTq
This commit is contained in:
Gergő Törcsvári
parent
0f19811602
commit
21e3df8fd0
9 changed files with 399 additions and 70 deletions
|
|
@ -26,8 +26,11 @@ async function bootAs(page: Page, user: string): Promise<void> {
|
|||
intervals: [1000],
|
||||
})
|
||||
.toMatch(TITLE);
|
||||
// The comment controls mount once the collab session + bridge are up.
|
||||
await expect(page.getByTestId('comment-mode-toggle')).toBeVisible({ timeout: 30000 });
|
||||
// The comment controls mount once the collab session + bridge are up; the
|
||||
// action buttons live inside the expandable bar — open it for the test.
|
||||
await expect(page.getByTestId('comment-bar-toggle')).toBeVisible({ timeout: 30000 });
|
||||
await page.getByTestId('comment-bar-toggle').click();
|
||||
await expect(page.getByTestId('comment-mode-toggle')).toBeVisible();
|
||||
}
|
||||
|
||||
/** Delete every leftover thread from previous runs — comments PERSIST in the
|
||||
|
|
|
|||
|
|
@ -165,6 +165,15 @@ inline double textWidth( const std::string& aText, double aGlyphH )
|
|||
return aText.size() * aGlyphH * 0.75;
|
||||
}
|
||||
|
||||
// Legible text color for a chip background: near-black on light colors,
|
||||
// white on dark ones (BitmapText draws with the GAL stroke color).
|
||||
inline KIGFX::COLOR4D chipTextColor( const KIGFX::COLOR4D& aBg )
|
||||
{
|
||||
double lum = 0.299 * aBg.r + 0.587 * aBg.g + 0.114 * aBg.b;
|
||||
return lum > 0.6 ? KIGFX::COLOR4D( 0.08, 0.08, 0.08, 1.0 )
|
||||
: KIGFX::COLOR4D( 1.0, 1.0, 1.0, 1.0 );
|
||||
}
|
||||
|
||||
/** Name tag next to (or inside) a box, per the label placement knobs. `px` is
|
||||
* world-units-per-screen-pixel. GAL BitmapText CENTERS on its position (the
|
||||
* default justify is CENTER/CENTER — GAL::ResetTextAttributes), so the block
|
||||
|
|
@ -203,7 +212,7 @@ inline void drawLabel( KIGFX::VIEW_OVERLAY* aOv, const BOX2I& aBox, const std::s
|
|||
VECTOR2D( x + w + padX, y + h + padY ) );
|
||||
aOv->SetIsStroke( true );
|
||||
aOv->SetIsFill( false );
|
||||
aOv->SetStrokeColor( KIGFX::COLOR4D( 1, 1, 1, 1 ) );
|
||||
aOv->SetStrokeColor( chipTextColor( aColor ) );
|
||||
}
|
||||
else
|
||||
{
|
||||
|
|
@ -376,7 +385,7 @@ inline void drawCursor( KIGFX::VIEW_OVERLAY* aOv, const VECTOR2D& aPos, const st
|
|||
at + VECTOR2D( w + padX, h + padY ) );
|
||||
aOv->SetIsStroke( true );
|
||||
aOv->SetIsFill( false );
|
||||
aOv->SetStrokeColor( KIGFX::COLOR4D( 1, 1, 1, 1 ) );
|
||||
aOv->SetStrokeColor( chipTextColor( aColor ) );
|
||||
}
|
||||
else
|
||||
{
|
||||
|
|
|
|||
|
|
@ -1 +1 @@
|
|||
Subproject commit b83ad94017531fef359c0478e616ab269d1234d0
|
||||
Subproject commit 5f5effa4e0d9cab0261a5e0f771c7c4cb31b427d
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
import * as React from "react";
|
||||
import { colorForUser, type CommentAnchor } from "@pcbjam/shared";
|
||||
import { MessageSquarePlus, MessageSquareText, X } from "lucide-react";
|
||||
import type { CommentAnchor } from "@pcbjam/shared";
|
||||
import { Eye, EyeOff, List, MessageSquarePlus, MessageSquareText, X } from "lucide-react";
|
||||
import {
|
||||
screenToWorld,
|
||||
worldToScreen,
|
||||
|
|
@ -12,12 +12,13 @@ import {
|
|||
/**
|
||||
* DOM half of the hybrid comment pins (collab-presence 0005): the GAL overlay
|
||||
* draws the dots (zero pan/zoom lag); this layer adds what DOM does better —
|
||||
* invisible click targets over each dot, the thread popover (read/reply/edit/
|
||||
* resolve/delete), the comment-mode click catcher + composer, and a small
|
||||
* list panel with jump-to. Positions map world→canvas-px via the exported
|
||||
* viewport transform, then canvas-px→CSS via the GAL panel's bounding rect
|
||||
* (`#glcanvas-*` — `#canvas` is the whole wx window). A ≤1-frame skew against
|
||||
* the GAL dot during pan is accepted; it only affects the hit target.
|
||||
* click/drag targets over each dot, the thread popover (read/reply/edit/
|
||||
* resolve/delete), the comment-mode click catcher + composer, and the list
|
||||
* panel. One comment icon (top-right) expands into a small horizontal bar:
|
||||
* new comment · list · show/hide all. Pins are draggable — the anchor is
|
||||
* re-written live while dragging (LWW), and re-snapped to the nearest item on
|
||||
* drop. Positions map world→canvas-px via the exported viewport transform,
|
||||
* then canvas-px→CSS via the GAL panel's bounding rect (`#glcanvas-*`).
|
||||
*/
|
||||
|
||||
interface CssRect {
|
||||
|
|
@ -46,6 +47,9 @@ function timeAgo(ms: number): string {
|
|||
return `${Math.floor(s / 86400)}d`;
|
||||
}
|
||||
|
||||
const DRAG_THRESHOLD_PX = 4;
|
||||
const DRAG_SYNC_MS = 60;
|
||||
|
||||
export function CommentLayer({
|
||||
controller,
|
||||
viewport,
|
||||
|
|
@ -56,13 +60,25 @@ export function CommentLayer({
|
|||
currentUser: string;
|
||||
}) {
|
||||
const [threads, setThreads] = React.useState<ResolvedThread[]>(controller.threads());
|
||||
const [barOpen, setBarOpen] = React.useState(false);
|
||||
const [mode, setMode] = React.useState(false);
|
||||
const [openId, setOpenId] = React.useState<string | null>(null);
|
||||
const [panel, setPanel] = React.useState(false);
|
||||
const [showResolved, setShowResolved] = React.useState(false);
|
||||
const [hidden, setHidden] = React.useState(!controller.pinsVisible());
|
||||
const [draft, setDraft] = React.useState<{ anchor: CommentAnchor; css: { x: number; y: number } } | null>(null);
|
||||
// The GAL panel's CSS rect — re-measured on viewport pushes + window resize.
|
||||
const [glRect, setGlRect] = React.useState<CssRect | null>(null);
|
||||
// Live drag state: the dragged thread follows the pointer in CSS space (the
|
||||
// GAL dot follows through the throttled anchor writes).
|
||||
const [drag, setDrag] = React.useState<{ id: string; css: { x: number; y: number } } | null>(null);
|
||||
const dragRef = React.useRef<{
|
||||
id: string;
|
||||
startX: number;
|
||||
startY: number;
|
||||
moved: boolean;
|
||||
lastSync: number;
|
||||
} | null>(null);
|
||||
|
||||
// Re-seed on controller rebind (eeschema sheet switch swaps the controller;
|
||||
// the useState initializer only covers the first mount).
|
||||
|
|
@ -70,6 +86,7 @@ export function CommentLayer({
|
|||
setThreads(controller.threads());
|
||||
setOpenId(null);
|
||||
setDraft(null);
|
||||
setHidden(!controller.pinsVisible());
|
||||
return controller.subscribe(setThreads);
|
||||
}, [controller]);
|
||||
|
||||
|
|
@ -104,19 +121,76 @@ export function CommentLayer({
|
|||
return { x, y };
|
||||
};
|
||||
|
||||
const cssToWorld = (css: { x: number; y: number }) => {
|
||||
if (!viewport || !glRect) return null;
|
||||
return screenToWorld(viewport, {
|
||||
x: (css.x - glRect.x) / cssRatio,
|
||||
y: (css.y - glRect.y) / cssRatio,
|
||||
});
|
||||
};
|
||||
|
||||
const snapRadiusIu = () =>
|
||||
viewport ? 14 / (viewport.scale * cssRatio) : 0;
|
||||
|
||||
const onModeClick = (e: React.MouseEvent) => {
|
||||
if (!viewport || !glRect) return;
|
||||
const px = {
|
||||
x: (e.clientX - glRect.x) / cssRatio,
|
||||
y: (e.clientY - glRect.y) / cssRatio,
|
||||
};
|
||||
const world = screenToWorld(viewport, px);
|
||||
// Snap to an item within ~14 CSS px so the pin tracks the obvious target.
|
||||
const maxDistIu = 14 / (viewport.scale * cssRatio);
|
||||
setDraft({ anchor: controller.anchorAt(world, maxDistIu), css: { x: e.clientX, y: e.clientY } });
|
||||
const world = cssToWorld({ x: e.clientX, y: e.clientY });
|
||||
if (!world) return;
|
||||
setDraft({
|
||||
anchor: controller.anchorAt(world, snapRadiusIu()),
|
||||
css: { x: e.clientX, y: e.clientY },
|
||||
});
|
||||
setMode(false);
|
||||
};
|
||||
|
||||
// ── pin dragging ────────────────────────────────────────────────────────
|
||||
const onPinPointerDown = (t: ResolvedThread) => (e: React.PointerEvent) => {
|
||||
(e.target as HTMLElement).setPointerCapture(e.pointerId);
|
||||
dragRef.current = { id: t.id, startX: e.clientX, startY: e.clientY, moved: false, lastSync: 0 };
|
||||
};
|
||||
|
||||
const onPinPointerMove = (e: React.PointerEvent) => {
|
||||
const d = dragRef.current;
|
||||
if (!d) return;
|
||||
if (!d.moved) {
|
||||
if (Math.hypot(e.clientX - d.startX, e.clientY - d.startY) < DRAG_THRESHOLD_PX) return;
|
||||
d.moved = true;
|
||||
setOpenId(null);
|
||||
}
|
||||
setDrag({ id: d.id, css: { x: e.clientX, y: e.clientY } });
|
||||
// Throttled live re-anchor (pos-only while dragging — snap happens on
|
||||
// drop) so the GAL dot and every peer follow the drag.
|
||||
const now = Date.now();
|
||||
if (now - d.lastSync >= DRAG_SYNC_MS) {
|
||||
d.lastSync = now;
|
||||
const world = cssToWorld({ x: e.clientX, y: e.clientY });
|
||||
if (world) controller.moveThread(d.id, { pos: world });
|
||||
}
|
||||
};
|
||||
|
||||
const onPinPointerUp = (t: ResolvedThread) => (e: React.PointerEvent) => {
|
||||
const d = dragRef.current;
|
||||
dragRef.current = null;
|
||||
setDrag(null);
|
||||
if (!d) return;
|
||||
if (d.moved) {
|
||||
const world = cssToWorld({ x: e.clientX, y: e.clientY });
|
||||
if (world) controller.moveThread(t.id, controller.anchorAt(world, snapRadiusIu()));
|
||||
} else {
|
||||
setOpenId((cur) => (cur === t.id ? null : t.id));
|
||||
}
|
||||
};
|
||||
|
||||
const toggleHidden = () => {
|
||||
const next = !hidden;
|
||||
setHidden(next);
|
||||
controller.setPinsVisible(!next);
|
||||
if (next) {
|
||||
setOpenId(null);
|
||||
setMode(false);
|
||||
setDraft(null);
|
||||
}
|
||||
};
|
||||
|
||||
const open = openId ? threads.find((t) => t.id === openId) : undefined;
|
||||
// A thread opened from the panel may sit off-screen (jump-to clamps at the
|
||||
// view bounds) — fall back to a centered popover rather than rendering none.
|
||||
|
|
@ -124,33 +198,59 @@ export function CommentLayer({
|
|||
? (toCss(open.world) ?? { x: window.innerWidth / 2 - 150, y: 120 })
|
||||
: null;
|
||||
const visibleThreads = threads.filter((t) => showResolved || !t.resolved);
|
||||
const pinThreads = hidden ? [] : visibleThreads;
|
||||
|
||||
return (
|
||||
<>
|
||||
{/* Mode + panel toggles, below the top-right chips. */}
|
||||
<div className="absolute right-3 top-12 z-30 flex flex-col items-end gap-2">
|
||||
{/* Comment toolbar: one icon expanding into a small horizontal bar. */}
|
||||
<div className="absolute right-3 top-12 z-30 flex flex-row-reverse items-center gap-2">
|
||||
<button
|
||||
data-testid="comment-mode-toggle"
|
||||
title={mode ? "Cancel comment (Esc)" : "Add a comment"}
|
||||
onClick={() => {
|
||||
setMode((m) => !m);
|
||||
setDraft(null);
|
||||
}}
|
||||
className={`flex h-8 w-8 items-center justify-center rounded-full shadow-sm ring-1 ring-inset ring-white/20 ${
|
||||
mode ? "bg-amber-500 text-black" : "bg-black/70 text-white hover:bg-black/85"
|
||||
data-testid="comment-bar-toggle"
|
||||
title="Comments"
|
||||
onClick={() => setBarOpen((o) => !o)}
|
||||
className={`flex h-8 min-w-8 items-center justify-center gap-1 rounded-full px-2 text-xs shadow-sm ring-1 ring-inset ring-white/20 ${
|
||||
barOpen ? "bg-sky-600 text-white" : "bg-black/70 text-white hover:bg-black/85"
|
||||
}`}
|
||||
>
|
||||
<MessageSquarePlus size={16} />
|
||||
</button>
|
||||
<button
|
||||
data-testid="comment-panel-toggle"
|
||||
title="Comments"
|
||||
onClick={() => setPanel((p) => !p)}
|
||||
className="flex h-8 min-w-8 items-center justify-center gap-1 rounded-full bg-black/70 px-2 text-xs text-white shadow-sm ring-1 ring-inset ring-white/20 hover:bg-black/85"
|
||||
>
|
||||
<MessageSquareText size={14} />
|
||||
<MessageSquareText size={15} />
|
||||
{threads.length > 0 && <span>{threads.length}</span>}
|
||||
</button>
|
||||
{barOpen && (
|
||||
<div className="flex items-center gap-1 rounded-full bg-black/70 p-1 shadow-sm ring-1 ring-inset ring-white/20">
|
||||
<button
|
||||
data-testid="comment-mode-toggle"
|
||||
title={mode ? "Cancel comment (Esc)" : "New comment"}
|
||||
onClick={() => {
|
||||
if (hidden) toggleHidden();
|
||||
setMode((m) => !m);
|
||||
setDraft(null);
|
||||
}}
|
||||
className={`flex h-6 w-6 items-center justify-center rounded-full ${
|
||||
mode ? "bg-amber-500 text-black" : "text-white hover:bg-white/15"
|
||||
}`}
|
||||
>
|
||||
<MessageSquarePlus size={14} />
|
||||
</button>
|
||||
<button
|
||||
data-testid="comment-panel-toggle"
|
||||
title="Comment list"
|
||||
onClick={() => setPanel((p) => !p)}
|
||||
className={`flex h-6 w-6 items-center justify-center rounded-full ${
|
||||
panel ? "bg-white/25 text-white" : "text-white hover:bg-white/15"
|
||||
}`}
|
||||
>
|
||||
<List size={14} />
|
||||
</button>
|
||||
<button
|
||||
data-testid="comment-visibility-toggle"
|
||||
title={hidden ? "Show comments" : "Hide comments"}
|
||||
onClick={toggleHidden}
|
||||
className="flex h-6 w-6 items-center justify-center rounded-full text-white hover:bg-white/15"
|
||||
>
|
||||
{hidden ? <EyeOff size={14} /> : <Eye size={14} />}
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Comment-mode click catcher over the drawing area only. */}
|
||||
|
|
@ -163,19 +263,23 @@ export function CommentLayer({
|
|||
/>
|
||||
)}
|
||||
|
||||
{/* Pin hit targets (the visual dot is GAL — these are the clickable DOM halves). */}
|
||||
{visibleThreads.map((t) => {
|
||||
const css = toCss(t.world);
|
||||
{/* Pin hit/drag targets (the visual dot is GAL — these are the DOM halves). */}
|
||||
{pinThreads.map((t) => {
|
||||
const css = drag?.id === t.id ? drag.css : toCss(t.world);
|
||||
if (!css) return null;
|
||||
return (
|
||||
<button
|
||||
key={t.id}
|
||||
data-testid="comment-pin"
|
||||
data-thread-id={t.id}
|
||||
title={`${t.createdBy}: ${t.messages[0]?.body ?? ""}${t.detached ? " (detached)" : ""}`}
|
||||
onClick={() => setOpenId((cur) => (cur === t.id ? null : t.id))}
|
||||
className="absolute z-30 -translate-x-1/2 -translate-y-1/2 rounded-full hover:ring-2 hover:ring-white/70"
|
||||
style={{ left: css.x, top: css.y, width: 22, height: 22, background: "transparent" }}
|
||||
title={`${t.createdBy}: ${t.messages[0]?.body ?? ""}${t.detached ? " (detached)" : ""} — drag to move`}
|
||||
onPointerDown={onPinPointerDown(t)}
|
||||
onPointerMove={onPinPointerMove}
|
||||
onPointerUp={onPinPointerUp(t)}
|
||||
className={`absolute z-30 -translate-x-1/2 -translate-y-1/2 rounded-full ${
|
||||
drag?.id === t.id ? "cursor-grabbing ring-2 ring-white" : "cursor-grab hover:ring-2 hover:ring-white/70"
|
||||
}`}
|
||||
style={{ left: css.x, top: css.y, width: 22, height: 22, background: "transparent", touchAction: "none" }}
|
||||
/>
|
||||
);
|
||||
})}
|
||||
|
|
@ -194,7 +298,7 @@ export function CommentLayer({
|
|||
)}
|
||||
|
||||
{/* Thread popover next to its pin. */}
|
||||
{open && openCss && (
|
||||
{open && openCss && !hidden && (
|
||||
<ThreadPopover
|
||||
thread={open}
|
||||
css={openCss}
|
||||
|
|
@ -228,12 +332,13 @@ export function CommentLayer({
|
|||
key={t.id}
|
||||
data-testid="comment-panel-item"
|
||||
onClick={() => {
|
||||
if (hidden) toggleHidden();
|
||||
controller.jumpTo(t.id);
|
||||
setOpenId(t.id);
|
||||
}}
|
||||
className="block w-full border-t border-white/10 px-3 py-2 text-left text-xs hover:bg-white/10"
|
||||
>
|
||||
<span className="font-semibold" style={{ color: colorForUser(t.createdBy) }}>
|
||||
<span className="font-semibold" style={{ color: controller.colorFor(t.createdBy) }}>
|
||||
{t.createdBy}
|
||||
</span>{" "}
|
||||
<span className="text-white/50">
|
||||
|
|
@ -368,7 +473,7 @@ function ThreadPopover({
|
|||
{thread.messages.map((m) => (
|
||||
<div key={m.id} data-testid="comment-message" className="group px-3 py-2 text-xs">
|
||||
<div className="flex items-baseline gap-2">
|
||||
<span className="font-semibold" style={{ color: colorForUser(m.author) }}>
|
||||
<span className="font-semibold" style={{ color: controller.colorFor(m.author) }}>
|
||||
{m.author}
|
||||
</span>
|
||||
<span className="text-[10px] text-white/40">
|
||||
|
|
|
|||
|
|
@ -821,7 +821,15 @@ export function WasmTool({
|
|||
if (!doc || (tool !== "pcbnew" && tool !== "eeschema") || !hasCommentsBridge(win.Module)) {
|
||||
return;
|
||||
}
|
||||
const ctl = createComments({ doc, mod: win.Module, user: presenceUser().id, tool });
|
||||
const ctl = createComments({
|
||||
doc,
|
||||
mod: win.Module,
|
||||
user: presenceUser().id,
|
||||
tool,
|
||||
// Author colors follow the live nth-in-room assignment when the
|
||||
// author is present; offline authors fall back to the name hash.
|
||||
colorFor: (id) => presenceRef.current?.colorOf(id),
|
||||
});
|
||||
commentsRef.current = ctl;
|
||||
setCommentsCtl(ctl);
|
||||
// Test/debug handle (mirrors window.kicadCollab): lets the e2e reset
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import {
|
|||
observeComments,
|
||||
removeMessage,
|
||||
resolveAnchor,
|
||||
setThreadAnchor,
|
||||
setThreadResolved,
|
||||
yToItemUnchecked,
|
||||
type CommentAnchor,
|
||||
|
|
@ -95,6 +96,14 @@ export interface CommentsController {
|
|||
remove(threadId: string, messageId: string): "removed" | "thread-deleted" | false;
|
||||
setResolved(threadId: string, resolved: boolean): void;
|
||||
deleteThread(threadId: string): void;
|
||||
/** Re-pin a thread (drag): LWW anchor replace, live-syncs to peers. */
|
||||
moveThread(threadId: string, anchor: CommentAnchor): void;
|
||||
/** Globally show/hide the pins (GAL dots follow; DOM targets are the
|
||||
* layer's own state). */
|
||||
setPinsVisible(visible: boolean): void;
|
||||
pinsVisible(): boolean;
|
||||
/** Presence-aware author color (nth-in-room when online, hash fallback). */
|
||||
colorFor(userId: string): string;
|
||||
/** Pan the editor to a thread's pin (comment panel "jump to"). */
|
||||
jumpTo(threadId: string): void;
|
||||
destroy(): void;
|
||||
|
|
@ -108,11 +117,15 @@ export function createComments(opts: {
|
|||
/** Author slug for new messages (presence identity). */
|
||||
user: string;
|
||||
tool: string;
|
||||
/** Presence color resolver (nth-in-room); undefined falls back to the hash. */
|
||||
colorFor?: (userId: string) => string | undefined;
|
||||
}): CommentsController {
|
||||
const { doc, mod, user } = opts;
|
||||
const iuPerMm = IU_PER_MM[opts.tool] ?? 1e6;
|
||||
const colorFor = (userId: string) => opts.colorFor?.(userId) ?? colorForUser(userId);
|
||||
|
||||
let cache: ResolvedThread[] = [];
|
||||
let visible = true;
|
||||
const subscribers = new Set<(threads: ResolvedThread[]) => void>();
|
||||
|
||||
const recompute = (): ResolvedThread[] => {
|
||||
|
|
@ -128,18 +141,21 @@ export function createComments(opts: {
|
|||
JSON.stringify({
|
||||
// Resolved threads drop their dot (figma-style) — they stay reachable
|
||||
// through the panel's "resolved" filter. Matches the DOM hit targets.
|
||||
pins: cache
|
||||
.filter((t) => !t.resolved)
|
||||
.map((t) => ({
|
||||
id: t.id,
|
||||
// Author name rides along so the tuner's palette override can
|
||||
// recolor pins consistently with that user's cursor/boxes.
|
||||
name: t.createdBy,
|
||||
x: t.world.x,
|
||||
y: t.world.y,
|
||||
color: colorForUser(t.createdBy),
|
||||
resolved: t.resolved,
|
||||
})),
|
||||
// A global hide (toolbar eye) empties the set entirely.
|
||||
pins: !visible
|
||||
? []
|
||||
: cache
|
||||
.filter((t) => !t.resolved)
|
||||
.map((t) => ({
|
||||
id: t.id,
|
||||
// Author name rides along so the tuner's palette override can
|
||||
// recolor pins consistently with that user's cursor/boxes.
|
||||
name: t.createdBy,
|
||||
x: t.world.x,
|
||||
y: t.world.y,
|
||||
color: colorFor(t.createdBy),
|
||||
resolved: t.resolved,
|
||||
})),
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
|
@ -218,6 +234,15 @@ export function createComments(opts: {
|
|||
deleteThread(threadId) {
|
||||
deleteThread(doc, threadId);
|
||||
},
|
||||
moveThread(threadId, anchor) {
|
||||
setThreadAnchor(doc, threadId, anchor);
|
||||
},
|
||||
setPinsVisible(v) {
|
||||
visible = v;
|
||||
pushPins();
|
||||
},
|
||||
pinsVisible: () => visible,
|
||||
colorFor,
|
||||
jumpTo(threadId) {
|
||||
const t = cache.find((x) => x.id === threadId);
|
||||
if (t) mod.kicadCollabSetViewport(t.world.x, t.world.y);
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ function stubPresence(peers: PresencePeer[] = []) {
|
|||
},
|
||||
setCursor: vi.fn(),
|
||||
setSelection: vi.fn(),
|
||||
colorOf: vi.fn((id: string) => colorForUser(id)),
|
||||
destroy: vi.fn(),
|
||||
firePeers(p) {
|
||||
peers = p;
|
||||
|
|
|
|||
|
|
@ -1,9 +1,14 @@
|
|||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import * as Y from "yjs";
|
||||
import { Awareness } from "y-protocols/awareness";
|
||||
import { colorForUser, type PresenceUser } from "@pcbjam/shared";
|
||||
import { colorForUser, PRESENCE_COLORS, type PresenceUser } from "@pcbjam/shared";
|
||||
import { connectAwarenessBroadcast } from "./awareness-bc";
|
||||
import { createPresence, publishSkeleton, type PresenceHandle } from "./presence";
|
||||
import {
|
||||
createPresence,
|
||||
publishSkeleton,
|
||||
resetPresenceColorClaims,
|
||||
type PresenceHandle,
|
||||
} from "./presence";
|
||||
|
||||
/**
|
||||
* Presence unit tests (collab-presence 0001): two Awareness instances relayed
|
||||
|
|
@ -46,6 +51,7 @@ function client(channel: string): Client {
|
|||
afterEach(() => {
|
||||
for (const c of clients) c.destroy();
|
||||
clients = [];
|
||||
resetPresenceColorClaims();
|
||||
});
|
||||
|
||||
describe("presence over the BroadcastChannel awareness relay", () => {
|
||||
|
|
@ -209,7 +215,65 @@ describe("presence over the BroadcastChannel awareness relay", () => {
|
|||
// setCursor must not have clobbered the selection set just before it.
|
||||
expect(bob.selection).toEqual(["uuid-1"]);
|
||||
expect(bob.cursor).toEqual({ x: 5, y: 7 });
|
||||
expect(bob.user.color).toBe(colorForUser("bob"));
|
||||
expect(PRESENCE_COLORS).toContain(bob.user.color);
|
||||
unsub();
|
||||
});
|
||||
|
||||
it("assigns colors by arrival order (nth-in-room), not name hash", async () => {
|
||||
// Sequential joins: each client's relay settles (peer states arrived —
|
||||
// production attaches presence after the provider synced) before claiming.
|
||||
const channel = `presence-test-${channelSeq++}`;
|
||||
const a = client(channel);
|
||||
await settle();
|
||||
a.presence = createPresence({ awareness: a.awareness, user: user("alice"), tool: "pcbnew" });
|
||||
await settle();
|
||||
|
||||
const b = client(channel);
|
||||
await settle();
|
||||
b.presence = createPresence({ awareness: b.awareness, user: user("bob"), tool: "pcbnew" });
|
||||
await settle();
|
||||
|
||||
const c = client(channel);
|
||||
await settle();
|
||||
c.presence = createPresence({ awareness: c.awareness, user: user("carol"), tool: "pcbnew" });
|
||||
await settle();
|
||||
|
||||
// First three arrivals take the first three palette slots, in order —
|
||||
// no birthday-problem collisions while the room fits the palette.
|
||||
const seen = c.presence.peers().map((p) => [p.user.id, p.user.color]);
|
||||
expect(seen).toContainEqual(["alice", PRESENCE_COLORS[0]]);
|
||||
expect(seen).toContainEqual(["bob", PRESENCE_COLORS[1]]);
|
||||
const carol = a.presence.peers().find((p) => p.user.id === "carol")!;
|
||||
expect(carol.user.color).toBe(PRESENCE_COLORS[2]);
|
||||
|
||||
// colorOf resolves present users to their claimed color, absent users to
|
||||
// the hash fallback (offline comment authors).
|
||||
expect(a.presence.colorOf("bob")).toBe(PRESENCE_COLORS[1]);
|
||||
expect(a.presence.colorOf("alice")).toBe(PRESENCE_COLORS[0]);
|
||||
expect(a.presence.colorOf("nobody")).toBe(colorForUser("nobody"));
|
||||
});
|
||||
|
||||
it("a second tab of the same user adopts the existing color", async () => {
|
||||
const channel = `presence-test-${channelSeq++}`;
|
||||
const a1 = client(channel);
|
||||
await settle();
|
||||
a1.presence = createPresence({ awareness: a1.awareness, user: user("alice"), tool: "pcbnew" });
|
||||
await settle();
|
||||
const b = client(channel);
|
||||
await settle();
|
||||
b.presence = createPresence({ awareness: b.awareness, user: user("bob"), tool: "pcbnew" });
|
||||
await settle();
|
||||
// The second alice tab simulates a separate browser: no local claim
|
||||
// memory, adopts the published color from the room.
|
||||
resetPresenceColorClaims();
|
||||
const a2 = client(channel);
|
||||
await settle();
|
||||
a2.presence = createPresence({ awareness: a2.awareness, user: user("alice"), tool: "pcbnew" });
|
||||
await settle();
|
||||
|
||||
// bob sees ONE alice with ONE consistent color; bob keeps his own slot.
|
||||
const fromBob = b.presence.peers();
|
||||
expect(fromBob.map((p) => p.user.id)).toEqual(["alice"]);
|
||||
expect(fromBob[0]!.user.color).toBe(PRESENCE_COLORS[0]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
import type { Awareness } from "y-protocols/awareness";
|
||||
import {
|
||||
colorForUser,
|
||||
PRESENCE_COLORS,
|
||||
presenceStateSchema,
|
||||
type PresenceState,
|
||||
type PresenceUser,
|
||||
|
|
@ -35,11 +37,57 @@ export interface PresenceHandle {
|
|||
setCursor(pos: { x: number; y: number } | null): void;
|
||||
/** 0002: publish the local selection's KIID strings. */
|
||||
setSelection(uuids: string[]): void;
|
||||
/** A user's current presence color: their published one if they're in the
|
||||
* room, our claimed one for ourselves, else the palette-hash fallback
|
||||
* (offline users, e.g. old comment authors). */
|
||||
colorOf(userId: string): string;
|
||||
/** Clear the local state and detach. The awareness instance itself belongs
|
||||
* to the provider and stays alive (rebind re-uses it). */
|
||||
destroy(): void;
|
||||
}
|
||||
|
||||
// ── nth-in-room color assignment ─────────────────────────────────────────────
|
||||
//
|
||||
// Colors are claimed by ARRIVAL ORDER, not name hash: each client takes the
|
||||
// lowest palette index no other peer holds, so N ≤ palette-size editors always
|
||||
// get distinct colors (a hash would collide birthday-problem-fast) and the
|
||||
// best palette colors go first. Coordination-free: the claim is published in
|
||||
// the awareness state; when two simultaneous joiners collide, the one with the
|
||||
// HIGHER awareness clientID yields and re-claims (deterministic convergence).
|
||||
// The claim is per-tab and sticky across room rebinds (eeschema sheet pool),
|
||||
// so one user keeps one color everywhere in a session; a second tab of the
|
||||
// same user ADOPTS the existing color instead of claiming a new one.
|
||||
|
||||
// Per-user claims in this JS context (one user per tab in production; the
|
||||
// map keeps multi-client unit tests deterministic).
|
||||
const g_claims = new Map<string, string>();
|
||||
|
||||
/** Test hook: forget all color claims. */
|
||||
export function resetPresenceColorClaims(): void {
|
||||
g_claims.clear();
|
||||
}
|
||||
|
||||
/** Least-used palette color among the OTHER users in `states` — the lowest
|
||||
* free slot while the room is smaller than the palette, fair reuse after. */
|
||||
function lowestFreeColor(
|
||||
states: Map<number, unknown>,
|
||||
ownClientId: number,
|
||||
ownUserId: string,
|
||||
): string {
|
||||
const usage = new Array(PRESENCE_COLORS.length).fill(0);
|
||||
|
||||
for (const [clientId, state] of states) {
|
||||
if (clientId === ownClientId) continue;
|
||||
const parsed = presenceStateSchema.safeParse(state);
|
||||
if (!parsed.success || parsed.data.user.id === ownUserId) continue;
|
||||
const idx = (PRESENCE_COLORS as readonly string[]).indexOf(parsed.data.user.color);
|
||||
if (idx >= 0) usage[idx]++;
|
||||
}
|
||||
|
||||
const min = Math.min(...usage);
|
||||
return PRESENCE_COLORS[usage.indexOf(min)]!;
|
||||
}
|
||||
|
||||
/**
|
||||
* Publish a SKELETON presence state into a room the user is connected to but
|
||||
* not looking at (eeschema warm pool, collab-presence 0003): identity + which
|
||||
|
|
@ -54,7 +102,9 @@ export function publishSkeleton(
|
|||
sheetPath: string,
|
||||
): void {
|
||||
const state: PresenceState = {
|
||||
user,
|
||||
// Skeletons reuse the claimed color so one user is one color on every
|
||||
// sheet's roster.
|
||||
user: { ...user, color: g_claims.get(user.id) ?? user.color },
|
||||
tool,
|
||||
sheetPath,
|
||||
cursor: null,
|
||||
|
|
@ -70,7 +120,28 @@ export function createPresence(opts: {
|
|||
tool: string;
|
||||
sheetPath?: string;
|
||||
}): PresenceHandle {
|
||||
const { awareness, user } = opts;
|
||||
const { awareness } = opts;
|
||||
|
||||
// Nth-in-room color: adopt a same-user tab's color, else keep an earlier
|
||||
// claim (sticky across sheet rebinds), else claim the lowest free palette
|
||||
// slot given the peers already in the room.
|
||||
let claimed = g_claims.get(opts.user.id) ?? null;
|
||||
if (!claimed) {
|
||||
for (const [clientId, raw] of awareness.getStates()) {
|
||||
if (clientId === awareness.clientID) continue;
|
||||
const parsed = presenceStateSchema.safeParse(raw);
|
||||
if (parsed.success && parsed.data.user.id === opts.user.id) {
|
||||
claimed = parsed.data.user.color;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!claimed) {
|
||||
claimed = lowestFreeColor(awareness.getStates(), awareness.clientID, opts.user.id);
|
||||
}
|
||||
g_claims.set(opts.user.id, claimed);
|
||||
|
||||
const user: PresenceUser = { ...opts.user, color: claimed };
|
||||
|
||||
const patch = (fields: Partial<PresenceState>) => {
|
||||
const current = (awareness.getLocalState() ?? {}) as Partial<PresenceState>;
|
||||
|
|
@ -84,7 +155,38 @@ export function createPresence(opts: {
|
|||
cursor: null,
|
||||
selection: [],
|
||||
});
|
||||
clog("presence: published local state for", user.id, "tool", opts.tool);
|
||||
clog("presence: published local state for", user.id, "color", user.color);
|
||||
|
||||
// Simultaneous-join collision: if a DIFFERENT user with a LOWER clientID
|
||||
// holds our color, we yield and re-claim the lowest free slot — exactly one
|
||||
// side of any collision yields, so this converges without coordination.
|
||||
// Same-user tabs converge the other way: adopt the lower clientID's color.
|
||||
const resolveCollision = () => {
|
||||
for (const [clientId, raw] of awareness.getStates()) {
|
||||
if (clientId === awareness.clientID) continue;
|
||||
const parsed = presenceStateSchema.safeParse(raw);
|
||||
if (!parsed.success) continue;
|
||||
|
||||
if (parsed.data.user.id === user.id) {
|
||||
if (parsed.data.user.color !== user.color && clientId < awareness.clientID) {
|
||||
user.color = parsed.data.user.color;
|
||||
g_claims.set(user.id, user.color);
|
||||
patch({ user: { ...user } });
|
||||
clog("presence: adopted same-user color", user.color);
|
||||
return;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if (parsed.data.user.color === user.color && clientId < awareness.clientID) {
|
||||
user.color = lowestFreeColor(awareness.getStates(), awareness.clientID, user.id);
|
||||
g_claims.set(user.id, user.color);
|
||||
patch({ user: { ...user } });
|
||||
clog("presence: color collision — re-claimed", user.color);
|
||||
return;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
function peers(): PresencePeer[] {
|
||||
const byUser = new Map<string, PresencePeer>();
|
||||
|
|
@ -105,6 +207,7 @@ export function createPresence(opts: {
|
|||
|
||||
const subscribers = new Set<(peers: PresencePeer[]) => void>();
|
||||
const onChange = () => {
|
||||
resolveCollision();
|
||||
if (!subscribers.size) return;
|
||||
const snapshot = peers();
|
||||
for (const cb of subscribers) cb(snapshot);
|
||||
|
|
@ -132,6 +235,17 @@ export function createPresence(opts: {
|
|||
setSelection(uuids) {
|
||||
patch({ selection: uuids });
|
||||
},
|
||||
colorOf(userId) {
|
||||
if (userId === user.id) return user.color;
|
||||
for (const [clientId, raw] of awareness.getStates()) {
|
||||
if (clientId === awareness.clientID) continue;
|
||||
const parsed = presenceStateSchema.safeParse(raw);
|
||||
if (parsed.success && parsed.data.user.id === userId) {
|
||||
return parsed.data.user.color;
|
||||
}
|
||||
}
|
||||
return colorForUser(userId);
|
||||
},
|
||||
destroy() {
|
||||
if (destroyed) return;
|
||||
destroyed = true;
|
||||
|
|
|
|||
Loading…
Reference in a new issue