-
Notifications
You must be signed in to change notification settings - Fork 160
Debate module does not respect global dark/light theme #272
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?
Debate module does not respect global dark/light theme #272
Conversation
📝 WalkthroughWalkthroughAdds EloChange tracking to debate transcripts and surfaces per-debate Elo deltas in user profiles; expands SaveDebateTranscript signature and persistence; reorganizes frontend routing and applies dark/high-contrast theming; adds judgment re-entrancy guard and parsing improvements; minor UI and component state refinements. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as TranscriptService
participant DB as MongoDB
Client->>Service: Submit debate result (userID, messages, transcripts, ratingChange)
Note over Service: Prepare SavedDebateTranscript including EloChange = ratingChange
Service->>DB: Upsert saved_debate_transcripts (match by userID, debate key)\nset EloChange and other fields
DB-->>Service: Upsert result
Service-->>Client: Acknowledge save / success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
frontend/src/components/ui/select.tsx (1)
115-136: Configure thehigh-contrast:variant in Tailwind config.The
high-contrast:utilities used in this component require anaddVariantdefinition intailwind.config.js. While the.high-contrastclass with CSS variables exists inindex.css, Tailwind will not recognize thehigh-contrast:prefix without explicit variant configuration. Add the following to./frontend/tailwind.config.jsin thepluginsarray orthemesection:addVariant('high-contrast', '.high-contrast &')Without this, the utilities
high-contrast:data-[highlighted]:bg-[hsl(var(--hc-hover-bg))]andhigh-contrast:data-[highlighted]:text-[hsl(var(--hc-hover-text))]will not be compiled and the high-contrast styling will not work.frontend/src/Pages/DebateRoom.tsx (6)
711-714: Text colors lack dark mode variants.The container has dark mode background styling, but the text inside uses hardcoded light-mode colors (
text-gray-900,text-gray-700) which will have poor contrast against dark backgrounds.🎨 Proposed fix
- <h1 className="text-3xl font-bold text-gray-900 tracking-tight"> + <h1 className="text-3xl font-bold text-gray-900 dark:text-gray-100 tracking-tight"> Debate: {debateData.topic} </h1> - <p className="mt-2 text-sm text-gray-700"> + <p className="mt-2 text-sm text-gray-700 dark:text-gray-300">
738-748: Popup text colors need dark mode variants.The popup container has dark mode styling but internal text uses light-mode-only colors.
🎨 Proposed fix
- <h2 className="text-xl font-semibold text-gray-800"> + <h2 className="text-xl font-semibold text-gray-800 dark:text-gray-100"> {popup.message} </h2> ... - <p className="text-gray-700 text-center text-sm"> + <p className="text-gray-700 dark:text-gray-300 text-center text-sm">
818-821: User panel is missing dark mode styling.The bot panel (line 774) includes
dark:bg-zinc-900 dark:border-zinc-700, but the user panel here only has light mode classes. This will cause the user section to remain white in dark mode, creating an inconsistent appearance.🎨 Proposed fix
<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`} >
775-791: Panel headers lack dark mode styling.Both bot and user panel headers use
bg-gray-50and varioustext-gray-*classes without dark mode variants. These elements will have poor contrast in dark mode.🎨 Proposed fix for bot panel header (apply similar pattern to user panel)
- <div className="p-2 bg-gray-50 flex items-center gap-2"> + <div className="p-2 bg-gray-50 dark:bg-zinc-800 flex items-center gap-2"> ... - <div className="text-sm font-medium text-gray-800"> + <div className="text-sm font-medium text-gray-800 dark:text-gray-100"> {debateData.botName} </div> - <div className="text-xs text-gray-500">{bot.desc}</div> - <div className="text-xs text-gray-500"> + <div className="text-xs text-gray-500 dark:text-gray-400">{bot.desc}</div> + <div className="text-xs text-gray-500 dark:text-gray-400">Also applies to: 823-841
683-694: Message bubbles need dark mode styling.The debate messages rendered here use light-mode-only colors. Since messages are the core content of the debate room, this is a significant gap in the dark mode implementation.
🎨 Proposed fix
<div key={idx} - className="p-3 bg-gray-50 rounded-lg shadow-sm text-gray-800 break-words" + className="p-3 bg-gray-50 dark:bg-zinc-800 rounded-lg shadow-sm text-gray-800 dark:text-gray-100 break-words" > - <span className="text-xs text-gray-500 block mb-1"> + <span className="text-xs text-gray-500 dark:text-gray-400 block mb-1"> {msg.phase} </span>
670-673: Timer text needs dark mode variant.🎨 Proposed fix
className={`font-mono ${ - seconds <= 5 ? "text-red-500 animate-pulse" : "text-gray-600" + seconds <= 5 ? "text-red-500 animate-pulse" : "text-gray-600 dark:text-gray-300" }`}backend/services/transcriptservice.go (1)
179-205: Fix undefinedforRecordand missingeloChangeargument (build break).Line 189 references
forRecordwhich isn’t declared, and theSaveDebateTranscriptcall at Lines 195‑204 omits the neweloChangeparameter. This won’t compile and also saves transcripts before rating changes are computed.🐛 Proposed fix (compute rating changes before saving transcripts)
- // Save transcript for "for" user - err = SaveDebateTranscript( - forUser.ID, - forUser.Email, - "user_vs_user", - topic, - againstUser.Email, - resultFor, - []models.Message{}, // You might want to reconstruct messages from transcripts - forSubmission.Transcripts, - forRecord.RatingChange - ) - if err != nil { - } - - // Save transcript for "against" user - err = SaveDebateTranscript( - againstUser.ID, - againstUser.Email, - "user_vs_user", - topic, - forUser.Email, - resultAgainst, - []models.Message{}, // You might want to reconstruct messages from transcripts - againstSubmission.Transcripts, - ) - if err != nil { - } - - // Update ratings based on the result + // Update ratings based on the result (so EloChange is available) outcomeFor := 0.5 switch strings.ToLower(resultFor) { case "win": outcomeFor = 1.0 case "loss": outcomeFor = 0.0 } - debateRecord, opponentRecord, ratingErr := UpdateRatings(forUser.ID, againstUser.ID, outcomeFor, time.Now()) - if ratingErr != nil { - } else { + ratingChangeFor, ratingChangeAgainst := 0.0, 0.0 + debateRecord, opponentRecord, ratingErr := UpdateRatings(forUser.ID, againstUser.ID, outcomeFor, time.Now()) + if ratingErr != nil { + } else { + ratingChangeFor = debateRecord.RatingChange + ratingChangeAgainst = opponentRecord.RatingChange debateRecord.Topic = topic debateRecord.Result = resultFor opponentRecord.Topic = topic opponentRecord.Result = resultAgainst @@ ratingSummary = map[string]interface{}{ "for": map[string]float64{ "rating": debateRecord.PostRating, "change": debateRecord.RatingChange, }, "against": map[string]float64{ "rating": opponentRecord.PostRating, "change": opponentRecord.RatingChange, }, } } + + // Save transcript for "for" user + err = SaveDebateTranscript( + forUser.ID, + forUser.Email, + "user_vs_user", + topic, + againstUser.Email, + resultFor, + []models.Message{}, // You might want to reconstruct messages from transcripts + forSubmission.Transcripts, + ratingChangeFor, + ) + if err != nil { + } + + // Save transcript for "against" user + err = SaveDebateTranscript( + againstUser.ID, + againstUser.Email, + "user_vs_user", + topic, + forUser.Email, + resultAgainst, + []models.Message{}, // You might want to reconstruct messages from transcripts + againstSubmission.Transcripts, + ratingChangeAgainst, + ) + if err != nil { + }To confirm there are no other call sites still missing the new
eloChangeparameter (and to ensureforRecordisn’t defined elsewhere), you can run:#!/bin/bash # List SaveDebateTranscript call sites and any lingering forRecord usage. rg -n --type=go 'SaveDebateTranscript\s*\(' rg -n --type=go '\bforRecord\b'
🤖 Fix all issues with AI agents
In `@frontend/src/App.tsx`:
- Around line 53-100: CoachPage is missing react-router's Outlet so nested
routes (paths "strengthen-argument" and "pros-cons" defined under Route
path="coach") never render; fix by importing Outlet from "react-router-dom" in
the CoachPage component and rendering <Outlet /> at the appropriate place inside
CoachPage (e.g., after the hero/header or in the main content area) so child
routes like StrengthenArgument and ProsConsChallenge can mount.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 603-609: The regex-based fixer using jsonString -> fixedJson (in
the block that builds fixedJson and then JSON.parse into judgment) is unsafe
because the (\w+): replacement can alter colons inside string values; replace
this with a safer strategy: either use a lenient parser like json5 (install and
call JSON5.parse(jsonString) instead of manual replacements) or implement a
targeted key-quoting routine that only quotes unquoted keys when they occur as
object keys (e.g., detect keys after { or , and before :) and avoid touching
content inside string literals; then parse into judgment. Use the symbols
jsonString, fixedJson, and judgment to locate and update the code.
🧹 Nitpick comments (1)
backend/controllers/profile_controller.go (1)
257-257: Remove stale TODO now thateloChangeis wired.Line 257 already pulls
transcript.EloChange, so the TODO is outdated.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
frontend/src/Pages/DebateRoom.tsx (5)
726-748: Incomplete dark mode styling inside popup.The popup container has dark mode classes, but the inner text elements still use light-only colors (
text-gray-800,text-gray-700,text-orange-600) which will have poor contrast in dark mode.Also, line 731 has a typo:
border-blue500should beborder-blue-500.🔧 Suggested fix
- <div className="animate-spin rounded-full h-16 w-16 border-t-4 border-blue500 mb-4"></div> - <h2 className="text-xl font-semibold text-gray-800"> + <div className="animate-spin rounded-full h-16 w-16 border-t-4 border-blue-500 mb-4"></div> + <h2 className="text-xl font-semibold text-gray-800 dark:text-gray-100"> {popup.message} </h2> ... - <h3 className="text-xl font-bold text-orange-600 mb-2"> + <h3 className="text-xl font-bold text-orange-600 dark:text-orange-400 mb-2"> Phase Transition </h3> - <p className="text-gray-700 text-center text-sm"> + <p className="text-gray-700 dark:text-gray-300 text-center text-sm"> {popup.message} </p>
768-795: Incomplete dark mode styling in bot section.The container has dark mode classes, but inner elements are missing dark variants:
- Line 769:
bg-gray-50needsdark:bg-zinc-800- Line 778:
text-gray-800needsdark:text-gray-100- Line 781-783:
text-gray-500needsdark:text-gray-400🔧 Suggested fix for header
- <div className="p-2 bg-gray-50 flex items-center gap-2"> + <div className="p-2 bg-gray-50 dark:bg-zinc-800 flex items-center gap-2"> ... - <div className="text-sm font-medium text-gray-800"> + <div className="text-sm font-medium text-gray-800 dark:text-gray-100"> {debateData.botName} </div> - <div className="text-xs text-gray-500">{bot.desc}</div> - <div className="text-xs text-gray-500"> + <div className="text-xs text-gray-500 dark:text-gray-400">{bot.desc}</div> + <div className="text-xs text-gray-500 dark:text-gray-400">
812-815: User section missing dark mode classes.The bot section (line 768) has
dark:bg-zinc-900 dark:border-zinc-700, but the user section is missing these dark mode classes, causing an inconsistent appearance in dark mode.🔧 Suggested fix
<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`} >
817-844: User section header also missing dark mode variants.Same issue as the bot section—the header background and text colors need dark mode variants for consistency.
🔧 Suggested fix
- <div className="p-2 bg-gray-50 flex items-center gap-2"> + <div className="p-2 bg-gray-50 dark:bg-zinc-800 flex items-center gap-2"> ... - <div className="text-sm font-medium text-gray-800"> + <div className="text-sm font-medium text-gray-800 dark:text-gray-100"> {user?.displayName || "You"} </div> - <div className="text-xs text-gray-500"> + <div className="text-xs text-gray-500 dark:text-gray-400"> {user?.bio || "Debater"} </div> - <div className="text-xs text-gray-500"> + <div className="text-xs text-gray-500 dark:text-gray-400">
674-692: Message boxes missing dark mode styling.The
renderPhaseMessagesfunction renders messages with light-only colors. In dark mode,bg-gray-50andtext-gray-800will have poor contrast against the dark container background.🔧 Suggested fix
<div key={idx} - className="p-3 bg-gray-50 rounded-lg shadow-sm text-gray-800 break-words" + className="p-3 bg-gray-50 dark:bg-zinc-800 rounded-lg shadow-sm text-gray-800 dark:text-gray-100 break-words" > - <span className="text-xs text-gray-500 block mb-1"> + <span className="text-xs text-gray-500 dark:text-gray-400 block mb-1"> {msg.phase} </span>
🤖 Fix all issues with AI agents
In `@frontend/src/Pages/DebateRoom.tsx`:
- Line 10: The file DebateRoom.tsx imports JSON5 (import JSON5 from "json5") but
the frontend project's package.json is missing the json5 dependency; add "json5"
to the frontend package.json dependencies (or run the package manager: e.g., npm
install --save json5 or yarn add json5) and then install to ensure JSON5 is
available at runtime.
🧹 Nitpick comments (1)
frontend/src/Pages/DebateRoom.tsx (1)
601-604: Minor formatting: missing space beforecatch.🔧 Suggested fix
- }catch (e) { + } catch (e) {
This PR fixes an issue where all debate rooms (bot mode and matchmaking chatroom) always rendered in light mode, even when global dark mode was enabled.
The rest of the application already correctly consumes the global ThemeProvider, but the debate module was overriding it with hard-coded light styles.
close #271
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX
✏️ Tip: You can customize this high-level summary in your review settings.