diff --git a/__tests__/components/ui/coaching-sessions/editor-cache-context.test.tsx b/__tests__/components/ui/coaching-sessions/editor-cache-context.test.tsx index 4d26830..2de4168 100644 --- a/__tests__/components/ui/coaching-sessions/editor-cache-context.test.tsx +++ b/__tests__/components/ui/coaching-sessions/editor-cache-context.test.tsx @@ -169,7 +169,7 @@ describe('EditorCacheProvider', () => { // THE CRITICAL TEST: Logout cleanup it('should destroy TipTap provider when user logs out', async () => { let cacheRef: any = null - + // Start with logged in user render( @@ -198,4 +198,350 @@ describe('EditorCacheProvider', () => { // After reset, the cache should be in loading state (not ready) expect(screen.getByTestId('is-ready')).toHaveTextContent('no') }) + + describe('Provider Lifecycle - Preventing Unnecessary Disconnections', () => { + it('should NOT disconnect provider when re-rendering with same sessionId', async () => { + const { TiptapCollabProvider } = await import('@hocuspocus/provider') + + const { rerender } = render( + + + + ) + + await waitFor(() => { + expect(screen.getByTestId('is-ready')).toHaveTextContent('yes') + }) + + // Get the mock instance and clear previous calls + const mockProvider = vi.mocked(TiptapCollabProvider).mock.results[0]?.value + vi.clearAllMocks() + + // Re-render the same component (simulates user clicking in editor) + rerender( + + + + ) + + // Wait a bit to ensure no cleanup happens + await new Promise(resolve => setTimeout(resolve, 50)) + + // CRITICAL: Provider should NOT be disconnected + expect(mockProvider?.disconnect).not.toHaveBeenCalled() + + // CRITICAL: Provider should NOT be recreated + expect(TiptapCollabProvider).not.toHaveBeenCalled() + }) + + it('should disconnect old provider when sessionId changes', async () => { + const { TiptapCollabProvider } = await import('@hocuspocus/provider') + + const { rerender } = render( + + + + ) + + await waitFor(() => { + expect(screen.getByTestId('is-ready')).toHaveTextContent('yes') + }) + + const oldProvider = vi.mocked(TiptapCollabProvider).mock.results[0]?.value + + // Change session ID + rerender( + + + + ) + + await waitFor(() => { + // Old provider should be disconnected + expect(oldProvider?.disconnect).toHaveBeenCalled() + }) + + // New provider should be created + expect(TiptapCollabProvider).toHaveBeenCalledTimes(2) + }) + }) + + describe('Extension Creation', () => { + it('should create extensions only once even if synced event fires multiple times', async () => { + const { Extensions } = await import('@/components/ui/coaching-sessions/coaching-notes/extensions') + + // Create a mock provider that we can control + const { TiptapCollabProvider } = await import('@hocuspocus/provider') + let syncedCallback: (() => void) | undefined + + vi.mocked(TiptapCollabProvider).mockImplementationOnce(function() { + const provider = { + on: vi.fn((event, callback) => { + if (event === 'synced') { + syncedCallback = callback + } + return provider + }), + off: vi.fn(), + setAwarenessField: vi.fn(), + destroy: vi.fn(), + disconnect: vi.fn(), + connect: vi.fn() + } + return provider as any + }) + + render( + + + + ) + + // Wait for provider initialization + await waitFor(() => { + expect(syncedCallback).toBeDefined() + }) + + // Trigger synced event multiple times + act(() => { + syncedCallback!() + syncedCallback!() + syncedCallback!() + }) + + // Extensions should only be created once + expect(Extensions).toHaveBeenCalledTimes(1) + }) + }) + + describe('Awareness State Management', () => { + it('should use setAwarenessField for setting user presence', async () => { + const { TiptapCollabProvider } = await import('@hocuspocus/provider') + + render( + + + + ) + + await waitFor(() => { + expect(screen.getByTestId('is-ready')).toHaveTextContent('yes') + }) + + const mockProvider = vi.mocked(TiptapCollabProvider).mock.results[0]?.value + + // Should have called setAwarenessField with presence data + expect(mockProvider?.setAwarenessField).toHaveBeenCalledWith( + 'presence', + expect.objectContaining({ + userId: 'user-1', + name: 'Test User', + status: 'connected' + }) + ) + }) + + it('should update presence state when awarenessChange event fires', async () => { + const { TiptapCollabProvider } = await import('@hocuspocus/provider') + let awarenessCallback: ((data: any) => void) | undefined + + vi.mocked(TiptapCollabProvider).mockImplementationOnce(function() { + const provider = { + on: vi.fn((event, callback) => { + if (event === 'awarenessChange') { + awarenessCallback = callback + } + return provider + }), + off: vi.fn(), + setAwarenessField: vi.fn(), + destroy: vi.fn(), + disconnect: vi.fn(), + connect: vi.fn() + } + return provider as any + }) + + let cacheRef: any = null + + render( + + { cacheRef = cache }} /> + + ) + + await waitFor(() => { + expect(awarenessCallback).toBeDefined() + }) + + // Simulate awareness change with user data + act(() => { + awarenessCallback!({ + states: [ + { + clientId: 1, + presence: { + userId: 'user-1', + name: 'Test User', + relationshipRole: 'Coach', + color: '#ff0000', + status: 'connected' + } + }, + { + clientId: 2, + presence: { + userId: 'user-2', + name: 'Other User', + relationshipRole: 'Coachee', + color: '#00ff00', + status: 'connected' + } + } + ] + }) + }) + + // Presence state should be updated + expect(cacheRef?.presenceState.users.size).toBe(2) + expect(cacheRef?.presenceState.users.get('user-1')).toMatchObject({ + userId: 'user-1', + name: 'Test User' + }) + }) + + it('should NOT call setAwarenessField on disconnect event (already offline)', async () => { + const { TiptapCollabProvider } = await import('@hocuspocus/provider') + let disconnectCallback: (() => void) | undefined + + vi.mocked(TiptapCollabProvider).mockImplementationOnce(function() { + const provider = { + on: vi.fn((event, callback) => { + if (event === 'disconnect') { + disconnectCallback = callback + } + return provider + }), + off: vi.fn(), + setAwarenessField: vi.fn(), + destroy: vi.fn(), + disconnect: vi.fn(), + connect: vi.fn() + } + return provider as any + }) + + render( + + + + ) + + await waitFor(() => { + expect(disconnectCallback).toBeDefined() + }) + + const mockProvider = vi.mocked(TiptapCollabProvider).mock.results[0]?.value + vi.clearAllMocks() + + // Trigger disconnect event + act(() => { + disconnectCallback!() + }) + + // Should NOT call setAwarenessField because we're already disconnected + // The awareness protocol will handle removing stale clients via timeout + expect(mockProvider?.setAwarenessField).not.toHaveBeenCalled() + }) + + it('should mark users as disconnected when they disappear from awareness states', async () => { + const { TiptapCollabProvider } = await import('@hocuspocus/provider') + let awarenessCallback: ((data: any) => void) | undefined + + vi.mocked(TiptapCollabProvider).mockImplementationOnce(function() { + const provider = { + on: vi.fn((event, callback) => { + if (event === 'awarenessChange') { + awarenessCallback = callback + } + return provider + }), + off: vi.fn(), + setAwarenessField: vi.fn(), + destroy: vi.fn(), + disconnect: vi.fn(), + connect: vi.fn() + } + return provider as any + }) + + let cacheRef: any = null + + render( + + { cacheRef = cache }} /> + + ) + + await waitFor(() => { + expect(awarenessCallback).toBeDefined() + }) + + // First, simulate both users being connected + act(() => { + awarenessCallback!({ + states: [ + { + clientId: 1, + presence: { + userId: 'user-1', + name: 'Test User', + relationshipRole: 'Coach', + color: '#ff0000', + isConnected: true + } + }, + { + clientId: 2, + presence: { + userId: 'user-2', + name: 'Other User', + relationshipRole: 'Coachee', + color: '#00ff00', + isConnected: true + } + } + ] + }) + }) + + // Verify both users are connected + expect(cacheRef?.presenceState.users.size).toBe(2) + expect(cacheRef?.presenceState.users.get('user-2')?.status).toBe('connected') + + // Now simulate user-2 disappearing (network disconnect) + act(() => { + awarenessCallback!({ + states: [ + { + clientId: 1, + presence: { + userId: 'user-1', + name: 'Test User', + relationshipRole: 'Coach', + color: '#ff0000', + isConnected: true + } + } + // user-2 is no longer in the states array + ] + }) + }) + + // User-2 should still be in the map but marked as disconnected + expect(cacheRef?.presenceState.users.size).toBe(2) + expect(cacheRef?.presenceState.users.get('user-2')?.status).toBe('disconnected') + expect(cacheRef?.presenceState.users.get('user-1')?.status).toBe('connected') + }) + }) }) \ No newline at end of file diff --git a/src/components/ui/coaching-sessions/editor-cache-context.tsx b/src/components/ui/coaching-sessions/editor-cache-context.tsx index 610ce5f..4bee35f 100644 --- a/src/components/ui/coaching-sessions/editor-cache-context.tsx +++ b/src/components/ui/coaching-sessions/editor-cache-context.tsx @@ -27,6 +27,7 @@ import { } from "@/types/presence"; import { useCurrentRelationshipRole } from "@/lib/hooks/use-current-relationship-role"; import { logoutCleanupRegistry } from "@/lib/hooks/logout-cleanup-registry"; +import { generateCollaborativeUserColor } from "@/lib/tiptap-utils"; /** * EditorCacheProvider manages TipTap collaboration lifecycle: @@ -86,6 +87,9 @@ export const EditorCacheProvider: React.FC = ({ const yDocRef = useRef(null); const lastSessionIdRef = useRef(null); + // Generate a consistent color for this user session + const userColor = useMemo(() => generateCollaborativeUserColor(), []); + const [cache, setCache] = useState({ yDoc: null, collaborationProvider: null, @@ -126,11 +130,33 @@ export const EditorCacheProvider: React.FC = ({ user: userSession.display_name, }); + // Awareness initialization: establishes user presence in collaborative session + // IMPORTANT: Set awareness BEFORE synced event so CollaborationCaret has user data + const userPresence = createConnectedPresence({ + userId: userSession.id, + name: userSession.display_name, + relationshipRole: userRole, + color: userColor, + }); + + // IMPORTANT: Only set our custom "presence" field + // Let CollaborationCaret manage the "user" field to avoid conflicts + provider.setAwarenessField("presence", userPresence); + // Provider event handlers: sync completion enables collaborative editing + // IMPORTANT: Track if we've already created extensions to prevent recreation + let extensionsCreated = false; + provider.on("synced", () => { + if (extensionsCreated) { + return; + } + + extensionsCreated = true; + const collaborativeExtensions = createExtensions(doc, provider, { name: userSession.display_name, - color: "#ffcc00", + color: userColor, }); setCache((prev) => ({ @@ -144,27 +170,12 @@ export const EditorCacheProvider: React.FC = ({ })); }); - // Awareness initialization: establishes user presence in collaborative session - const userPresence = createConnectedPresence({ - userId: userSession.id, - name: userSession.display_name, - relationshipRole: userRole, - color: "#ffcc00", - }); - - provider.setAwarenessField("user", { - name: userSession.display_name, - color: "#ffcc00", - }); - - provider.setAwarenessField("presence", userPresence); - providerRef.current = provider; // Awareness synchronization: tracks all connected users for presence indicators provider.on( "awarenessChange", - ({ states }: { states: Map }) => { + ({ states }: { states: Array<{ clientId: number; [key: string]: any }> }) => { const updatedUsers = new Map(); let currentUserPresence: UserPresence | null = null; @@ -179,15 +190,46 @@ export const EditorCacheProvider: React.FC = ({ } }); - setCache((prev) => ({ - ...prev, - presenceState: { - ...prev.presenceState, - users: updatedUsers, - currentUser: - currentUserPresence || prev.presenceState.currentUser, - }, - })); + // IMPORTANT: Preserve previous users who are no longer in states array + // as disconnected instead of removing them entirely. + // This ensures smooth UX when users go offline (they appear as disconnected + // rather than disappearing completely). + setCache((prev) => { + const mergedUsers = new Map(prev.presenceState.users); + + // Mark users who disappeared from awareness as disconnected + // When users "disappear," it means they're no longer in the awareness states array - + // typically due to network disconnect, browser crash, or navigation away from the coaching + // session page. Without this code, disconnected users would instantly vanish from the UI, + // creating an unwanted UX. This preserves them as status: 'disconnected' instead, enabling + // smooth UX transitions (like showing grayed-out presence indicators). + for (const [userId, oldPresence] of prev.presenceState.users) { + if (!updatedUsers.has(userId) && oldPresence.status === 'connected') { + // User was connected but no longer in awareness states - mark as disconnected + mergedUsers.set(userId, { + ...oldPresence, + status: 'disconnected', + isConnected: false, + lastSeen: new Date() + }); + } + } + + // Overlay current awareness data (takes precedence) + for (const [userId, presence] of updatedUsers) { + mergedUsers.set(userId, presence); + } + + return { + ...prev, + presenceState: { + ...prev.presenceState, + users: mergedUsers, + currentUser: + currentUserPresence || prev.presenceState.currentUser, + }, + }; + }); } ); @@ -197,34 +239,20 @@ export const EditorCacheProvider: React.FC = ({ userId: userSession.id, name: userSession.display_name, relationshipRole: userRole, - color: "#ffcc00", + color: userColor, }); + // Only update our custom "presence" field on reconnect + // CollaborationCaret will handle the "user" field provider.setAwarenessField("presence", connectedPresence); - provider.setAwarenessField("user", { - name: userSession.display_name, - color: "#ffcc00", - }); }); provider.on("disconnect", () => { - const disconnectedPresence = createDisconnectedPresence(userPresence); - provider.setAwarenessField("presence", disconnectedPresence); + // NOTE: Don't call setAwarenessField here - we're already disconnected + // so the message won't be delivered to other clients anyway. + // The awareness protocol will automatically remove our state via timeout. + // This event is just for local cleanup/logging if needed. }); - // Mouse tracking: enables collaborative cursor positioning - const handleMouseMove = (event: MouseEvent) => { - if (providerRef.current) { - providerRef.current.setAwarenessField("user", { - name: userSession.display_name, - color: "#ffcc00", - mouseX: event.clientX, - mouseY: event.clientY, - }); - } - }; - - document.addEventListener("mousemove", handleMouseMove); - // Graceful disconnect on page unload const handleBeforeUnload = () => { const disconnectedPresence = createDisconnectedPresence(userPresence); @@ -234,7 +262,6 @@ export const EditorCacheProvider: React.FC = ({ window.addEventListener("beforeunload", handleBeforeUnload); return () => { - document.removeEventListener("mousemove", handleMouseMove); window.removeEventListener("beforeunload", handleBeforeUnload); }; } catch (error) { @@ -274,6 +301,11 @@ export const EditorCacheProvider: React.FC = ({ // Provider initialization or fallback to offline mode if (jwt && !tokenError && userSession) { + // Skip if provider already exists and session hasn't changed + if (providerRef.current && lastSessionIdRef.current === sessionId) { + return; + } + initializeProvider(); } else { // Fallback to offline mode when token is unavailable @@ -294,8 +326,11 @@ export const EditorCacheProvider: React.FC = ({ } return () => { - if (providerRef.current) { + // IMPORTANT: Only disconnect on unmount or session change + // Don't disconnect if dependencies change but provider should stay + if (providerRef.current && lastSessionIdRef.current !== sessionId) { providerRef.current.disconnect(); + providerRef.current = null; } }; }, [ @@ -316,9 +351,9 @@ export const EditorCacheProvider: React.FC = ({ if (provider) { try { - // Clear user presence to signal departure + // Clear our custom presence field on logout + // CollaborationCaret will clean up the "user" field provider.setAwarenessField("presence", null); - provider.setAwarenessField("user", null); // Graceful provider shutdown provider.disconnect(); diff --git a/src/styles/simple-editor.scss b/src/styles/simple-editor.scss index 88d6dbf..8d3d5c8 100644 --- a/src/styles/simple-editor.scss +++ b/src/styles/simple-editor.scss @@ -395,8 +395,10 @@ position: relative; word-break: normal; animation: blink 1.5s infinite; - height: 1.2em; - min-height: 16px; + /* Use line-height to match the actual text height where cursor appears */ + height: 1em; + line-height: inherit; + vertical-align: baseline; display: inline-block; } @@ -430,11 +432,11 @@ /* Smooth transitions */ transition: all 0.2s ease; transform: translateY(-2px); - + /* CRITICAL: Prevent any animations or opacity changes */ animation: none !important; opacity: 1 !important; - + /* Pointer tail */ &::after { content: ''; @@ -446,13 +448,21 @@ border-left: 6px solid transparent; border-right: 6px solid transparent; border-top: 6px solid var(--collaboration-user-color, #6366f1); + transition: opacity 0.2s ease; } - - /* Hover effect for better visibility */ + + /* Hover effect - make highly transparent with enhanced shadow */ &:hover { + opacity: 0.2 !important; + cursor: pointer; transform: translateY(-4px); box-shadow: 0 10px 15px -3px rgba(0, 0, 0, 0.1), 0 4px 6px -2px rgba(0, 0, 0, 0.05); } + + /* Make tail transparent on hover too */ + &:hover::after { + opacity: 0.2; + } /* Dark theme adjustments */ .dark & { @@ -488,7 +498,7 @@ .ProseMirror .collaboration-cursor__label { z-index: 100; - pointer-events: none; + pointer-events: auto; /* Allow hover interactions */ } /* Mobile responsiveness for collaboration carets */