From 43b4845baa1761d40d2c7fd22372f546e2e80e94 Mon Sep 17 00:00:00 2001 From: Farhan Yahaya Date: Mon, 10 Nov 2025 08:14:40 +0000 Subject: [PATCH 1/4] fix: inactive collaborators should still ping their last seen --- .../components/ActiveCollaborators.tsx | 18 +- .../stores/createAwarenessStore.ts | 170 +++++++++++++----- 2 files changed, 133 insertions(+), 55 deletions(-) diff --git a/assets/js/collaborative-editor/components/ActiveCollaborators.tsx b/assets/js/collaborative-editor/components/ActiveCollaborators.tsx index 43e2bf3b6d..e1353428e6 100644 --- a/assets/js/collaborative-editor/components/ActiveCollaborators.tsx +++ b/assets/js/collaborative-editor/components/ActiveCollaborators.tsx @@ -1,8 +1,8 @@ -import { cn } from "../../utils/cn"; -import { useRemoteUsers } from "../hooks/useAwareness"; -import { getAvatarInitials } from "../utils/avatar"; +import { cn } from '../../utils/cn'; +import { useRemoteUsers } from '../hooks/useAwareness'; +import { getAvatarInitials } from '../utils/avatar'; -import { Tooltip } from "./Tooltip"; +import { Tooltip } from './Tooltip'; function lessthanmin(val: number, mins: number) { const now = Date.now(); @@ -22,12 +22,12 @@ export function ActiveCollaborators({ className }: ActiveCollaboratorsProps) { } return ( -
+
{remoteUsers.map(user => { const nameParts = user.user.name.split(/\s+/); - const firstName = nameParts[0] || ""; + const firstName = nameParts[0] || ''; const lastName = - nameParts.length > 1 ? nameParts[nameParts.length - 1] : ""; + nameParts.length > 1 ? nameParts[nameParts.length - 1] : ''; const userForInitials = { first_name: firstName, @@ -44,14 +44,14 @@ export function ActiveCollaborators({ className }: ActiveCollaboratorsProps) { return (
{initials} diff --git a/assets/js/collaborative-editor/stores/createAwarenessStore.ts b/assets/js/collaborative-editor/stores/createAwarenessStore.ts index e5302c637a..1ae07936b8 100644 --- a/assets/js/collaborative-editor/stores/createAwarenessStore.ts +++ b/assets/js/collaborative-editor/stores/createAwarenessStore.ts @@ -90,22 +90,22 @@ * rawAwareness (too large/circular) */ -import { produce } from "immer"; -import type { Awareness } from "y-protocols/awareness"; +import { produce } from 'immer'; +import type { Awareness } from 'y-protocols/awareness'; -import _logger from "#/utils/logger"; +import _logger from '#/utils/logger'; import type { AwarenessState, AwarenessStore, AwarenessUser, LocalUserData, -} from "../types/awareness"; +} from '../types/awareness'; -import { createWithSelector } from "./common"; -import { wrapStoreWithDevTools } from "./devtools"; +import { createWithSelector } from './common'; +import { wrapStoreWithDevTools } from './devtools'; -const logger = _logger.ns("AwarenessStore").seal(); +const logger = _logger.ns('AwarenessStore').seal(); /** * Creates an awareness store instance with useSyncExternalStore + Immer pattern @@ -131,12 +131,12 @@ export const createAwarenessStore = (): AwarenessStore => { // Redux DevTools integration const devtools = wrapStoreWithDevTools({ - name: "AwarenessStore", - excludeKeys: ["rawAwareness"], // Exclude Y.js Awareness object + name: 'AwarenessStore', + excludeKeys: ['rawAwareness'], // Exclude Y.js Awareness object maxAge: 200, // Higher limit since awareness changes are frequent }); - const notify = (actionName: string = "stateChange") => { + const notify = (actionName: string = 'stateChange') => { devtools.notifyWithAction(actionName, () => state); listeners.forEach(listener => { listener(); @@ -169,22 +169,22 @@ export const createAwarenessStore = (): AwarenessStore => { awareness.getStates().forEach((awarenessState, clientId) => { // Validate user data structure - if (awarenessState["user"]) { + if (awarenessState['user']) { try { // Note: We're not using Zod validation here as it's runtime performance critical // and we trust the awareness protocol more than external API data const user: AwarenessUser = { clientId, - user: awarenessState["user"] as AwarenessUser["user"], - cursor: awarenessState["cursor"] as AwarenessUser["cursor"], + user: awarenessState['user'] as AwarenessUser['user'], + cursor: awarenessState['cursor'] as AwarenessUser['cursor'], selection: awarenessState[ - "selection" - ] as AwarenessUser["selection"], - lastSeen: awarenessState["lastSeen"] as number | undefined, + 'selection' + ] as AwarenessUser['selection'], + lastSeen: awarenessState['lastSeen'] as number | undefined, }; users.push(user); } catch (error) { - logger.warn("Invalid user data for client", clientId, error); + logger.warn('Invalid user data for client', clientId, error); } } }); @@ -200,7 +200,7 @@ export const createAwarenessStore = (): AwarenessStore => { */ const handleAwarenessChange = () => { if (!awarenessInstance) { - logger.warn("handleAwarenessChange called without awareness instance"); + logger.warn('handleAwarenessChange called without awareness instance'); return; } @@ -210,7 +210,7 @@ export const createAwarenessStore = (): AwarenessStore => { draft.users = users; draft.lastUpdated = Date.now(); }); - notify("awarenessChange"); + notify('awarenessChange'); }; // ============================================================================= @@ -224,16 +224,16 @@ export const createAwarenessStore = (): AwarenessStore => { awareness: Awareness, userData: LocalUserData ) => { - logger.debug("Initializing awareness", { userData }); + logger.debug('Initializing awareness', { userData }); awarenessInstance = awareness; // Set up awareness with user data - awareness.setLocalStateField("user", userData); - awareness.setLocalStateField("lastSeen", Date.now()); + awareness.setLocalStateField('user', userData); + awareness.setLocalStateField('lastSeen', Date.now()); // Set up awareness observer for Pattern 1 updates - awareness.on("change", handleAwarenessChange); + awareness.on('change', handleAwarenessChange); // Update local state state = produce(state, draft => { @@ -248,17 +248,17 @@ export const createAwarenessStore = (): AwarenessStore => { handleAwarenessChange(); devtools.connect(); - notify("initializeAwareness"); + notify('initializeAwareness'); }; /** * Clean up awareness instance */ const destroyAwareness = () => { - logger.debug("Destroying awareness"); + logger.debug('Destroying awareness'); if (awarenessInstance) { - awarenessInstance.off("change", handleAwarenessChange); + awarenessInstance.off('change', handleAwarenessChange); awarenessInstance = null; } @@ -277,7 +277,7 @@ export const createAwarenessStore = (): AwarenessStore => { draft.isConnected = false; draft.lastUpdated = Date.now(); }); - notify("destroyAwareness"); + notify('destroyAwareness'); }; /** @@ -285,20 +285,20 @@ export const createAwarenessStore = (): AwarenessStore => { */ const updateLocalUserData = (userData: Partial) => { if (!awarenessInstance || !state.localUser) { - logger.warn("Cannot update user data - awareness not initialized"); + logger.warn('Cannot update user data - awareness not initialized'); return; } const updatedUserData = { ...state.localUser, ...userData }; // Update awareness first - awarenessInstance.setLocalStateField("user", updatedUserData); + awarenessInstance.setLocalStateField('user', updatedUserData); // Update local state for immediate UI response state = produce(state, draft => { draft.localUser = updatedUserData; }); - notify("updateLocalUserData"); + notify('updateLocalUserData'); // Note: awareness observer will also fire and update the users array }; @@ -308,12 +308,12 @@ export const createAwarenessStore = (): AwarenessStore => { */ const updateLocalCursor = (cursor: { x: number; y: number } | null) => { if (!awarenessInstance) { - logger.warn("Cannot update cursor - awareness not initialized"); + logger.warn('Cannot update cursor - awareness not initialized'); return; } // Update awareness - awarenessInstance.setLocalStateField("cursor", cursor); + awarenessInstance.setLocalStateField('cursor', cursor); // Immediate local state update for responsiveness state = produce(state, draft => { @@ -330,22 +330,22 @@ export const createAwarenessStore = (): AwarenessStore => { } } }); - notify("updateLocalCursor"); + notify('updateLocalCursor'); }; /** * Update local text selection */ const updateLocalSelection = ( - selection: AwarenessUser["selection"] | null + selection: AwarenessUser['selection'] | null ) => { if (!awarenessInstance) { - logger.warn("Cannot update selection - awareness not initialized"); + logger.warn('Cannot update selection - awareness not initialized'); return; } // Update awareness - awarenessInstance.setLocalStateField("selection", selection); + awarenessInstance.setLocalStateField('selection', selection); // Immediate local state update for responsiveness state = produce(state, draft => { @@ -362,19 +362,20 @@ export const createAwarenessStore = (): AwarenessStore => { } } }); - notify("updateLocalSelection"); + notify('updateLocalSelection'); }; /** * Update last seen timestamp + * @param forceTimestamp - Optional timestamp to use instead of Date.now() */ - const updateLastSeen = () => { + const updateLastSeen = (forceTimestamp?: number) => { if (!awarenessInstance) { return; } - const timestamp = Date.now(); - awarenessInstance.setLocalStateField("lastSeen", timestamp); + const timestamp = forceTimestamp ?? Date.now(); + awarenessInstance.setLocalStateField('lastSeen', timestamp); // Note: We don't update local state here as awareness observer will handle it }; @@ -383,19 +384,96 @@ export const createAwarenessStore = (): AwarenessStore => { * Set up automatic last seen updates */ const setupLastSeenTimer = () => { - if (lastSeenTimer) { - clearInterval(lastSeenTimer); + let frozenTimestamp: number | null = null; + + const startTimer = () => { + if (lastSeenTimer) { + clearInterval(lastSeenTimer); + } + lastSeenTimer = setInterval(() => { + // If page is hidden, use frozen timestamp, otherwise use current time + if (frozenTimestamp) frozenTimestamp++; // This is to make sure that state is updated and data gets transmitted + updateLastSeen(frozenTimestamp ?? undefined); + }, 10000); // Update every 10 seconds + }; + + const getVisibilityProps = () => { + if (typeof document.hidden !== 'undefined') { + return { hidden: 'hidden', visibilityChange: 'visibilitychange' }; + } + + if ( + // @ts-expect-error webkitHidden not defined + typeof (document as unknown as Document).webkitHidden !== 'undefined' + ) { + return { + hidden: 'webkitHidden', + visibilityChange: 'webkitvisibilitychange', + }; + } + // @ts-expect-error mozHidden not defined + if (typeof (document as unknown as Document).mozHidden !== 'undefined') { + return { hidden: 'mozHidden', visibilityChange: 'mozvisibilitychange' }; + } + // @ts-expect-error msHidden not defined + if (typeof (document as unknown as Document).msHidden !== 'undefined') { + return { hidden: 'msHidden', visibilityChange: 'msvisibilitychange' }; + } + return null; + }; + + const visibilityProps = getVisibilityProps(); + + const handleVisibilityChange = () => { + if (!visibilityProps) return; + + const isHidden = (document as unknown as Document)[ + visibilityProps.hidden as keyof Document + ]; + + if (isHidden) { + // Page is hidden, freeze the current timestamp + frozenTimestamp = Date.now(); + } else { + // Page is visible, unfreeze and update immediately + frozenTimestamp = null; + updateLastSeen(); + } + }; + + // Set up visibility change listener if supported + if (visibilityProps) { + document.addEventListener( + visibilityProps.visibilityChange, + handleVisibilityChange + ); + + // Check initial visibility state + const isHidden = (document as unknown as Document)[ + visibilityProps.hidden as keyof Document + ]; + if (isHidden) { + // Start with frozen timestamp if already hidden + frozenTimestamp = Date.now(); + } } - lastSeenTimer = setInterval(() => { - updateLastSeen(); - }, 10000); // Update every 10 seconds + // Always start the timer (whether visible or hidden) + startTimer(); + // cleanup return () => { if (lastSeenTimer) { clearInterval(lastSeenTimer); lastSeenTimer = null; } + + if (visibilityProps) { + document.removeEventListener( + visibilityProps.visibilityChange, + handleVisibilityChange + ); + } }; }; @@ -410,7 +488,7 @@ export const createAwarenessStore = (): AwarenessStore => { state = produce(state, draft => { draft.isConnected = isConnected; }); - notify("setConnected"); + notify('setConnected'); }; // ============================================================================= From c72f4d37999ff20ca1588c2c074512e8c9bb4bea Mon Sep 17 00:00:00 2001 From: Farhan Yahaya Date: Mon, 17 Nov 2025 18:21:21 +0000 Subject: [PATCH 2/4] feat: implement cache to make active collaborators stable --- .../components/ActiveCollaborators.tsx | 2 +- .../stores/createAwarenessStore.ts | 100 +++++++++++++++++- .../collaborative-editor/types/awareness.ts | 21 +++- 3 files changed, 114 insertions(+), 9 deletions(-) diff --git a/assets/js/collaborative-editor/components/ActiveCollaborators.tsx b/assets/js/collaborative-editor/components/ActiveCollaborators.tsx index e1353428e6..8eb0e0c6b8 100644 --- a/assets/js/collaborative-editor/components/ActiveCollaborators.tsx +++ b/assets/js/collaborative-editor/components/ActiveCollaborators.tsx @@ -44,7 +44,7 @@ export function ActiveCollaborators({ className }: ActiveCollaboratorsProps) { return (
{ rawAwareness: null, isConnected: false, lastUpdated: null, + userCache: new Map(), } as AwarenessState, // No initial transformations needed draft => draft @@ -128,11 +129,15 @@ export const createAwarenessStore = (): AwarenessStore => { const listeners = new Set<() => void>(); let awarenessInstance: Awareness | null = null; let lastSeenTimer: NodeJS.Timeout | null = null; + let cacheCleanupTimer: NodeJS.Timeout | null = null; + + // Cache configuration + const CACHE_TTL = 60 * 1000; // 1 minute in milliseconds // Redux DevTools integration const devtools = wrapStoreWithDevTools({ name: 'AwarenessStore', - excludeKeys: ['rawAwareness'], // Exclude Y.js Awareness object + excludeKeys: ['rawAwareness', 'userCache'], // Exclude Y.js Awareness object and Map cache maxAge: 200, // Higher limit since awareness changes are frequent }); @@ -195,6 +200,53 @@ export const createAwarenessStore = (): AwarenessStore => { return users; }; + /** + * Update cache with current users + */ + const updateCache = (users: AwarenessUser[]) => { + const now = Date.now(); + const newCache = new Map(state.userCache); + + // Add/update users in cache + users.forEach(user => { + newCache.set(user.user.id, { + user, + cachedAt: now, + }); + }); + + // Clean up expired entries (older than 1 minute) + newCache.forEach((cachedUser, userId) => { + if (now - cachedUser.cachedAt > CACHE_TTL) { + newCache.delete(userId); + } + }); + + return newCache; + }; + + /** + * Merge live users with cached users + * Cached users are used as fallback when they're missing from live awareness + */ + const mergeUsersWithCache = (liveUsers: AwarenessUser[]): AwarenessUser[] => { + const liveUserIds = new Set(liveUsers.map(u => u.user.id)); + const mergedUsers = [...liveUsers]; + + // Add cached users that aren't in the live set + state.userCache.forEach((cachedUser, userId) => { + if (!liveUserIds.has(userId)) { + // Only add if cache is still valid + const now = Date.now(); + if (now - cachedUser.cachedAt <= CACHE_TTL) { + mergedUsers.push(cachedUser.user); + } + } + }); + + return mergedUsers; + }; + /** * Handle awareness state changes - core collaborative data update pattern */ @@ -204,10 +256,13 @@ export const createAwarenessStore = (): AwarenessStore => { return; } - const users = extractUsersFromAwareness(awarenessInstance); + const liveUsers = extractUsersFromAwareness(awarenessInstance); + const mergedUsers = mergeUsersWithCache(liveUsers); + const updatedCache = updateCache(liveUsers); state = produce(state, draft => { - draft.users = users; + draft.users = mergedUsers; + draft.userCache = updatedCache; draft.lastUpdated = Date.now(); }); notify('awarenessChange'); @@ -217,6 +272,36 @@ export const createAwarenessStore = (): AwarenessStore => { // PATTERN 2: Direct Immer → Notify + Awareness Update (Local Commands) // ============================================================================= + /** + * Set up periodic cache cleanup + */ + const setupCacheCleanup = () => { + if (cacheCleanupTimer) { + clearInterval(cacheCleanupTimer); + } + + // Clean up expired cache entries every 30 seconds + cacheCleanupTimer = setInterval(() => { + const now = Date.now(); + const newCache = new Map(state.userCache); + let hasChanges = false; + + newCache.forEach((cachedUser, userId) => { + if (now - cachedUser.cachedAt > CACHE_TTL) { + newCache.delete(userId); + hasChanges = true; + } + }); + + if (hasChanges) { + state = produce(state, draft => { + draft.userCache = newCache; + }); + notify('cacheCleanup'); + } + }, 30000); // Check every 30 seconds + }; + /** * Initialize awareness instance and set up observers */ @@ -235,6 +320,9 @@ export const createAwarenessStore = (): AwarenessStore => { // Set up awareness observer for Pattern 1 updates awareness.on('change', handleAwarenessChange); + // Set up cache cleanup + setupCacheCleanup(); + // Update local state state = produce(state, draft => { draft.localUser = userData; @@ -267,6 +355,11 @@ export const createAwarenessStore = (): AwarenessStore => { lastSeenTimer = null; } + if (cacheCleanupTimer) { + clearInterval(cacheCleanupTimer); + cacheCleanupTimer = null; + } + devtools.disconnect(); state = produce(state, draft => { @@ -276,6 +369,7 @@ export const createAwarenessStore = (): AwarenessStore => { draft.isInitialized = false; draft.isConnected = false; draft.lastUpdated = Date.now(); + draft.userCache = new Map(); }); notify('destroyAwareness'); }; diff --git a/assets/js/collaborative-editor/types/awareness.ts b/assets/js/collaborative-editor/types/awareness.ts index f6a8dafeab..e955277fde 100644 --- a/assets/js/collaborative-editor/types/awareness.ts +++ b/assets/js/collaborative-editor/types/awareness.ts @@ -1,8 +1,8 @@ -import type { Awareness } from "y-protocols/awareness"; -import type { RelativePosition } from "yjs"; -import { z } from "zod"; +import type { Awareness } from 'y-protocols/awareness'; +import type { RelativePosition } from 'yjs'; +import { z } from 'zod'; -import type { WithSelector } from "../stores/common"; +import type { WithSelector } from '../stores/common'; /** * User information stored in awareness @@ -37,6 +37,14 @@ export interface LocalUserData { color: string; } +/** + * Cached user entry for fallback when awareness is throttled + */ +export interface CachedUser { + user: AwarenessUser; + cachedAt: number; +} + /** * Awareness store state */ @@ -52,6 +60,9 @@ export interface AwarenessState { // Connection state isConnected: boolean; lastUpdated: number | null; + + // Fallback cache for throttled awareness updates (1 minute TTL) + userCache: Map; } /** @@ -75,7 +86,7 @@ export interface AwarenessCommands { // Local user updates updateLocalUserData: (userData: Partial) => void; updateLocalCursor: (cursor: { x: number; y: number } | null) => void; - updateLocalSelection: (selection: AwarenessUser["selection"] | null) => void; + updateLocalSelection: (selection: AwarenessUser['selection'] | null) => void; updateLastSeen: () => void; // Connection state From ecacf899df2c32ad129878be54089a7c11a4dee0 Mon Sep 17 00:00:00 2001 From: Farhan Yahaya Date: Mon, 17 Nov 2025 19:28:05 +0000 Subject: [PATCH 3/4] tests: update tests --- .../components/ActiveCollaborators.test.tsx | 462 +++++++++++------- 1 file changed, 277 insertions(+), 185 deletions(-) diff --git a/assets/test/collaborative-editor/components/ActiveCollaborators.test.tsx b/assets/test/collaborative-editor/components/ActiveCollaborators.test.tsx index 5aaf117c97..6d713f1446 100644 --- a/assets/test/collaborative-editor/components/ActiveCollaborators.test.tsx +++ b/assets/test/collaborative-editor/components/ActiveCollaborators.test.tsx @@ -11,16 +11,16 @@ * 7. Edge Cases - Empty states and state transitions */ -import { render, screen } from "@testing-library/react"; -import { beforeEach, describe, expect, test, vi } from "vitest"; +import { render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; -import { ActiveCollaborators } from "../../../js/collaborative-editor/components/ActiveCollaborators"; -import type { AwarenessUser } from "../../../js/collaborative-editor/types/awareness"; +import { ActiveCollaborators } from '../../../js/collaborative-editor/components/ActiveCollaborators'; +import type { AwarenessUser } from '../../../js/collaborative-editor/types/awareness'; // Mock the useRemoteUsers hook let mockRemoteUsers: AwarenessUser[] = []; -vi.mock("../../../js/collaborative-editor/hooks/useAwareness", () => ({ +vi.mock('../../../js/collaborative-editor/hooks/useAwareness', () => ({ useRemoteUsers: () => mockRemoteUsers, })); @@ -34,21 +34,21 @@ function createMockAwarenessUser( clientId: Math.floor(Math.random() * 1000000), user: { id: `user-${Math.random().toString(36).substring(7)}`, - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, lastSeen: Date.now(), ...overrides, }; } -describe("ActiveCollaborators - Basic Rendering", () => { +describe('ActiveCollaborators - Basic Rendering', () => { beforeEach(() => { mockRemoteUsers = []; }); - test("renders nothing when there are no remote users", () => { + test('renders nothing when there are no remote users', () => { mockRemoteUsers = []; const { container } = render(); @@ -56,22 +56,22 @@ describe("ActiveCollaborators - Basic Rendering", () => { expect(container.firstChild).toBeNull(); }); - test("renders avatars for remote users", () => { + test('renders avatars for remote users', () => { mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, }), createMockAwarenessUser({ user: { - id: "user-2", - name: "Jane Smith", - email: "jane@example.com", - color: "#00ff00", + id: 'user-2', + name: 'Jane Smith', + email: 'jane@example.com', + color: '#00ff00', }, }), ]; @@ -79,53 +79,53 @@ describe("ActiveCollaborators - Basic Rendering", () => { const { container } = render(); expect(container.firstChild).not.toBeNull(); - expect(screen.getByText("JD")).toBeInTheDocument(); - expect(screen.getByText("JS")).toBeInTheDocument(); + expect(screen.getByText('JD')).toBeInTheDocument(); + expect(screen.getByText('JS')).toBeInTheDocument(); }); - test("renders correct number of avatars matching remote users count", () => { + test('renders correct number of avatars matching remote users count', () => { mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "Alice Anderson", - email: "alice@example.com", - color: "#ff0000", + id: 'user-1', + name: 'Alice Anderson', + email: 'alice@example.com', + color: '#ff0000', }, }), createMockAwarenessUser({ user: { - id: "user-2", - name: "Bob Brown", - email: "bob@example.com", - color: "#00ff00", + id: 'user-2', + name: 'Bob Brown', + email: 'bob@example.com', + color: '#00ff00', }, }), createMockAwarenessUser({ user: { - id: "user-3", - name: "Charlie Chen", - email: "charlie@example.com", - color: "#0000ff", + id: 'user-3', + name: 'Charlie Chen', + email: 'charlie@example.com', + color: '#0000ff', }, }), ]; render(); - expect(screen.getByText("AA")).toBeInTheDocument(); - expect(screen.getByText("BB")).toBeInTheDocument(); - expect(screen.getByText("CC")).toBeInTheDocument(); + expect(screen.getByText('AA')).toBeInTheDocument(); + expect(screen.getByText('BB')).toBeInTheDocument(); + expect(screen.getByText('CC')).toBeInTheDocument(); }); - test("renders correct initials for single-word names", () => { + test('renders correct initials for single-word names', () => { mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "Madonna", - email: "madonna@example.com", - color: "#ff0000", + id: 'user-1', + name: 'Madonna', + email: 'madonna@example.com', + color: '#ff0000', }, }), ]; @@ -133,17 +133,17 @@ describe("ActiveCollaborators - Basic Rendering", () => { render(); // Single-word name should result in "??" (no last name) - expect(screen.getByText("??")).toBeInTheDocument(); + expect(screen.getByText('??')).toBeInTheDocument(); }); - test("renders correct initials for multi-part names (uses first and last parts)", () => { + test('renders correct initials for multi-part names (uses first and last parts)', () => { mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "Mary Jane Watson Parker", - email: "mary@example.com", - color: "#ff0000", + id: 'user-1', + name: 'Mary Jane Watson Parker', + email: 'mary@example.com', + color: '#ff0000', }, }), ]; @@ -151,67 +151,67 @@ describe("ActiveCollaborators - Basic Rendering", () => { render(); // Should use first part "Mary" and last part "Parker" - expect(screen.getByText("MP")).toBeInTheDocument(); + expect(screen.getByText('MP')).toBeInTheDocument(); }); }); -describe("ActiveCollaborators - Activity Indicator", () => { +describe('ActiveCollaborators - Activity Indicator', () => { beforeEach(() => { mockRemoteUsers = []; }); - test("shows green border for users active within last 2 minutes", () => { + test('shows green border for users active within last 12 seconds (0.2 minutes)', () => { const now = Date.now(); - const oneMinuteAgo = now - 60 * 1000; // 1 minute ago + const fiveSecondsAgo = now - 5 * 1000; // 5 seconds ago mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, - lastSeen: oneMinuteAgo, + lastSeen: fiveSecondsAgo, }), ]; const { container } = render(); - const borderDiv = container.querySelector(".border-green-500"); + const borderDiv = container.querySelector('.border-green-500'); expect(borderDiv).toBeInTheDocument(); }); - test("shows gray border for users inactive for more than 2 minutes", () => { + test('shows gray border for users inactive for more than 12 seconds (0.2 minutes)', () => { const now = Date.now(); - const threeMinutesAgo = now - 3 * 60 * 1000; // 3 minutes ago + const thirtySecondsAgo = now - 30 * 1000; // 30 seconds ago mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, - lastSeen: threeMinutesAgo, + lastSeen: thirtySecondsAgo, }), ]; const { container } = render(); - const borderDiv = container.querySelector(".border-gray-500"); + const borderDiv = container.querySelector('.border-gray-500'); expect(borderDiv).toBeInTheDocument(); }); - test("shows gray border when lastSeen is undefined", () => { + test('shows gray border when lastSeen is undefined', () => { mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, lastSeen: undefined, }), @@ -219,59 +219,59 @@ describe("ActiveCollaborators - Activity Indicator", () => { const { container } = render(); - const borderDiv = container.querySelector(".border-gray-500"); + const borderDiv = container.querySelector('.border-gray-500'); expect(borderDiv).toBeInTheDocument(); }); - test("correctly implements the 2-minute threshold (120,000ms)", () => { + test('correctly implements the 12-second threshold (0.2 minutes = 12,000ms)', () => { const now = Date.now(); - const justUnderTwoMinutes = now - (2 * 60 * 1000 - 1000); // 1 second before threshold - const justOverTwoMinutes = now - (2 * 60 * 1000 + 1000); // 1 second after threshold + const justUnderThreshold = now - (0.2 * 60 * 1000 - 1000); // 1 second before threshold (11 seconds ago) + const justOverThreshold = now - (0.2 * 60 * 1000 + 1000); // 1 second after threshold (13 seconds ago) - // User just under 2 minutes should have green border + // User just under 12 seconds should have green border mockRemoteUsers = [ createMockAwarenessUser({ clientId: 1, user: { - id: "user-1", - name: "Active User", - email: "active@example.com", - color: "#ff0000", + id: 'user-1', + name: 'Active User', + email: 'active@example.com', + color: '#ff0000', }, - lastSeen: justUnderTwoMinutes, + lastSeen: justUnderThreshold, }), createMockAwarenessUser({ clientId: 2, user: { - id: "user-2", - name: "Inactive User", - email: "inactive@example.com", - color: "#00ff00", + id: 'user-2', + name: 'Inactive User', + email: 'inactive@example.com', + color: '#00ff00', }, - lastSeen: justOverTwoMinutes, + lastSeen: justOverThreshold, }), ]; const { container } = render(); - const greenBorder = container.querySelector(".border-green-500"); - const grayBorder = container.querySelector(".border-gray-500"); + const greenBorder = container.querySelector('.border-green-500'); + const grayBorder = container.querySelector('.border-gray-500'); expect(greenBorder).toBeInTheDocument(); expect(grayBorder).toBeInTheDocument(); }); - test("border color updates when user crosses the 2-minute threshold", () => { + test('border color updates when user crosses the 12-second threshold', () => { vi.useFakeTimers(); const now = Date.now(); mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, lastSeen: now, }), @@ -280,35 +280,35 @@ describe("ActiveCollaborators - Activity Indicator", () => { const { container, rerender } = render(); // Initially should have green border - expect(container.querySelector(".border-green-500")).toBeInTheDocument(); + expect(container.querySelector('.border-green-500')).toBeInTheDocument(); - // Advance time by 3 minutes - vi.advanceTimersByTime(3 * 60 * 1000); + // Advance time by 15 seconds (beyond the 12-second threshold) + vi.advanceTimersByTime(15 * 1000); // Re-render to trigger the component to re-evaluate rerender(); // Now should have gray border - expect(container.querySelector(".border-gray-500")).toBeInTheDocument(); + expect(container.querySelector('.border-gray-500')).toBeInTheDocument(); vi.useRealTimers(); }); }); -describe("ActiveCollaborators - Store Integration", () => { +describe('ActiveCollaborators - Store Integration', () => { beforeEach(() => { mockRemoteUsers = []; }); - test("uses useRemoteUsers hook correctly (excludes local user)", () => { + test('uses useRemoteUsers hook correctly (excludes local user)', () => { // The hook should already filter out local users, so we only set remote users mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "remote-user", - name: "Remote User", - email: "remote@example.com", - color: "#00ff00", + id: 'remote-user', + name: 'Remote User', + email: 'remote@example.com', + color: '#00ff00', }, }), ]; @@ -316,25 +316,25 @@ describe("ActiveCollaborators - Store Integration", () => { render(); // Should show remote user - expect(screen.getByText("RU")).toBeInTheDocument(); + expect(screen.getByText('RU')).toBeInTheDocument(); }); - test("updates when awareness state changes (users join)", () => { + test('updates when awareness state changes (users join)', () => { mockRemoteUsers = []; const { rerender } = render(); // Initially no users - expect(screen.queryByText("JD")).not.toBeInTheDocument(); + expect(screen.queryByText('JD')).not.toBeInTheDocument(); // Add a user mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, }), ]; @@ -342,17 +342,17 @@ describe("ActiveCollaborators - Store Integration", () => { rerender(); // User should now appear - expect(screen.getByText("JD")).toBeInTheDocument(); + expect(screen.getByText('JD')).toBeInTheDocument(); }); - test("updates when awareness state changes (users leave)", () => { + test('updates when awareness state changes (users leave)', () => { mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, }), ]; @@ -360,17 +360,17 @@ describe("ActiveCollaborators - Store Integration", () => { const { rerender } = render(); // User should be visible - expect(screen.getByText("JD")).toBeInTheDocument(); + expect(screen.getByText('JD')).toBeInTheDocument(); // Remove user mockRemoteUsers = []; rerender(); // User should no longer be visible - expect(screen.queryByText("JD")).not.toBeInTheDocument(); + expect(screen.queryByText('JD')).not.toBeInTheDocument(); }); - test("handles empty users array gracefully", () => { + test('handles empty users array gracefully', () => { mockRemoteUsers = []; const { container } = render(); @@ -379,27 +379,119 @@ describe("ActiveCollaborators - Store Integration", () => { }); }); -describe("ActiveCollaborators - Edge Cases", () => { +describe('ActiveCollaborators - Cache Behavior', () => { beforeEach(() => { mockRemoteUsers = []; }); - test("transitions from showing avatars to null when all users leave", () => { + test('continues showing users from cache when awareness is throttled', () => { + const now = Date.now(); + + // Initial user list + mockRemoteUsers = [ + createMockAwarenessUser({ + user: { + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', + }, + lastSeen: now, + }), + ]; + + const { rerender } = render(); + expect(screen.getByText('JD')).toBeInTheDocument(); + + // Simulate throttling - user still in cache (via merged users from store) + // The cache keeps users for 1 minute, so they should still appear + mockRemoteUsers = [ + createMockAwarenessUser({ + user: { + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', + }, + lastSeen: now, + }), + ]; + + rerender(); + expect(screen.getByText('JD')).toBeInTheDocument(); + }); + + test('cached users appear with their last known state', () => { + const now = Date.now(); + const fiveSecondsAgo = now - 5 * 1000; + + // User active 5 seconds ago + mockRemoteUsers = [ + createMockAwarenessUser({ + user: { + id: 'user-1', + name: 'Jane Smith', + email: 'jane@example.com', + color: '#00ff00', + }, + lastSeen: fiveSecondsAgo, + }), + ]; + + render(); + + // Should still show as active (green border) because < 12 seconds + expect(screen.getByText('JS')).toBeInTheDocument(); + expect(screen.getByText('JS').closest('div')?.parentElement).toHaveClass( + 'border-green-500' + ); + }); + + test('cached users eventually expire after threshold', () => { + const now = Date.now(); + const thirtySecondsAgo = now - 30 * 1000; // Well over 12 seconds + + mockRemoteUsers = [ + createMockAwarenessUser({ + user: { + id: 'user-1', + name: 'Old User', + email: 'old@example.com', + color: '#ff0000', + }, + lastSeen: thirtySecondsAgo, + }), + ]; + + const { container } = render(); + + // Should show with gray border (inactive) because > 12 seconds + expect(screen.getByText('OU')).toBeInTheDocument(); + expect(container.querySelector('.border-gray-500')).toBeInTheDocument(); + }); +}); + +describe('ActiveCollaborators - Edge Cases', () => { + beforeEach(() => { + mockRemoteUsers = []; + }); + + test('transitions from showing avatars to null when all users leave', () => { mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, }), createMockAwarenessUser({ user: { - id: "user-2", - name: "Jane Smith", - email: "jane@example.com", - color: "#00ff00", + id: 'user-2', + name: 'Jane Smith', + email: 'jane@example.com', + color: '#00ff00', }, }), ]; @@ -407,8 +499,8 @@ describe("ActiveCollaborators - Edge Cases", () => { const { container, rerender } = render(); // Should show users - expect(screen.getByText("JD")).toBeInTheDocument(); - expect(screen.getByText("JS")).toBeInTheDocument(); + expect(screen.getByText('JD')).toBeInTheDocument(); + expect(screen.getByText('JS')).toBeInTheDocument(); // All users leave mockRemoteUsers = []; @@ -418,33 +510,33 @@ describe("ActiveCollaborators - Edge Cases", () => { expect(container.firstChild).toBeNull(); }); - test("handles multiple users with similar names", () => { + test('handles multiple users with similar names', () => { mockRemoteUsers = [ createMockAwarenessUser({ clientId: 1, user: { - id: "user-1", - name: "John Doe", - email: "john1@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john1@example.com', + color: '#ff0000', }, }), createMockAwarenessUser({ clientId: 2, user: { - id: "user-2", - name: "John Doe", - email: "john2@example.com", - color: "#00ff00", + id: 'user-2', + name: 'John Doe', + email: 'john2@example.com', + color: '#00ff00', }, }), createMockAwarenessUser({ clientId: 3, user: { - id: "user-3", - name: "Jane Doe", - email: "jane@example.com", - color: "#0000ff", + id: 'user-3', + name: 'Jane Doe', + email: 'jane@example.com', + color: '#0000ff', }, }), ]; @@ -452,18 +544,18 @@ describe("ActiveCollaborators - Edge Cases", () => { render(); // All three should render (React will handle duplicate text content) - const jdElements = screen.getAllByText("JD"); + const jdElements = screen.getAllByText('JD'); expect(jdElements).toHaveLength(3); // 2 John Doe + 1 Jane Doe }); - test("handles user with undefined lastSeen timestamp", () => { + test('handles user with undefined lastSeen timestamp', () => { mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, lastSeen: undefined, }), @@ -472,34 +564,34 @@ describe("ActiveCollaborators - Edge Cases", () => { const { container } = render(); // Should render without crashing - expect(screen.getByText("JD")).toBeInTheDocument(); + expect(screen.getByText('JD')).toBeInTheDocument(); // Should have gray border (inactive) - expect(container.querySelector(".border-gray-500")).toBeInTheDocument(); + expect(container.querySelector('.border-gray-500')).toBeInTheDocument(); }); - test("handles rapidly changing awareness states", () => { + test('handles rapidly changing awareness states', () => { const user1 = createMockAwarenessUser({ user: { - id: "user-1", - name: "John Doe", - email: "john@example.com", - color: "#ff0000", + id: 'user-1', + name: 'John Doe', + email: 'john@example.com', + color: '#ff0000', }, }); const user2 = createMockAwarenessUser({ user: { - id: "user-2", - name: "Jane Smith", - email: "jane@example.com", - color: "#00ff00", + id: 'user-2', + name: 'Jane Smith', + email: 'jane@example.com', + color: '#00ff00', }, }); const user3 = createMockAwarenessUser({ user: { - id: "user-3", - name: "Bob Brown", - email: "bob@example.com", - color: "#0000ff", + id: 'user-3', + name: 'Bob Brown', + email: 'bob@example.com', + color: '#0000ff', }, }); @@ -508,33 +600,33 @@ describe("ActiveCollaborators - Edge Cases", () => { // Rapid state changes mockRemoteUsers = [user1]; rerender(); - expect(screen.getByText("JD")).toBeInTheDocument(); + expect(screen.getByText('JD')).toBeInTheDocument(); mockRemoteUsers = [user1, user2]; rerender(); - expect(screen.getByText("JD")).toBeInTheDocument(); - expect(screen.getByText("JS")).toBeInTheDocument(); + expect(screen.getByText('JD')).toBeInTheDocument(); + expect(screen.getByText('JS')).toBeInTheDocument(); mockRemoteUsers = [user2, user3]; rerender(); - expect(screen.queryByText("JD")).not.toBeInTheDocument(); - expect(screen.getByText("JS")).toBeInTheDocument(); - expect(screen.getByText("BB")).toBeInTheDocument(); + expect(screen.queryByText('JD')).not.toBeInTheDocument(); + expect(screen.getByText('JS')).toBeInTheDocument(); + expect(screen.getByText('BB')).toBeInTheDocument(); mockRemoteUsers = []; rerender(); - expect(screen.queryByText("JS")).not.toBeInTheDocument(); - expect(screen.queryByText("BB")).not.toBeInTheDocument(); + expect(screen.queryByText('JS')).not.toBeInTheDocument(); + expect(screen.queryByText('BB')).not.toBeInTheDocument(); }); - test("handles empty name strings", () => { + test('handles empty name strings', () => { mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: "", - email: "empty@example.com", - color: "#ff0000", + id: 'user-1', + name: '', + email: 'empty@example.com', + color: '#ff0000', }, }), ]; @@ -542,17 +634,17 @@ describe("ActiveCollaborators - Edge Cases", () => { render(); // Should show fallback "??" - expect(screen.getByText("??")).toBeInTheDocument(); + expect(screen.getByText('??')).toBeInTheDocument(); }); - test("handles names with only whitespace", () => { + test('handles names with only whitespace', () => { mockRemoteUsers = [ createMockAwarenessUser({ user: { - id: "user-1", - name: " ", - email: "whitespace@example.com", - color: "#ff0000", + id: 'user-1', + name: ' ', + email: 'whitespace@example.com', + color: '#ff0000', }, }), ]; @@ -560,6 +652,6 @@ describe("ActiveCollaborators - Edge Cases", () => { render(); // Should show fallback "??" because trimmed name parts are empty - expect(screen.getByText("??")).toBeInTheDocument(); + expect(screen.getByText('??')).toBeInTheDocument(); }); }); From b917230d520e28c46cf51503ad6c6720fb7df978 Mon Sep 17 00:00:00 2001 From: Farhan Yahaya Date: Tue, 18 Nov 2025 13:41:31 +0000 Subject: [PATCH 4/4] chore: update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3c3e557bc..ce1210b3f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ and this project adheres to ### Fixed +- Fix flickering of active collaborator icons between states(active, inactive, + unavailable) [#3931](https://github.com/OpenFn/lightning/issues/3931) - Fix ghost edges blocking saves and breaking autolayout when deleting jobs in collaborative editor [#3983](https://github.com/OpenFn/lightning/issues/3983) - Fix tooltip styling inconsistencies in collaborative editor