-
Notifications
You must be signed in to change notification settings - Fork 113
fix(frontend): resolve unresolved merge conflicts in pages #139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughCentralized WebSocket message parsing/dispatch added to Game.tsx; merge-conflict markers resolved across MatchLogs, TeamBuilder, TeamDebateRoom; TeamDebateRoom ref/state consolidation and media/setup relocation; TeamBuilder capacity default tightened; OnlineDebateRoom and TeamDebateRoom standardized to BASE_URL/WS_BASE_URL for API and WS endpoints. All edits are internal frontend component changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client (Browser UI)
participant WS as WebSocket Server
participant Handler as handleWebSocketMessage
participant State as UI State / Components
participant Transcript as Transcript/Chat subsystem
WS-->Browser: raw JSON message
Browser->>Handler: forward raw message
Handler-->>Handler: parseContent -> validated payload
alt control messages (DEBATE_START / TURN_START / TURN_END / DEBATE_END)
Handler->>State: update phase/turn/gameEnded
State-->>Browser: re-render controls (disabled = loading || gameEnded)
else chat/transcript (CHAT_MESSAGE / GENERATING_TRANSCRIPT)
Handler->>Transcript: append or begin generation
Transcript-->>Browser: update transcript UI
else presence (TYPING_START/STOP / SPEAKING_START/STOP)
Handler->>State: toggle typing/speaking indicators
else GAME_RESULT
Handler->>State: set final results
end
note right of Handler: malformed messages are logged and ignored
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/Pages/TeamDebateRoom.tsx (2)
196-198: DuplicateuseEffecthooks for ref synchronization remain after merge.The merge conflict resolution left multiple duplicate
useEffecthooks that synchronize the same refs:
Ref Duplicate locations debatePhaseRefLines 196-198, 311-313, and 681-683 (3 copies!) currentUserIdRefLines 297-299 and 685-687 myTeamIdRefLines 301-303 and 677-679 isTeam1RefLines 306-309 and 673-675 These don't break functionality but indicate incomplete merge resolution. Keep only one set of these useEffects (lines 673-687 appear to be the consolidated block).
Suggested fix: Remove duplicate useEffects
Remove the first
debatePhaseRefsync at lines 196-198:- useEffect(() => { - debatePhaseRef.current = debatePhase; - }, [debatePhase]);Remove the duplicate sync block at lines 297-313:
const currentUserIdRef = useRef<string | undefined>(currentUser?.id); const isTeam1Ref = useRef<boolean>(isTeam1); const debatePhaseRef = useRef<DebatePhase>(debatePhase); const myTeamIdRef = useRef<string | null>(myTeamId); - useEffect(() => { - currentUserIdRef.current = currentUser?.id; - }, [currentUser?.id]); - - useEffect(() => { - myTeamIdRef.current = myTeamId; -}, [myTeamId]); - - - useEffect(() => { - - isTeam1Ref.current = isTeam1; - }, [isTeam1]); - - useEffect(() => { - debatePhaseRef.current = debatePhase; - }, [debatePhase]);Keep only the consolidated block at lines 673-687.
Also applies to: 311-313, 673-687
1508-1508: Hardcoded localhost URL for transcript submission.The transcript submission uses a hardcoded
http://localhost:1313URL instead of theBASE_URLconstant defined at line 129-131. This will fail in non-local environments.Suggested fix
- const response = await fetch(`http://localhost:1313/submit-transcripts`, { + const response = await fetch(`${BASE_URL}/submit-transcripts`, {
🧹 Nitpick comments (3)
frontend/src/Pages/TeamDebateRoom.tsx (3)
306-309: Malformed indentation inside useEffect block.The formatting here has inconsistent indentation and an extra blank line inside the hook body, likely a merge artifact.
Suggested fix
- useEffect(() => { - - isTeam1Ref.current = isTeam1; - }, [isTeam1]); + useEffect(() => { + isTeam1Ref.current = isTeam1; + }, [isTeam1]);
691-709: UnusedgetMediacallback.The
getMediacallback is defined but never called anywhere in the component. It appears to have been superseded by the inlineensureMediaStreamfunction within the WebSocketuseEffect(lines 730-758). This is likely leftover code from the merge.Suggested fix: Remove unused callback
- const getMedia = useCallback(async () => { - try { - const stream = await navigator.mediaDevices.getUserMedia({ - video: { width: 1280, height: 720 }, - audio: true, - }); - localStreamRef.current = stream; - - const currentUserId = currentUserIdRef.current || ""; - const localVideo = localVideoRefs.current.get(currentUserId); - if (localVideo) { - localVideo.srcObject = stream; - } - } catch (err) { - setMediaError( - "Failed to access camera/microphone. Please check permissions." - ); - } - }, [setMediaError]);
1434-1434: Stray whitespace lines.Lines 1434 and 1573 contain only whitespace, likely remnants from merge conflict resolution. Consider removing these for cleaner code.
Also applies to: 1573-1573
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/Pages/Game.tsxfrontend/src/Pages/MatchLogs.tsxfrontend/src/Pages/TeamBuilder.tsxfrontend/src/Pages/TeamDebateRoom.tsx
💤 Files with no reviewable changes (3)
- frontend/src/Pages/MatchLogs.tsx
- frontend/src/Pages/TeamBuilder.tsx
- frontend/src/Pages/Game.tsx
🔇 Additional comments (1)
frontend/src/Pages/TeamDebateRoom.tsx (1)
1-2210: No remaining merge conflict markers found.The file contains no
<<<<<<<,=======, or>>>>>>>markers. The JSX and TypeScript syntax is valid. The PR objective of removing merge conflict markers has been achieved, though some duplicate code artifacts remain as noted above.
|
Thanks for the review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/Pages/OnlineDebateRoom.tsxfrontend/src/Pages/TeamDebateRoom.tsx
🔇 Additional comments (2)
frontend/src/Pages/TeamDebateRoom.tsx (1)
129-131: LGTM: Proper fallback for BASE_URL.This implementation correctly provides a fallback to
window.location.originwhenVITE_BASE_URLis not set, and strips trailing slashes. This pattern should be adopted inOnlineDebateRoom.tsxas well (see my comment on that file).frontend/src/Pages/OnlineDebateRoom.tsx (1)
139-140: Add fallback for BASE_URL and fix WebSocket URL protocol conversion.Two critical issues:
Missing fallback:
BASE_URLwill beundefinedifVITE_BASE_URLis not set, causing runtime errors in all fetch calls (lines 513, 600, 872, 906).Incorrect WebSocket protocol: The replacement
/^http/to"ws"converts bothhttp://andhttps://tows://, buthttps://should map towss://(secure WebSocket). This breaks WebSocket connections on HTTPS sites.🔎 Proposed fix
-const BASE_URL = import.meta.env.VITE_BASE_URL; -const WS_BASE_URL = import.meta.env.VITE_BASE_URL?.replace(/^http/, "ws"); +const BASE_URL = import.meta.env.VITE_BASE_URL || window.location.origin; +const WS_BASE_URL = BASE_URL.replace(/^https?/, (match) => match === 'https' ? 'wss' : 'ws');This matches the pattern used in
TeamDebateRoom.tsx(lines 129-131) and correctly handles both HTTP and HTTPS protocols.Likely an incorrect or invalid review comment.
|
Thanks for the detailed review! |
Summary
This PR resolves leftover merge conflict markers in frontend pages that were causing Vite/React build failures.
Changes
<<<<<<<,=======,>>>>>>>)Game.tsxMatchLogs.tsxTeamBuilder.tsxTeamDebateRoom.tsxResult
Notes
No functional logic changes were introduced — this PR strictly fixes merge-related issues.
Summary by CodeRabbit
Bug Fixes
Refactor
Chore
✏️ Tip: You can customize this high-level summary in your review settings.