Skip to content

Commit 9ffb668

Browse files
committed
Fix presence indicator showing wrong user on network disconnect
Root cause: When a user's network goes offline, the disconnect event would try to call setAwarenessField, but this message can't be delivered since the connection is already lost. Additionally, when users disappeared from the awareness states array, they were completely removed from the UI rather than being marked as disconnected. Changes: - Remove setAwarenessField call from disconnect event handler (already offline) - Preserve users who disappear from awareness states as disconnected - Merge previous presence state with current awareness data - Users now show correct disconnected status when they go offline This ensures network disconnect and browser close produce identical behavior: both result in the correct user showing as disconnected (grey indicator).
1 parent 47fbb07 commit 9ffb668

File tree

2 files changed

+132
-18
lines changed

2 files changed

+132
-18
lines changed

__tests__/components/ui/coaching-sessions/editor-cache-context.test.tsx

Lines changed: 93 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ describe('EditorCacheProvider', () => {
410410
})
411411
})
412412

413-
it('should set disconnected status on disconnect event', async () => {
413+
it('should NOT call setAwarenessField on disconnect event (already offline)', async () => {
414414
const { TiptapCollabProvider } = await import('@hocuspocus/provider')
415415
let disconnectCallback: (() => void) | undefined
416416

@@ -449,13 +449,99 @@ describe('EditorCacheProvider', () => {
449449
disconnectCallback!()
450450
})
451451

452-
// Should update awareness with disconnected status
453-
expect(mockProvider?.setAwarenessField).toHaveBeenCalledWith(
454-
'presence',
455-
expect.objectContaining({
456-
status: 'disconnected'
457-
})
452+
// Should NOT call setAwarenessField because we're already disconnected
453+
// The awareness protocol will handle removing stale clients via timeout
454+
expect(mockProvider?.setAwarenessField).not.toHaveBeenCalled()
455+
})
456+
457+
it('should mark users as disconnected when they disappear from awareness states', async () => {
458+
const { TiptapCollabProvider } = await import('@hocuspocus/provider')
459+
let awarenessCallback: ((data: any) => void) | undefined
460+
461+
vi.mocked(TiptapCollabProvider).mockImplementationOnce(function() {
462+
const provider = {
463+
on: vi.fn((event, callback) => {
464+
if (event === 'awarenessChange') {
465+
awarenessCallback = callback
466+
}
467+
return provider
468+
}),
469+
off: vi.fn(),
470+
setAwarenessField: vi.fn(),
471+
destroy: vi.fn(),
472+
disconnect: vi.fn(),
473+
connect: vi.fn()
474+
}
475+
return provider as any
476+
})
477+
478+
let cacheRef: any = null
479+
480+
render(
481+
<EditorCacheProvider sessionId="test-session">
482+
<TestConsumer onCacheReady={(cache) => { cacheRef = cache }} />
483+
</EditorCacheProvider>
458484
)
485+
486+
await waitFor(() => {
487+
expect(awarenessCallback).toBeDefined()
488+
})
489+
490+
// First, simulate both users being connected
491+
act(() => {
492+
awarenessCallback!({
493+
states: [
494+
{
495+
clientId: 1,
496+
presence: {
497+
userId: 'user-1',
498+
name: 'Test User',
499+
relationshipRole: 'Coach',
500+
color: '#ff0000',
501+
isConnected: true
502+
}
503+
},
504+
{
505+
clientId: 2,
506+
presence: {
507+
userId: 'user-2',
508+
name: 'Other User',
509+
relationshipRole: 'Coachee',
510+
color: '#00ff00',
511+
isConnected: true
512+
}
513+
}
514+
]
515+
})
516+
})
517+
518+
// Verify both users are connected
519+
expect(cacheRef?.presenceState.users.size).toBe(2)
520+
expect(cacheRef?.presenceState.users.get('user-2')?.status).toBe('connected')
521+
522+
// Now simulate user-2 disappearing (network disconnect)
523+
act(() => {
524+
awarenessCallback!({
525+
states: [
526+
{
527+
clientId: 1,
528+
presence: {
529+
userId: 'user-1',
530+
name: 'Test User',
531+
relationshipRole: 'Coach',
532+
color: '#ff0000',
533+
isConnected: true
534+
}
535+
}
536+
// user-2 is no longer in the states array
537+
]
538+
})
539+
})
540+
541+
// User-2 should still be in the map but marked as disconnected
542+
expect(cacheRef?.presenceState.users.size).toBe(2)
543+
expect(cacheRef?.presenceState.users.get('user-2')?.status).toBe('disconnected')
544+
expect(cacheRef?.presenceState.users.get('user-1')?.status).toBe('connected')
459545
})
460546
})
461547
})

src/components/ui/coaching-sessions/editor-cache-context.tsx

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -190,15 +190,41 @@ export const EditorCacheProvider: React.FC<EditorCacheProviderProps> = ({
190190
}
191191
});
192192

193-
setCache((prev) => ({
194-
...prev,
195-
presenceState: {
196-
...prev.presenceState,
197-
users: updatedUsers,
198-
currentUser:
199-
currentUserPresence || prev.presenceState.currentUser,
200-
},
201-
}));
193+
// IMPORTANT: Preserve previous users who are no longer in states array
194+
// as disconnected instead of removing them entirely.
195+
// This ensures smooth UX when users go offline (they appear as disconnected
196+
// rather than disappearing completely).
197+
setCache((prev) => {
198+
const mergedUsers = new Map(prev.presenceState.users);
199+
200+
// Mark users who disappeared from awareness as disconnected
201+
for (const [userId, oldPresence] of prev.presenceState.users) {
202+
if (!updatedUsers.has(userId) && oldPresence.status === 'connected') {
203+
// User was connected but no longer in awareness states - mark as disconnected
204+
mergedUsers.set(userId, {
205+
...oldPresence,
206+
status: 'disconnected',
207+
isConnected: false,
208+
lastSeen: new Date()
209+
});
210+
}
211+
}
212+
213+
// Overlay current awareness data (takes precedence)
214+
for (const [userId, presence] of updatedUsers) {
215+
mergedUsers.set(userId, presence);
216+
}
217+
218+
return {
219+
...prev,
220+
presenceState: {
221+
...prev.presenceState,
222+
users: mergedUsers,
223+
currentUser:
224+
currentUserPresence || prev.presenceState.currentUser,
225+
},
226+
};
227+
});
202228
}
203229
);
204230

@@ -216,8 +242,10 @@ export const EditorCacheProvider: React.FC<EditorCacheProviderProps> = ({
216242
});
217243

218244
provider.on("disconnect", () => {
219-
const disconnectedPresence = createDisconnectedPresence(userPresence);
220-
provider.setAwarenessField("presence", disconnectedPresence);
245+
// NOTE: Don't call setAwarenessField here - we're already disconnected
246+
// so the message won't be delivered to other clients anyway.
247+
// The awareness protocol will automatically remove our state via timeout.
248+
// This event is just for local cleanup/logging if needed.
221249
});
222250

223251
// Graceful disconnect on page unload

0 commit comments

Comments
 (0)