Join Room button turns green when room code is entered#343
Open
Abhishek2005-ard wants to merge 24 commits into
Open
Join Room button turns green when room code is entered#343Abhishek2005-ard wants to merge 24 commits into
Abhishek2005-ard wants to merge 24 commits into
Conversation
… initialized via useState)
…anted restart loop
📝 WalkthroughWalkthroughAdds EloChange tracking to debate transcripts (model, marshal, service, and controller call sites), updates profile to surface stored eloChange, introduces admin-protected routing, improves speech-recognition robustness, tweaks UI styling and high-contrast support, enables a disabled Join button when room code is empty, and switches DebateRoom judgment parsing to JSON5. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
%% Actors
participant Client as Client (frontend)
participant Controller as Controller (backend)
participant Service as TranscriptService
participant DB as Database
Client->>Controller: Submit transcript + rating changes
Controller->>Service: SaveDebateTranscript(..., eloChange)
Service->>DB: Upsert SavedDebateTranscript (includes EloChange)
DB-->>Service: OK
Service-->>Controller: OK / error
Controller-->>Client: Response (success/failure)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
frontend/src/components/Matchmaking.tsx (1)
27-28: Inconsistent with PR objectives: unrelated whitespace-only change.The PR objectives state this PR should modify
frontend/src/components/DebatePopup.tsxto add conditional styling for the Join Room button based onroomCodestate. However, this file (Matchmaking.tsx) contains only whitespace formatting changes (two blank lines) and is unrelated to the Join Room feature.Additionally, the high-level AI summary mentions "addresses the room code validation issue by disabling the join button when no valid code is entered," but this file contains no such logic.
Recommendation: Consider whether this whitespace-only change should be part of this PR, or if it was included unintentionally. If the Join Room button changes are in
DebatePopup.tsx, ensure that file is included in the review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Matchmaking.tsx` around lines 27 - 28, This change introduces only whitespace in the Matchmaking component which is unrelated to the PR objective of adding conditional styling to the Join Room button; revert the whitespace-only edits in the Matchmaking component (Matchmaking) and ensure the actual functional changes are present in the DebatePopup component logic that manages roomCode and the Join Room button (e.g., the roomCode state and the button render/disabled/class logic in DebatePopup). Remove the accidental whitespace-only commit from this PR or move it to a separate formatting-only PR, and confirm DebatePopup contains the conditional styling/disabled behavior for the Join Room button based on roomCode before marking this PR ready.frontend/src/App.tsx (1)
111-112: Minor: Inconsistent closing tag indentation.The
</Routes>closing tag appears to be misaligned with its opening tag.♻️ Suggested fix
<Route path="*" element={<Navigate to="/" replace />} /> - </Routes> + </Routes>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/App.tsx` around lines 111 - 112, The closing </Routes> tag in App.tsx is misindented relative to its opening <Routes> block; fix by aligning the </Routes> indentation to match the opening <Routes> tag so the <Route path="*" element={<Navigate to="/" replace />} /> line and the </Routes> tag share the same indentation level, ensuring consistent JSX formatting for the Routes, Route and Navigate elements.frontend/src/utils/speechTest.ts (1)
62-72: Auto-restart timeout stored but type may cause issues on cleanup.The
restartTimeoutis typed asnumber | null, which is correct forwindow.setTimeout. However, if this code runs in a Node.js environment (e.g., SSR or tests),setTimeoutreturns aTimeoutobject, not a number. Consider usingReturnType<typeof setTimeout>for cross-environment compatibility.♻️ Optional fix for type safety
- private restartTimeout: number | null = null; + private restartTimeout: ReturnType<typeof setTimeout> | null = null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/utils/speechTest.ts` around lines 62 - 72, The restartTimeout field currently typed as number | null can break in Node/SSR where setTimeout returns a Timeout object; change the type of restartTimeout to ReturnType<typeof setTimeout> | null (and update any uses like clearTimeout(this.restartTimeout) to accept that type) so the window.setTimeout call and cleanup work across environments; update the declaration of restartTimeout and any TypeScript annotations in the class/method that uses this.restartTimeout (the block that calls this.restartTimeout = window.setTimeout(...) and the cleanup that clears it).frontend/src/Pages/DebateRoom.tsx (1)
811-814: Inconsistent dark mode styling between panels.The bot section (Line 767) includes dark mode classes (
dark:bg-zinc-900 border border-gray-200 dark:border-zinc-700), but the user section retains only light mode styling. This creates visual inconsistency in dark mode.♻️ Suggested fix to add dark mode classes to user section
<div className={`relative w-full md:w-1/2 ${ !state.isBotTurn && !state.isDebateEnded ? "animate-glow" : "" - } bg-white border border-gray-200 shadow-md h-[540px] flex flex-col`} + } bg-white dark:bg-zinc-900 border border-gray-200 dark:border-zinc-700 shadow-md h-[540px] flex flex-col`} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Pages/DebateRoom.tsx` around lines 811 - 814, The user panel div that renders when !state.isBotTurn (the JSX block with className starting "relative w-full md:w-1/2 ... bg-white border border-gray-200 shadow-md h-[540px]") lacks dark-mode classes, causing inconsistent styling with the bot panel; update that component's className (the user panel div) to mirror the bot panel by adding dark:bg-zinc-900 and dark:border-zinc-700 (and any other dark-variants used by the bot panel) so the panel background and border switch correctly in dark mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/services/transcriptservice.go`:
- Around line 181-207: The transcript saves pass a hardcoded 0.0 for Elo change
causing transcript.EloChange to remain zero; change the flow so UpdateRatings()
runs (or compute the rating deltas) before calling SaveDebateTranscript and pass
the actual deltas (use debateRecord.RatingChange for the "forUser" save and
opponentRecord.RatingChange for the "againstUser" save) into the corresponding
SaveDebateTranscript calls instead of 0.0 so transcript.EloChange reflects real
rating changes.
- Around line 667-668: SaveDebateTranscript's signature gained a trailing
eloChange parameter but two callers still pass only eight arguments; update the
call sites that invoke SaveDebateTranscript in transcript_controller and
debatevsbot_controller to supply the new eloChange float64 argument (compute and
pass the actual Elo delta available in their context or, if not applicable yet,
pass 0.0) so the calls match the function signature; ensure you reference the
SaveDebateTranscript call expressions and propagate any needed local variable or
computation that yields the eloChange value.
In `@frontend/src/components/DebatePopup.tsx`:
- Around line 128-135: The button enables based on roomCode.trim(), but
handleJoinRoom still uses the raw roomCode, causing routes like "/room/ ABC123 "
to be generated; update handleJoinRoom to compute a normalized value (e.g.,
const normalized = roomCode.trim()) and use that for validation and routing
(return early if normalized is empty, then navigate using normalized), ensuring
any route generation or API calls reference normalized instead of raw roomCode
and keeping the existing disabled logic in sync.
In `@frontend/src/components/ProtectedRoute.tsx`:
- Around line 7-13: AdminProtectedRoute is reading
localStorage.getItem("adminData") but AdminSignup.tsx stores the admin payload
under "admin", causing the guard to always redirect; update AdminProtectedRoute
to check the same key used when saving (replace
localStorage.getItem("adminData") with localStorage.getItem("admin")) or
alternatively change AdminSignup.tsx to set "adminData"—ensure the keys for
admin payload (admin or adminData) and the token (adminToken) match across
AdminSignup.tsx and AdminProtectedRoute so the check in AdminProtectedRoute
succeeds.
In `@frontend/tailwind.config.js`:
- Around line 65-68: The high-contrast Tailwind variant added via
addVariant('high-contrast', '@media (prefers-contrast: more)') assumes CSS
custom properties are available, but --hc-hover-bg and --hc-hover-text are only
defined under a .high-contrast class in frontend/src/index.css; move or
duplicate those custom property definitions into a `@media` (prefers-contrast:
more) block in frontend/src/index.css (or add both the media query and the
.high-contrast selector) so that the properties are defined when the media query
activates without requiring the .high-contrast class.
---
Nitpick comments:
In `@frontend/src/App.tsx`:
- Around line 111-112: The closing </Routes> tag in App.tsx is misindented
relative to its opening <Routes> block; fix by aligning the </Routes>
indentation to match the opening <Routes> tag so the <Route path="*"
element={<Navigate to="/" replace />} /> line and the </Routes> tag share the
same indentation level, ensuring consistent JSX formatting for the Routes, Route
and Navigate elements.
In `@frontend/src/components/Matchmaking.tsx`:
- Around line 27-28: This change introduces only whitespace in the Matchmaking
component which is unrelated to the PR objective of adding conditional styling
to the Join Room button; revert the whitespace-only edits in the Matchmaking
component (Matchmaking) and ensure the actual functional changes are present in
the DebatePopup component logic that manages roomCode and the Join Room button
(e.g., the roomCode state and the button render/disabled/class logic in
DebatePopup). Remove the accidental whitespace-only commit from this PR or move
it to a separate formatting-only PR, and confirm DebatePopup contains the
conditional styling/disabled behavior for the Join Room button based on roomCode
before marking this PR ready.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 811-814: The user panel div that renders when !state.isBotTurn
(the JSX block with className starting "relative w-full md:w-1/2 ... bg-white
border border-gray-200 shadow-md h-[540px]") lacks dark-mode classes, causing
inconsistent styling with the bot panel; update that component's className (the
user panel div) to mirror the bot panel by adding dark:bg-zinc-900 and
dark:border-zinc-700 (and any other dark-variants used by the bot panel) so the
panel background and border switch correctly in dark mode.
In `@frontend/src/utils/speechTest.ts`:
- Around line 62-72: The restartTimeout field currently typed as number | null
can break in Node/SSR where setTimeout returns a Timeout object; change the type
of restartTimeout to ReturnType<typeof setTimeout> | null (and update any uses
like clearTimeout(this.restartTimeout) to accept that type) so the
window.setTimeout call and cleanup work across environments; update the
declaration of restartTimeout and any TypeScript annotations in the class/method
that uses this.restartTimeout (the block that calls this.restartTimeout =
window.setTimeout(...) and the cleanup that clears it).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 03cae5e6-0e79-4b79-a094-152e03833d6d
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
backend/controllers/debatevsbot_controller.gobackend/controllers/profile_controller.gobackend/models/transcript.gobackend/services/transcriptservice.gofrontend/package.jsonfrontend/src/App.tsxfrontend/src/Pages/About.tsxfrontend/src/Pages/DebateRoom.tsxfrontend/src/Pages/StartDebate.tsxfrontend/src/Pages/TeamDebateRoom.tsxfrontend/src/components/DebatePopup.tsxfrontend/src/components/Header.tsxfrontend/src/components/JudgementPopup.tsxfrontend/src/components/Matchmaking.tsxfrontend/src/components/ProtectedRoute.tsxfrontend/src/components/ui/select.tsxfrontend/src/index.cssfrontend/src/utils/speechTest.tsfrontend/tailwind.config.js
…admin route protection
There was a problem hiding this comment.
♻️ Duplicate comments (1)
backend/services/transcriptservice.go (1)
181-207:⚠️ Potential issue | 🔴 CriticalCompute ratings before these transcript saves.
Lines 190 and 206 read
debateRecord/opponentRecordbefore Line 222 creates them, and Line 197’serr = err = ...is invalid Go syntax. MoveUpdateRatings(...)ahead of bothSaveDebateTranscript(...)calls, then pass the returnedRatingChangevalues into each save.Proposed fix
- // Save transcript for "for" user - err = SaveDebateTranscript( - forUser.ID, - forUser.Email, - "user_vs_user", - topic, - againstUser.Email, - resultFor, - []models.Message{}, - forSubmission.Transcripts, - debateRecord.RatingChange, - ) - if err != nil { - log.Println("Error saving transcript for user:", err) - } - - // Save transcript for "against" user - err = err = SaveDebateTranscript( - againstUser.ID, - againstUser.Email, - "user_vs_user", - topic, - forUser.Email, - resultAgainst, - []models.Message{}, - againstSubmission.Transcripts, - opponentRecord.RatingChange, - ) - if err != nil { - log.Println("Error saving transcript for opponent:", err) - } - // Update ratings based on the result outcomeFor := 0.5 switch strings.ToLower(resultFor) { case "win": outcomeFor = 1.0 @@ debateRecord, opponentRecord, ratingErr := UpdateRatings(forUser.ID, againstUser.ID, outcomeFor, time.Now()) if ratingErr != nil { } else { + err = SaveDebateTranscript( + forUser.ID, + forUser.Email, + "user_vs_user", + topic, + againstUser.Email, + resultFor, + []models.Message{}, + forSubmission.Transcripts, + debateRecord.RatingChange, + ) + if err != nil { + log.Println("Error saving transcript for user:", err) + } + + err = SaveDebateTranscript( + againstUser.ID, + againstUser.Email, + "user_vs_user", + topic, + forUser.Email, + resultAgainst, + []models.Message{}, + againstSubmission.Transcripts, + opponentRecord.RatingChange, + ) + if err != nil { + log.Println("Error saving transcript for opponent:", err) + } + debateRecord.Topic = topic debateRecord.Result = resultFor opponentRecord.Topic = topic#!/bin/bash # Verify that the Elo deltas are referenced before they are declared and that the # second save call contains invalid syntax. rg -n -C2 'debateRecord\.RatingChange|opponentRecord\.RatingChange|err = err = SaveDebateTranscript|UpdateRatings\(' backend/services/transcriptservice.go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/transcriptservice.go` around lines 181 - 207, The code calls SaveDebateTranscript using debateRecord.RatingChange and opponentRecord.RatingChange before those rating changes are computed and also has an invalid assignment "err = err = SaveDebateTranscript"; move the UpdateRatings(...) call (the function that returns debateRecord and opponentRecord or their RatingChange values) to run before both SaveDebateTranscript(...) calls, capture its returned RatingChange values (e.g., debateDelta, opponentDelta) and pass those into the two SaveDebateTranscript calls instead of debateRecord.RatingChange/opponentRecord.RatingChange, and fix the duplicated assignment by using a single "err =" when assigning the SaveDebateTranscript result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@backend/services/transcriptservice.go`:
- Around line 181-207: The code calls SaveDebateTranscript using
debateRecord.RatingChange and opponentRecord.RatingChange before those rating
changes are computed and also has an invalid assignment "err = err =
SaveDebateTranscript"; move the UpdateRatings(...) call (the function that
returns debateRecord and opponentRecord or their RatingChange values) to run
before both SaveDebateTranscript(...) calls, capture its returned RatingChange
values (e.g., debateDelta, opponentDelta) and pass those into the two
SaveDebateTranscript calls instead of
debateRecord.RatingChange/opponentRecord.RatingChange, and fix the duplicated
assignment by using a single "err =" when assigning the SaveDebateTranscript
result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b733582-d787-4a11-99a6-ae6aed74dd7d
📒 Files selected for processing (4)
backend/controllers/debatevsbot_controller.gobackend/controllers/transcript_controller.gobackend/services/transcriptservice.gofrontend/src/index.css
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/index.css
- backend/controllers/debatevsbot_controller.go
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Improves the Join Room UI by changing the button color to green when a room code is entered. This gives clear visual feedback to users that they can now join the room.
Problem
Previously, the Join Room button remained in the default grey color even after entering a room code.
This made it unclear whether the action was available or if the input was detected.
Solution
Added conditional styling to the Join Room button based on the
roomCodestate.When
roomCodeis empty → button remains greyWhen
roomCodeis entered → button turns greenThis improves the user experience and provides clear visual feedback.
Changes
Updated
frontend/src/components/DebatePopup.tsxAdded conditional button styling based on
roomCodeBefore Fix
The Join Room button stays grey even after entering a room code.
After Fix
The Join Room button turns green when a room code is entered.
Impact
Improves UI clarity
Better user feedback
Small frontend-only change
close #342
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
UI/Style