From 57ea4e61d163318edf0f71bec566439821ad7f1b Mon Sep 17 00:00:00 2001 From: David Luzar <5153846+dwelle@users.noreply.github.com> Date: Mon, 18 Dec 2023 18:21:57 +0100 Subject: [PATCH] fix: mixing clientId & socketId in UserList (#7461) --- excalidraw-app/collab/Collab.tsx | 6 +-- excalidraw-app/collab/Portal.tsx | 9 ++-- excalidraw-app/data/index.ts | 7 +-- .../excalidraw/actions/actionNavigate.tsx | 3 +- packages/excalidraw/components/App.tsx | 1 - .../components/LaserTool/LaserPathManager.ts | 3 +- packages/excalidraw/components/UserList.tsx | 46 ++++++++++--------- packages/excalidraw/types.ts | 10 ++-- 8 files changed, 46 insertions(+), 39 deletions(-) diff --git a/excalidraw-app/collab/Collab.tsx b/excalidraw-app/collab/Collab.tsx index a0cdc277..9b26af05 100644 --- a/excalidraw-app/collab/Collab.tsx +++ b/excalidraw-app/collab/Collab.tsx @@ -123,7 +123,7 @@ class Collab extends PureComponent { private socketInitializationTimer?: number; private lastBroadcastedOrReceivedSceneVersion: number = -1; - private collaborators = new Map(); + private collaborators = new Map(); constructor(props: Props) { super(props); @@ -618,7 +618,7 @@ class Collab extends PureComponent { this.portal.socket.on( WS_EVENTS.USER_FOLLOW_ROOM_CHANGE, - (followedBy: string[]) => { + (followedBy: SocketId[]) => { this.excalidrawAPI.updateScene({ appState: { followedBy: new Set(followedBy) }, }); @@ -795,7 +795,7 @@ class Collab extends PureComponent { document.addEventListener(EVENT.VISIBILITY_CHANGE, this.onVisibilityChange); }; - setCollaborators(sockets: string[]) { + setCollaborators(sockets: SocketId[]) { const collaborators: InstanceType["collaborators"] = new Map(); for (const socketId of sockets) { diff --git a/excalidraw-app/collab/Portal.tsx b/excalidraw-app/collab/Portal.tsx index 66dd8a56..bf8ffa5d 100644 --- a/excalidraw-app/collab/Portal.tsx +++ b/excalidraw-app/collab/Portal.tsx @@ -10,6 +10,7 @@ import { ExcalidrawElement } from "../../packages/excalidraw/element/types"; import { WS_EVENTS, FILE_UPLOAD_TIMEOUT, WS_SUBTYPES } from "../app_constants"; import { OnUserFollowedPayload, + SocketId, UserIdleState, } from "../../packages/excalidraw/types"; import { trackEvent } from "../../packages/excalidraw/analytics"; @@ -51,7 +52,7 @@ class Portal { /* syncAll */ true, ); }); - this.socket.on("room-user-change", (clients: string[]) => { + this.socket.on("room-user-change", (clients: SocketId[]) => { this.collab.setCollaborators(clients); }); @@ -186,7 +187,7 @@ class Portal { const data: SocketUpdateDataSource["IDLE_STATUS"] = { type: WS_SUBTYPES.IDLE_STATUS, payload: { - socketId: this.socket.id, + socketId: this.socket.id as SocketId, userState, username: this.collab.state.username, }, @@ -206,7 +207,7 @@ class Portal { const data: SocketUpdateDataSource["MOUSE_LOCATION"] = { type: WS_SUBTYPES.MOUSE_LOCATION, payload: { - socketId: this.socket.id, + socketId: this.socket.id as SocketId, pointer: payload.pointer, button: payload.button || "up", selectedElementIds: @@ -232,7 +233,7 @@ class Portal { const data: SocketUpdateDataSource["USER_VISIBLE_SCENE_BOUNDS"] = { type: WS_SUBTYPES.USER_VISIBLE_SCENE_BOUNDS, payload: { - socketId: this.socket.id, + socketId: this.socket.id as SocketId, username: this.collab.state.username, sceneBounds: payload.sceneBounds, }, diff --git a/excalidraw-app/data/index.ts b/excalidraw-app/data/index.ts index fe44dc13..0f54ee88 100644 --- a/excalidraw-app/data/index.ts +++ b/excalidraw-app/data/index.ts @@ -22,6 +22,7 @@ import { AppState, BinaryFileData, BinaryFiles, + SocketId, UserIdleState, } from "../../packages/excalidraw/types"; import { bytesToHexString } from "../../packages/excalidraw/utils"; @@ -117,7 +118,7 @@ export type SocketUpdateDataSource = { MOUSE_LOCATION: { type: WS_SUBTYPES.MOUSE_LOCATION; payload: { - socketId: string; + socketId: SocketId; pointer: { x: number; y: number; tool: "pointer" | "laser" }; button: "down" | "up"; selectedElementIds: AppState["selectedElementIds"]; @@ -127,7 +128,7 @@ export type SocketUpdateDataSource = { USER_VISIBLE_SCENE_BOUNDS: { type: WS_SUBTYPES.USER_VISIBLE_SCENE_BOUNDS; payload: { - socketId: string; + socketId: SocketId; username: string; sceneBounds: SceneBounds; }; @@ -135,7 +136,7 @@ export type SocketUpdateDataSource = { IDLE_STATUS: { type: WS_SUBTYPES.IDLE_STATUS; payload: { - socketId: string; + socketId: SocketId; userState: UserIdleState; username: string; }; diff --git a/packages/excalidraw/actions/actionNavigate.tsx b/packages/excalidraw/actions/actionNavigate.tsx index 1c55f104..8e8d3023 100644 --- a/packages/excalidraw/actions/actionNavigate.tsx +++ b/packages/excalidraw/actions/actionNavigate.tsx @@ -12,6 +12,7 @@ export const actionGoToCollaborator = register({ trackEvent: { category: "collab" }, perform: (_elements, appState, collaborator: Collaborator) => { if ( + !collaborator.socketId || appState.userToFollow?.socketId === collaborator.socketId || collaborator.isCurrentUser ) { @@ -28,7 +29,7 @@ export const actionGoToCollaborator = register({ appState: { ...appState, userToFollow: { - socketId: collaborator.socketId!, + socketId: collaborator.socketId, username: collaborator.username || "", }, // Close mobile menu diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 5f3f8be2..71d6ea82 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -3472,7 +3472,6 @@ class App extends React.Component { }; private maybeUnfollowRemoteUser = () => { - console.warn("maybeUnfollowRemoteUser"); if (this.state.userToFollow) { this.setState({ userToFollow: null }); } diff --git a/packages/excalidraw/components/LaserTool/LaserPathManager.ts b/packages/excalidraw/components/LaserTool/LaserPathManager.ts index 2f0c6395..b6e462aa 100644 --- a/packages/excalidraw/components/LaserTool/LaserPathManager.ts +++ b/packages/excalidraw/components/LaserTool/LaserPathManager.ts @@ -3,6 +3,7 @@ import { LaserPointer } from "@excalidraw/laser-pointer"; import { sceneCoordsToViewportCoords } from "../../utils"; import App from "../App"; import { getClientColor } from "../../clients"; +import { SocketId } from "../../types"; // decay time in milliseconds const DECAY_TIME = 1000; @@ -88,7 +89,7 @@ type CollabolatorState = { export class LaserPathManager { private ownState: CollabolatorState; - private collaboratorsState: Map = new Map(); + private collaboratorsState: Map = new Map(); private rafId: number | undefined; private isDrawing = false; diff --git a/packages/excalidraw/components/UserList.tsx b/packages/excalidraw/components/UserList.tsx index 4b6aa8e4..2791d9ef 100644 --- a/packages/excalidraw/components/UserList.tsx +++ b/packages/excalidraw/components/UserList.tsx @@ -14,51 +14,54 @@ import { t } from "../i18n"; import { isShallowEqual } from "../utils"; export type GoToCollaboratorComponentProps = [ - SocketId, + ClientId, Collaborator, boolean, boolean, ]; +/** collaborator user id or socket id (fallback) */ +type ClientId = string & { _brand: "UserId" }; + const FIRST_N_AVATARS = 3; const SHOW_COLLABORATORS_FILTER_AT = 8; const ConditionalTooltipWrapper = ({ shouldWrap, children, - socketId, + clientId, username, }: { shouldWrap: boolean; children: React.ReactNode; username?: string | null; - socketId: string; + clientId: ClientId; }) => shouldWrap ? ( - + {children} ) : ( - {children} + {children} ); const renderCollaborator = ({ actionManager, collaborator, - socketId, + clientId, withName = false, shouldWrapWithTooltip = false, isBeingFollowed, }: { actionManager: ActionManager; collaborator: Collaborator; - socketId: string; + clientId: ClientId; withName?: boolean; shouldWrapWithTooltip?: boolean; isBeingFollowed: boolean; }) => { const data: GoToCollaboratorComponentProps = [ - socketId, + clientId, collaborator, withName, isBeingFollowed, @@ -67,8 +70,8 @@ const renderCollaborator = ({ return ( @@ -100,12 +103,13 @@ export const UserList = React.memo( ({ className, mobile, collaborators, userToFollow }: UserListProps) => { const actionManager = useExcalidrawActionManager(); - const uniqueCollaboratorsMap = new Map(); + const uniqueCollaboratorsMap = new Map(); collaborators.forEach((collaborator, socketId) => { + const userId = (collaborator.id || socketId) as ClientId; uniqueCollaboratorsMap.set( // filter on user id, else fall back on unique socketId - collaborator.id || socketId, + userId, { ...collaborator, socketId }, ); }); @@ -134,25 +138,25 @@ export const UserList = React.memo( ); const firstNAvatarsJSX = firstNCollaborators.map( - ([socketId, collaborator]) => + ([clientId, collaborator]) => renderCollaborator({ actionManager, collaborator, - socketId, + clientId, shouldWrapWithTooltip: true, - isBeingFollowed: socketId === userToFollow, + isBeingFollowed: collaborator.socketId === userToFollow, }), ); return mobile ? (
- {uniqueCollaboratorsArray.map(([socketId, collaborator]) => + {uniqueCollaboratorsArray.map(([clientId, collaborator]) => renderCollaborator({ actionManager, collaborator, - socketId, + clientId, shouldWrapWithTooltip: true, - isBeingFollowed: socketId === userToFollow, + isBeingFollowed: collaborator.socketId === userToFollow, }), )}
@@ -205,13 +209,13 @@ export const UserList = React.memo(
{t("userList.hint.text")}
- {filteredCollaborators.map(([socketId, collaborator]) => + {filteredCollaborators.map(([clientId, collaborator]) => renderCollaborator({ actionManager, collaborator, - socketId, + clientId, withName: true, - isBeingFollowed: socketId === userToFollow, + isBeingFollowed: collaborator.socketId === userToFollow, }), )} diff --git a/packages/excalidraw/types.ts b/packages/excalidraw/types.ts index 249ead30..854a2751 100644 --- a/packages/excalidraw/types.ts +++ b/packages/excalidraw/types.ts @@ -41,7 +41,7 @@ import { Merge, ValueOf } from "./utility-types"; export type Point = Readonly; -export type SocketId = string; +export type SocketId = string & { _brand: "SocketId" }; export type Collaborator = Readonly<{ pointer?: CollaboratorPointer; @@ -128,7 +128,7 @@ export type SidebarName = string; export type SidebarTabName = string; export type UserToFollow = { - socketId: string; + socketId: SocketId; username: string; }; @@ -296,7 +296,7 @@ export interface AppState { offsetLeft: number; fileHandle: FileSystemHandle | null; - collaborators: Map; + collaborators: Map; showStats: boolean; currentChartType: ChartType; pasteDialog: @@ -321,7 +321,7 @@ export interface AppState { /** the user's clientId & username who is being followed on the canvas */ userToFollow: UserToFollow | null; /** the clientIds of the users following the current user */ - followedBy: Set; + followedBy: Set; } export type UIAppState = Omit< @@ -474,7 +474,7 @@ export interface ExcalidrawProps { export type SceneData = { elements?: ImportedDataState["elements"]; appState?: ImportedDataState["appState"]; - collaborators?: Map; + collaborators?: Map; commitToHistory?: boolean; };