Fix collaboration cursor bubbles not appearing in coaching notes #230
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resolved issue where collaboration cursors would disappear when clicking between browsers by fixing critical useEffect cleanup bug. Key fixes: - Prevent provider disconnect on dependency changes (only on session change) - Prevent extension recreation with synced event guard - Add provider reinitialization guard - Fix awareness state type (Array not Map) - Generate unique colors per user - Remove mouse position tracking
Use provider.setAwarenessField() instead of provider.awareness.setLocalStateField() for better encapsulation and consistency with the public Hocuspocus API.
Add test coverage for Issue #201 fixes: - Provider lifecycle: verify provider NOT disconnected on re-render with same sessionId - Provider cleanup: verify provider IS disconnected when sessionId changes - Extension creation: verify extensions created only once despite multiple synced events - Awareness API: verify high-level setAwarenessField() is used for presence - Awareness state: verify presence state updates on awarenessChange events - Disconnect handling: verify awareness set to disconnected status on disconnect event
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).
The caret height was using 1.2em which is relative to the font-size of the current context, causing different heights depending on where the cursor appears in the document. Changes: - Changed height from 1.2em to 1em to match actual text height - Added line-height: inherit to match the line-height of surrounding text - Added vertical-align: baseline for proper alignment - Removed min-height: 16px as it's no longer needed This ensures all collaboration carets have consistent heights that match the line height of the text where they appear.
When hovering over a collaboration caret name bubble with the mouse, it now becomes highly transparent (20% opacity) to improve UX by allowing users to see through the label when it's covering content. Changes: - Changed pointer-events from none to auto to enable hover interactions - Added opacity: 0.2 on hover for the label bubble and tail (80% transparent) - Consolidated duplicate hover effects into single rule - Maintains existing hover transform and shadow effects - Added cursor: pointer to indicate interactivity
calebbourg
requested changes
Nov 14, 2025
Collaborator
calebbourg
left a comment
calebbourg
left a comment
There was a problem hiding this comment.
1 question and one suggested change for the tests!
| expect(screen.getByTestId('is-ready')).toHaveTextContent('no') | ||
| }) | ||
|
|
||
| describe('Provider Lifecycle (Issue #201 Fixes)', () => { |
Collaborator
There was a problem hiding this comment.
Maybe a more descriptive name so we don't have to look back at the issue to understand the need for the test?
Comment on lines
197
to
210
| setCache((prev) => { | ||
| const mergedUsers = new Map(prev.presenceState.users); | ||
|
|
||
| // Mark users who disappeared from awareness as disconnected | ||
| 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() | ||
| }); | ||
| } |
Collaborator
There was a problem hiding this comment.
I'm curious what this provides? What is the issue if users "Disappear" and what does that mean exactly?
Member
Author
There was a problem hiding this comment.
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).
Member
Author
There was a problem hiding this comment.
Added this as a code comment in commit 1c4d370
- Update test description to be self-explanatory without referencing issue number - Add detailed comment explaining awareness state handling for disconnected users
calebbourg
approved these changes
Nov 14, 2025
github-project-automation
bot
moved this from Review
to ✅ Done
in Refactor Coaching Platform
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fix collaboration cursor bubbles not appearing in coaching session notes. The issue was caused by the React useEffect cleanup function disconnecting the TipTap collaboration provider whenever dependencies changed, even when the session ID remained the same. This resulted in cursors disappearing when users clicked between browsers.
This also seems to fix the instability of the presence indicators at the top of the coaching session page as well. The unstable user presence in each browser (coach and coachee) seem to be directly related to unstable user carets as well.
Additional fixes: Further improvements to presence indicator behavior during network disconnects, visual consistency of collaboration carets, and UX enhancements for caret name bubbles.
GitHub Issue: Fixes #201 and Closes #184
Changes
Core Collaboration Fixes:
provider.awareness.setLocalStateField()to high-levelprovider.setAwarenessField()APIMaptoArrayto match Hocuspocus APIgenerateCollaborativeUserColor()Presence Indicator Fixes:
Visual Consistency & UX Fixes:
1.2emto1emto match actual text heightline-height: inheritandvertical-align: baselinefor consistent caret renderingpointer-eventsfromnonetoautoto enable hover interactionsTest Coverage:
Screenshots / Videos Showing UI Changes (if applicable)
Testing Strategy
Unit Tests: All 11 tests in
editor-cache-context.test.tsxpass, covering:setAwarenessField())Manual Testing:
Concerns
None. The fix addresses the root cause of the provider disconnection issue and includes comprehensive test coverage to prevent regression.