fix: improve collaborator cursor color generation - #177
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
👋 Thanks for opening a PR, @prathik-05!Your PR has entered the 🚦 PR Review Pipeline.
What happens next
A pipeline status comment will appear below and update automatically as your PR progresses. While you wait
This comment is posted only once. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughReplaces single-character trig/hex email hashing with a full-string deterministic ChangesCollaboration Cursor Color Fix
Sequence Diagram(s)sequenceDiagram
participant PlaygroundEditor
participant hashStringToColor
participant withOpacity
participant YjsAwareness
PlaygroundEditor->>hashStringToColor: provide session.user.email or fallback seed
hashStringToColor-->>PlaygroundEditor: return hsl(...) / hsla(...)
PlaygroundEditor->>YjsAwareness: set local user.color (hsl/hsla)
PlaygroundEditor->>withOpacity: provide user.color and alpha
withOpacity-->>PlaygroundEditor: return reduced-opacity color
PlaygroundEditor->>YjsAwareness: set remote selection background color
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/playground/components/playground-editor.tsx`:
- Around line 309-310: The computed user color from hashStringToColor (assigned
to userColor) may be hex or HSL, but the selection highlight code currently
naively appends "40" to form translucency, which breaks for HSL; update the
playground-editor.tsx logic that builds the collaborator selection color (where
`${color}40` is used) to handle both formats: detect if userColor is hex and
append the hex alpha, otherwise convert HSL to an alpha form (produce an HSLA
string or use the modern hsl(... / alpha) syntax) or normalize hashStringToColor
to always return hex; adjust either hashStringToColor or the selection color
helper so collaborator highlights are generated with a valid alpha-aware color
string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: dc8d140f-bb3c-407b-b5f9-68dfe0457284
📒 Files selected for processing (1)
modules/playground/components/playground-editor.tsx
piyushdotcomm
left a comment
piyushdotcomm
left a comment
There was a problem hiding this comment.
The HSL conversion is a good idea, but the opacity handling is broken. Please address the inline comments.
| css += ` | ||
| .yRemoteSelection-${clientId} { | ||
| background-color: ${color}40; /* 40 hex is 25% opacity */ | ||
| background-color: ${color}; /* 40 hex is 25% opacity */ |
There was a problem hiding this comment.
Since color is now an HSL string with alpha 1, this makes the selection background completely opaque and it will obscure the text. You can't append 40 to HSL either. The safest cross-format way to handle this (since some remote clients might still send Hex) is to apply opacity: 0.25; to this class and keep background-color: ${color};.
| left: -2px; | ||
| font-size: 11px; | ||
| background-color: ${color}; | ||
| background-color: ${hashStringToColor(name, 0.25)}; |
There was a problem hiding this comment.
This sets the name label's background to 25% opacity, making the text unreadable. It also ignores the collaborator's actual assigned color and generates a new one from their name. You should just use background-color: ${color}; here so it matches the cursor exactly and remains opaque.
github-actions
Bot
added
gssoc:approved
level:beginner
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
modules/playground/components/playground-editor.tsx (1)
342-342: 💤 Low valueUpdate the outdated comment.
The comment
/* 40 hex is 25% opacity */references the previous hex-based approach and is no longer accurate. Consider updating it to reflect the current HSL alpha replacement approach or remove it entirely.📝 Suggested comment update
-background-color: ${color.replace("/ 1)", "/ 0.25)")}; /* 40 hex is 25% opacity */ +background-color: ${color.replace("/ 1)", "/ 0.25)")}; /* 25% opacity for selection background */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/playground/components/playground-editor.tsx` at line 342, The inline comment next to the background-color assignment in playground-editor.tsx is outdated (it reads "/* 40 hex is 25% opacity */") while the code now uses color.replace("/ 1)", "/ 0.25)") to set an HSL alpha; update or remove that comment to reflect the HSL alpha approach (e.g., "/* set 25% alpha via HSL alpha replacement */") or simply delete it next to the background-color line inside the PlaygroundEditor component so the comment matches the current color.replace usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/playground/components/playground-editor.tsx`:
- Line 316: The current userColor assignment uses a constant fallback ("hsl(210
100% 56% / 1)") for anonymous users, causing identical cursor colors; change the
fallback to generate a unique color by hashing a stable anonymous identifier
(e.g., session.id, session.clientId, or session.connectionTimestamp) or a
per-session random seed so each anonymous session gets a distinct color. Update
the expression that sets userColor (currently referencing session?.user?.email
and hashStringToColor) to prefer hashing session.user.email when present,
otherwise hash a stable anonymous key (session.id || session.clientId ||
session.connectionTimestamp) and only fall back to a truly random color if no
stable identifier exists. Ensure you keep using hashStringToColor for generation
so colors remain in the same palette and reference the userColor variable and
hashStringToColor function when making the change.
- Line 342: The current string .replace("/ 1)", "/ 0.25)") breaks when
state.user.color falls back to the named color "orange"; add a small helper
function addAlpha(colorStr: string, alpha: number) and call it where color is
used so you always produce a color with the desired alpha. Implement addAlpha
to: detect and replace an existing alpha token (e.g. match /\/\s*[\d.]+\)/ and
replace with `/ ${alpha})`), and for non-HSL/named colors resolve them to an
RGB(A) form (e.g. compute the computed style or parse rgb(...) and append the
alpha) so the background-color assignment always receives a valid
semitransparent color; replace the direct .replace call with addAlpha(color,
0.25).
---
Nitpick comments:
In `@modules/playground/components/playground-editor.tsx`:
- Line 342: The inline comment next to the background-color assignment in
playground-editor.tsx is outdated (it reads "/* 40 hex is 25% opacity */") while
the code now uses color.replace("/ 1)", "/ 0.25)") to set an HSL alpha; update
or remove that comment to reflect the HSL alpha approach (e.g., "/* set 25%
alpha via HSL alpha replacement */") or simply delete it next to the
background-color line inside the PlaygroundEditor component so the comment
matches the current color.replace usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0dc59bcb-d23b-438a-b8d0-2ca037b89609
📒 Files selected for processing (1)
modules/playground/components/playground-editor.tsx
piyushdotcomm
left a comment
piyushdotcomm
left a comment
There was a problem hiding this comment.
add the screenshot of the change and address all the suggestion by code rabbit
2 participants
Summary
Type of change
Related issue
Closes #144
Validation
npm run lintnpm testnpm run buildList any additional manual verification you performed:
Checklist
Summary by CodeRabbit