From 3317bf23969e73895c9fd06ac59db7980a0aa97a Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 22 May 2023 21:05:40 -0400 Subject: [PATCH] fix: User presence is not updated when leaving a document --- app/components/WebsocketProvider.tsx | 35 --------- .../Document/components/MultiplayerEditor.tsx | 13 ++-- app/stores/DocumentPresenceStore.ts | 72 ++++++++++--------- app/types.ts | 4 ++ shared/constants.ts | 2 - 5 files changed, 47 insertions(+), 79 deletions(-) diff --git a/app/components/WebsocketProvider.tsx b/app/components/WebsocketProvider.tsx index e9280b8e70..3f56f38a49 100644 --- a/app/components/WebsocketProvider.tsx +++ b/app/components/WebsocketProvider.tsx @@ -85,9 +85,7 @@ class WebsocketProvider extends React.Component { stars, memberships, policies, - presence, comments, - views, subscriptions, fileOperations, notifications, @@ -105,12 +103,6 @@ class WebsocketProvider extends React.Component { }); }); - this.socket.on("disconnect", () => { - // when the socket is disconnected we need to clear all presence state as - // it's no longer reliable. - presence.clear(); - }); - // on reconnection, reset the transports option, as the Websocket // connection may have failed (caused by proxy, firewall, browser, ...) this.socket.io.on("reconnect_attempt", () => { @@ -468,33 +460,6 @@ class WebsocketProvider extends React.Component { this.socket.on("leave", (event: any) => { this.socket?.emit("leave", event); }); - - // received whenever we join a document room, the payload includes - // userIds that are present/viewing and those that are editing. - this.socket.on("document.presence", (event: any) => { - presence.init(event.documentId, event.userIds, event.editingIds); - }); - - // received whenever a new user joins a document room, aka they - // navigate to / start viewing a document - this.socket.on("user.join", (event: any) => { - presence.touch(event.documentId, event.userId, event.isEditing); - views.touch(event.documentId, event.userId); - }); - - // received whenever a new user leaves a document room, aka they - // navigate away / stop viewing a document - this.socket.on("user.leave", (event: any) => { - presence.leave(event.documentId, event.userId); - views.touch(event.documentId, event.userId); - }); - - // received when another client in a document room wants to change - // or update it's presence. Currently the only property is whether - // the client is in editing state or not. - this.socket.on("user.presence", (event: any) => { - presence.touch(event.documentId, event.userId, event.isEditing); - }); }; render() { diff --git a/app/scenes/Document/components/MultiplayerEditor.tsx b/app/scenes/Document/components/MultiplayerEditor.tsx index 621eac9f9b..abbabad22c 100644 --- a/app/scenes/Document/components/MultiplayerEditor.tsx +++ b/app/scenes/Document/components/MultiplayerEditor.tsx @@ -15,6 +15,7 @@ import useIsMounted from "~/hooks/useIsMounted"; import usePageVisibility from "~/hooks/usePageVisibility"; import useStores from "~/hooks/useStores"; import useToasts from "~/hooks/useToasts"; +import { AwarenessChangeEvent } from "~/types"; import Logger from "~/utils/Logger"; import { supportsPassiveListener } from "~/utils/browser"; import { homePath } from "~/utils/routeHelpers"; @@ -30,10 +31,6 @@ export type ConnectionStatus = | "disconnected" | void; -type AwarenessChangeEvent = { - states: { user: { id: string }; cursor: any; scrollY: number | undefined }[]; -}; - type ConnectionStatusEvent = { status: ConnectionStatus }; type MessageEvent = { @@ -108,11 +105,11 @@ function MultiplayerEditor({ onSynced, ...props }: Props, ref: any) { history.replace(homePath()); }); - provider.on("awarenessChange", ({ states }: AwarenessChangeEvent) => { - states.forEach(({ user, cursor, scrollY }) => { - if (user) { - presence.touch(documentId, user.id, !!cursor); + provider.on("awarenessChange", (event: AwarenessChangeEvent) => { + presence.updateFromAwarenessChangeEvent(documentId, event); + event.states.forEach(({ user, scrollY }) => { + if (user) { if (scrollY !== undefined && user.id === ui.observingUserId) { window.scrollTo({ top: scrollY * window.innerHeight, diff --git a/app/stores/DocumentPresenceStore.ts b/app/stores/DocumentPresenceStore.ts index acd105b600..8e18e32f79 100644 --- a/app/stores/DocumentPresenceStore.ts +++ b/app/stores/DocumentPresenceStore.ts @@ -1,5 +1,5 @@ import { observable, action } from "mobx"; -import { USER_PRESENCE_INTERVAL } from "@shared/constants"; +import { AwarenessChangeEvent } from "~/types"; type DocumentPresence = Map< string, @@ -15,19 +15,11 @@ export default class PresenceStore { timeouts: Map> = new Map(); - // called to setup when we get the initial state from document.presence - // websocket message. overrides any existing state - @action - init(documentId: string, userIds: string[], editingIds: string[]) { - this.data.set(documentId, new Map()); - userIds.forEach((userId) => - this.touch(documentId, userId, editingIds.includes(userId)) - ); - } + offlineTimeout = 30000; - // called when a user leave the room – user.leave websocket message. + // called when a user leaves the document @action - leave(documentId: string, userId: string) { + public leave(documentId: string, userId: string) { const existing = this.data.get(documentId); if (existing) { @@ -35,23 +27,27 @@ export default class PresenceStore { } } - @action - update(documentId: string, userId: string, isEditing: boolean) { - const existing = this.data.get(documentId) || new Map(); - existing.set(userId, { - isEditing, - userId, + public updateFromAwarenessChangeEvent( + documentId: string, + event: AwarenessChangeEvent + ) { + const presence = this.data.get(documentId); + let existingUserIds = (presence ? Array.from(presence.values()) : []).map( + (p) => p.userId + ); + + event.states.forEach((state) => { + const { user, cursor } = state; + this.update(documentId, user.id, !!cursor); + existingUserIds = existingUserIds.filter((id) => id !== user.id); + }); + + existingUserIds.forEach((userId) => { + this.leave(documentId, userId); }); - this.data.set(documentId, existing); } - // called when a user presence message is received – user.presence websocket - // message. - // While in edit mode a message is sent every USER_PRESENCE_INTERVAL, if - // the other clients don't receive within USER_PRESENCE_INTERVAL*2 then a - // timeout is triggered causing the users presence to default back to not - // editing state as a safety measure. - touch(documentId: string, userId: string, isEditing: boolean) { + public touch(documentId: string, userId: string, isEditing: boolean) { const id = `${documentId}-${userId}`; let timeout = this.timeouts.get(id); @@ -62,20 +58,28 @@ export default class PresenceStore { this.update(documentId, userId, isEditing); - if (isEditing) { - timeout = setTimeout(() => { - this.update(documentId, userId, false); - }, USER_PRESENCE_INTERVAL * 2); - this.timeouts.set(id, timeout); - } + timeout = setTimeout(() => { + this.leave(documentId, userId); + }, this.offlineTimeout); + this.timeouts.set(id, timeout); } - get(documentId: string): DocumentPresence | null | undefined { + @action + private update(documentId: string, userId: string, isEditing: boolean) { + const existing = this.data.get(documentId) || new Map(); + existing.set(userId, { + isEditing, + userId, + }); + this.data.set(documentId, existing); + } + + public get(documentId: string): DocumentPresence | null | undefined { return this.data.get(documentId); } @action - clear() { + public clear() { this.data.clear(); } } diff --git a/app/types.ts b/app/types.ts index 7d4c489da9..b134a84712 100644 --- a/app/types.ts +++ b/app/types.ts @@ -202,3 +202,7 @@ export type WebsocketEvent = | WebsocketCollectionUpdateIndexEvent | WebsocketEntityDeletedEvent | WebsocketEntitiesEvent; + +export type AwarenessChangeEvent = { + states: { user: { id: string }; cursor: any; scrollY: number | undefined }[]; +}; diff --git a/shared/constants.ts b/shared/constants.ts index 89b73e8fd3..3c4b729094 100644 --- a/shared/constants.ts +++ b/shared/constants.ts @@ -5,8 +5,6 @@ import { UserPreferences, } from "./types"; -export const USER_PRESENCE_INTERVAL = 5000; - export const MAX_AVATAR_DISPLAY = 6; export const TeamPreferenceDefaults: TeamPreferences = {