Implement Unified DebateAI Branding, AOSSIE Interactive Logo & UI Micro-Interactions#358
Open
Abhishek2005-ard wants to merge 16 commits into
Open
Implement Unified DebateAI Branding, AOSSIE Interactive Logo & UI Micro-Interactions#358Abhishek2005-ard wants to merge 16 commits into
Abhishek2005-ard wants to merge 16 commits into
Conversation
📝 WalkthroughWalkthroughThis PR removes the production sample config, adds EloChange to saved debate transcripts (model, service, marshal), updates SaveDebateTranscript signature and call sites, surfaces EloChange in profile responses, restructures frontend routes and theming (dark mode, high-contrast), adds JSON5 parsing and concurrency guard in judging, and several UI/branding tweaks. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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)
📝 Coding Plan
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 Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/src/components/Header.tsx (1)
266-270:⚠️ Potential issue | 🟠 MajorHeader logo in mobile drawer is still non-clickable.
Line 266-270 renders only an image, so AOSSIE discoverability is missing in this header path. Wrap it in an external link like Sidebar.
Suggested fix
- <img - src={debateAiLogo} - alt="DebateAI Logo" - className="h-8 w-auto object-contain" - /> + <a + href="https://aossie.org" + target="_blank" + rel="noopener noreferrer" + aria-label="Visit AOSSIE website" + > + <img + src={debateAiLogo} + alt="AOSSIE logo" + className="h-8 w-auto object-contain" + /> + </a>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Header.tsx` around lines 266 - 270, The header's mobile-drawer logo is rendered as a plain img (debateAiLogo) inside the Header component, making it non-clickable; wrap this <img> element in the same external link used in the Sidebar (i.e., an anchor tag that points to the AOSSIE/external URL) so the logo becomes clickable in the mobile drawer—update the JSX in Header.tsx to wrap the img with the external link component/anchor used elsewhere (matching the Sidebar implementation) and ensure alt text and className are preserved.backend/controllers/transcript_controller.go (1)
32-39:⚠️ Potential issue | 🟠 MajorThis route cannot persist nonzero Elo deltas.
SaveDebateTranscriptnow supportseloChange, butSaveTranscriptRequesthas no corresponding input and Line 118 hardcodes0. Any debate with a real rating change saved through this handler will lose that data unless the value is derived server-side here.Also applies to: 109-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/controllers/transcript_controller.go` around lines 32 - 39, The request struct SaveTranscriptRequest is missing an eloChange field and the controller call to SaveDebateTranscript uses a hardcoded 0; add an EloChange int (or *int if you want nullable) with the JSON tag "eloChange" to SaveTranscriptRequest and change the call site in the handler (where SaveDebateTranscript is invoked and currently passing 0) to pass req.EloChange (or deref with default 0 if nullable) so incoming elo deltas are persisted.backend/services/transcriptservice.go (1)
180-190:⚠️ Potential issue | 🟠 MajorPersist the real Elo change, not the placeholder.
Lines 180-205 save both human-vs-human transcripts with
eloChange = 0, but the actual deltas are only known afterUpdateRatingsat Lines 218-239. The update gate at Lines 686-694 also ignoreseloChange, so those laterRatingChangevalues never backfill the saved records and transcript history will keep reporting0.Move the transcript saves below
UpdateRatings, or letexistingTranscript.EloChange != eloChangetrigger the update path as well.Also applies to: 195-205, 218-239, 686-694
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/transcriptservice.go` around lines 180 - 190, The transcript save is persisting eloChange as 0 before ratings are computed; move the calls to SaveDebateTranscript (and the similar block that writes human-vs-human transcripts) to after UpdateRatings so the real RatingChange/eloChange is available, or modify the save/update logic to detect when existingTranscript.EloChange != eloChange and call the update path to backfill RatingChange; update the code paths that reference UpdateRatings, SaveDebateTranscript, existingTranscript.EloChange and RatingChange so saved transcripts always include the post-UpdateRatings elo delta.backend/models/transcript.go (1)
41-54:⚠️ Potential issue | 🟡 MinorDon't serialize zero ObjectIDs as valid-looking IDs.
Lines 51–52 unconditionally call
Hex()onIDandUserID. When these ObjectIDs are unset, this produces000000000000000000000000, which appears as a valid ID in the JSON response. The struct tags on these fields includeomitempty, indicating the intent to exclude them when unset. Guard withIsZero()and use pointer fields withomitemptyto respect that intent.🛠️ Suggested fix
func (s SavedDebateTranscript) MarshalJSON() ([]byte, error) { type Alias SavedDebateTranscript a := Alias(s) a.ID = primitive.NilObjectID a.UserID = primitive.NilObjectID - return json.Marshal(&struct { - ID string `json:"id"` - UserID string `json:"userId"` - Alias - }{ - ID: s.ID.Hex(), - UserID: s.UserID.Hex(), - Alias: a, - }) + payload := struct { + ID *string `json:"id,omitempty"` + UserID *string `json:"userId,omitempty"` + Alias + }{ + Alias: a, + } + if !s.ID.IsZero() { + id := s.ID.Hex() + payload.ID = &id + } + if !s.UserID.IsZero() { + userID := s.UserID.Hex() + payload.UserID = &userID + } + return json.Marshal(&payload) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/models/transcript.go` around lines 41 - 54, In MarshalJSON for SavedDebateTranscript, avoid emitting zero ObjectIDs as hex strings: check s.ID.IsZero() and s.UserID.IsZero() and only populate the JSON fields when they are set; change the anonymous struct's ID and UserID fields to pointer-to-string (e.g., *string) with `omitempty` so they are omitted when nil, and set them to pointers to s.ID.Hex()/s.UserID.Hex() only when the corresponding IsZero() is false (preserve use of Alias and the existing a := Alias(s) pattern).
🧹 Nitpick comments (3)
frontend/src/components/Matchmaking.tsx (2)
38-42: Consider whether this useEffect is necessary.This effect sets
isInPooltofalseandwaitTimeto0on mount, but both values are already initialized to those exact values in theiruseStatedeclarations (lines 32-33). Additionally, the backend removes users from the matchmaking pool on WebSocket disconnect (as shown in thereadPump()cleanup), and the existingpool_updatehandler (lines 91-97) correctly restoresisInPoolbased onstartedMatchmakingfrom server state.If this is defensive coding for a specific edge case (e.g., React Fast Refresh preserving state in development), consider adding a comment explaining the rationale. Otherwise, this effect is redundant.
🤖 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 38 - 42, The useEffect that calls setIsInPool(false) and setWaitTime(0) on mount is redundant because isInPool and waitTime are already initialized to those values and server handlers (readPump cleanup and the pool_update handler) restore state; either remove this useEffect entirely or keep it but add a concise comment explaining it's defensive for React Fast Refresh/state-preservation in development. Locate the effect in Matchmaking.tsx (the useEffect that invokes setIsInPool and setWaitTime) and implement one of those two options, ensuring pool_update and readPump logic remain unchanged.
38-42: Indentation inconsistency.This code block uses 4-space indentation while the rest of the component uses 2-space indentation (see lines 30-36, 45+). Please fix the indentation to maintain consistency.
🔧 Proposed fix
- // Reset matchmaking UI state on page refresh - useEffect(() => { - setIsInPool(false); - setWaitTime(0); - }, []); + // Reset matchmaking UI state on page refresh + useEffect(() => { + setIsInPool(false); + setWaitTime(0); + }, []);🤖 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 38 - 42, The useEffect block in Matchmaking.tsx is indented with 4 spaces while the rest of the file uses 2-space indentation; reformat the block containing useEffect, setIsInPool(false), and setWaitTime(0) to use 2-space indentation to match the surrounding code (preserve the same logic and empty dependency array).frontend/src/App.tsx (1)
62-63: Consider adding router-level auth guard for/admin/dashboardas a defense-in-depth measure.While
AdminDashboarddoes enforce authentication internally (checksadminTokenin localStorage on mount and redirects to login if missing), adding a router-levelProtectedRoutewrapper would follow security best practices and prevent unauthorized components from momentarily rendering. This is an optional architectural improvement rather than a critical fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/App.tsx` around lines 62 - 63, Wrap the /admin/dashboard route with a router-level guard by replacing the element using AdminDashboard with a ProtectedRoute that checks admin auth before rendering; create or reuse a ProtectedRoute component (e.g., ProtectedRoute) that verifies the same adminToken logic (or a shared auth helper) and renders <AdminDashboard /> only when authorized otherwise navigates to /admin/login, and update the Route declaration that currently references AdminDashboard so the guard prevents transient rendering of the dashboard component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/Sidebar.tsx`:
- Around line 23-29: The logo link's accessible name is misleading—update the
anchor/img to clearly convey the destination for screen readers: either set the
img alt to an explicit label like "AOSSIE website" or add an aria-label on the
anchor (e.g., aria-label="Visit AOSSIE website (opens in new tab)") while
keeping the visible logo unchanged; adjust the <img> alt and/or the <a>
aria-label in Sidebar.tsx (the <a> wrapping debateAiLogo and the img element) so
the external link purpose is clear.
In `@frontend/src/components/ui/select.tsx`:
- Around line 121-122: The Tailwind variant "high-contrast" used in
frontend/src/components/ui/select.tsx (e.g., data-[highlighted] rules) isn't
registered, so those utility classes won't be generated; update
frontend/tailwind.config.js to register the variant (call
addVariant('high-contrast', '.high-contrast &') or equivalent) so Tailwind emits
those rules, or alternatively change the utilities in select.tsx to use an
ancestor selector pattern supported by Tailwind (e.g., rewrite the classes to
apply under a .high-contrast ancestor) and ensure the change targets the same
selectors referenced in select.tsx.
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 588-595: The current console.log calls print full debate messages
and raw judge payloads (see usages of messages, judgeDebate, and result in
DebateRoom.tsx); replace these with production-safe logging by removing or
gating full content behind a dev-only flag (e.g., process.env.NODE_ENV ===
"development"), and when you must log, only emit non-sensitive metadata such as
message counts, userId (debateData.userId), timestamps, or a truncated/hashed
preview of content rather than the full messages or raw result payload.
- Around line 699-704: The dark-theme background additions in DebateRoom (the
top-level container div and the inner rounded container using classes like
"min-h-screen p-4 bg-gradient-to-br ..." and "rounded-xl p-4 ...
bg-gradient-to-r ...") introduce low-contrast text because existing text classes
(e.g., text-gray-900, text-gray-800, text-gray-700) aren't overridden for dark
mode; fix by adding appropriate dark-mode text utility classes (for example
dark:text-gray-100 or dark:text-neutral-200) to those container elements and any
child elements that use text-gray-900/800/700 so they get high-contrast colors
in dark mode, ensuring all instances within DebateRoom (search for
text-gray-900/800/700) are updated to include matching dark:text-* counterparts.
- Line 728: In DebateRoom.tsx replace the incorrect Tailwind utility in the
popup container's class string: locate the div with className containing
"bg-white dark:bg-zinc-900 rounded-xl p-6 ... max-w-m w-full ..." and change
"max-w-m" to the correct Tailwind utility "max-w-md" so the component
(DebateRoom) matches other popup components like TeamDebateRoom and
OnlineDebateRoom.
- Around line 223-226: Guard against location.state being undefined by
checking/useLocation() before casting to DebateProps: update the logic around
useLocation, debateData, phases, and debateKey to handle a missing state (e.g.,
if (!location.state) then redirect via useNavigate or render a fallback/error
UI); use optional chaining when accessing phaseTimings and userId/topic/debateId
to avoid crashes and construct debateKey only when those fields exist, otherwise
short-circuit to a safe default or navigation action.
In `@frontend/src/Pages/Home.tsx`:
- Around line 93-97: The heart icon (FaHeart) in the Home component is styled to
look interactive (cursor-pointer, hover effects) but lacks accessible semantics
or keyboard interaction; either remove the pointer affordance or make it a real
control: wrap FaHeart in a <button> (or add role="button", tabIndex, onKeyDown
handling) inside the Home.tsx markup, add an appropriate aria-label like
"Support DebateAI" and implement an onClick handler (or navigation/action) so it
is keyboard-accessible and screen-reader friendly, and keep the hover/active
styles on the new interactive element.
In `@frontend/src/Pages/TeamBuilder.tsx`:
- Around line 341-342: In TeamBuilder.tsx, replace the hardcoded utility class
"text-gray-700" on the subtitle <p> element with the theme token class (e.g.,
"text-muted-foreground" or "text-foreground") to ensure proper contrast in dark
mode and consistency with the app theme; locate the <p className="text-lg
text-gray-700"> in the TeamBuilder component and swap the color utility to the
chosen token.
---
Outside diff comments:
In `@backend/controllers/transcript_controller.go`:
- Around line 32-39: The request struct SaveTranscriptRequest is missing an
eloChange field and the controller call to SaveDebateTranscript uses a hardcoded
0; add an EloChange int (or *int if you want nullable) with the JSON tag
"eloChange" to SaveTranscriptRequest and change the call site in the handler
(where SaveDebateTranscript is invoked and currently passing 0) to pass
req.EloChange (or deref with default 0 if nullable) so incoming elo deltas are
persisted.
In `@backend/models/transcript.go`:
- Around line 41-54: In MarshalJSON for SavedDebateTranscript, avoid emitting
zero ObjectIDs as hex strings: check s.ID.IsZero() and s.UserID.IsZero() and
only populate the JSON fields when they are set; change the anonymous struct's
ID and UserID fields to pointer-to-string (e.g., *string) with `omitempty` so
they are omitted when nil, and set them to pointers to s.ID.Hex()/s.UserID.Hex()
only when the corresponding IsZero() is false (preserve use of Alias and the
existing a := Alias(s) pattern).
In `@backend/services/transcriptservice.go`:
- Around line 180-190: The transcript save is persisting eloChange as 0 before
ratings are computed; move the calls to SaveDebateTranscript (and the similar
block that writes human-vs-human transcripts) to after UpdateRatings so the real
RatingChange/eloChange is available, or modify the save/update logic to detect
when existingTranscript.EloChange != eloChange and call the update path to
backfill RatingChange; update the code paths that reference UpdateRatings,
SaveDebateTranscript, existingTranscript.EloChange and RatingChange so saved
transcripts always include the post-UpdateRatings elo delta.
In `@frontend/src/components/Header.tsx`:
- Around line 266-270: The header's mobile-drawer logo is rendered as a plain
img (debateAiLogo) inside the Header component, making it non-clickable; wrap
this <img> element in the same external link used in the Sidebar (i.e., an
anchor tag that points to the AOSSIE/external URL) so the logo becomes clickable
in the mobile drawer—update the JSX in Header.tsx to wrap the img with the
external link component/anchor used elsewhere (matching the Sidebar
implementation) and ensure alt text and className are preserved.
---
Nitpick comments:
In `@frontend/src/App.tsx`:
- Around line 62-63: Wrap the /admin/dashboard route with a router-level guard
by replacing the element using AdminDashboard with a ProtectedRoute that checks
admin auth before rendering; create or reuse a ProtectedRoute component (e.g.,
ProtectedRoute) that verifies the same adminToken logic (or a shared auth
helper) and renders <AdminDashboard /> only when authorized otherwise navigates
to /admin/login, and update the Route declaration that currently references
AdminDashboard so the guard prevents transient rendering of the dashboard
component.
In `@frontend/src/components/Matchmaking.tsx`:
- Around line 38-42: The useEffect that calls setIsInPool(false) and
setWaitTime(0) on mount is redundant because isInPool and waitTime are already
initialized to those values and server handlers (readPump cleanup and the
pool_update handler) restore state; either remove this useEffect entirely or
keep it but add a concise comment explaining it's defensive for React Fast
Refresh/state-preservation in development. Locate the effect in Matchmaking.tsx
(the useEffect that invokes setIsInPool and setWaitTime) and implement one of
those two options, ensuring pool_update and readPump logic remain unchanged.
- Around line 38-42: The useEffect block in Matchmaking.tsx is indented with 4
spaces while the rest of the file uses 2-space indentation; reformat the block
containing useEffect, setIsInPool(false), and setWaitTime(0) to use 2-space
indentation to match the surrounding code (preserve the same logic and empty
dependency array).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 98af4a2c-8559-437b-b262-aba26ae0feba
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
backend/config/config.prod.sample.ymlbackend/controllers/debatevsbot_controller.gobackend/controllers/profile_controller.gobackend/controllers/transcript_controller.gobackend/models/transcript.gobackend/services/transcriptservice.gofrontend/package.jsonfrontend/src/App.tsxfrontend/src/Pages/DebateRoom.tsxfrontend/src/Pages/Home.tsxfrontend/src/Pages/TeamBuilder.tsxfrontend/src/components/Header.tsxfrontend/src/components/JudgementPopup.tsxfrontend/src/components/Matchmaking.tsxfrontend/src/components/Sidebar.tsxfrontend/src/components/ui/select.tsxfrontend/src/index.css
💤 Files with no reviewable changes (1)
- backend/config/config.prod.sample.yml
Added dark:text classes for better readability in dark mode Updated message cards with dark background support Fixed low-contrast text issues across UI Removed sensitive console logs (messages, API results) Added dev-only logging using import.meta.env.DEV Cleaned duplicate container styles Improves accessibility, security, and production readiness
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/Pages/DebateRoom.tsx (1)
746-746:⚠️ Potential issue | 🟡 MinorFix Tailwind utility typo:
max-w-mshould bemax-w-md.
max-w-mis not a standard utility, so width styling won’t apply as intended.Patch
- <div className="bg-white dark:bg-zinc-900 rounded-xl p-6 max-w-m w-full transform transition-all duration-300 scale-105 shadow-2xl border border-orange-200 dark:border-orange-400/30"> + <div className="bg-white dark:bg-zinc-900 rounded-xl p-6 max-w-md w-full transform transition-all duration-300 scale-105 shadow-2xl border border-orange-200 dark:border-orange-400/30">#!/bin/bash set -euo pipefail rg -n '\bmax-w-m\b' frontend/src/Pages/DebateRoom.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Pages/DebateRoom.tsx` at line 746, The Tailwind class "max-w-m" in the JSX div's className within the DebateRoom component is a typo and not a valid utility; update that class to "max-w-md" so the intended max-width applies (look for the div with className starting "bg-white dark:bg-zinc-900 rounded-xl p-6 ... scale-105 shadow-2xl border border-orange-200 dark:border-orange-400/30" inside DebateRoom.tsx and replace max-w-m with max-w-md).
🧹 Nitpick comments (1)
frontend/src/Pages/DebateRoom.tsx (1)
10-10: Addjson5as a direct dependency infrontend/package.json.The import at line 10 relies on
json5being available transitively, but it is not declared as a direct dependency. This is fragile and should be explicitly added tofrontend/package.jsonto ensure clarity and prevent unexpected breakage if the transitive path changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Pages/DebateRoom.tsx` at line 10, The import JSON5 in DebateRoom.tsx is relying on a transitive dependency; add "json5" as a direct dependency in frontend/package.json and run yarn/npm install to lock it in. Update frontend/package.json dependencies to include the latest compatible "json5" version, then reinstall (and regenerate lockfile) so the import of JSON5 in DebateRoom.tsx resolves reliably.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/Pages/DebateRoom.tsx`:
- Around line 777-780: The current onClose handler passed from DebateRoom
(setShowJudgment and navigate("/")) makes the JudgementPopup's Close button
behave like “Back to Home”; change the handler so closing the popup only hides
it: update the prop passed to JudgementPopup by replacing the current onClose
that calls navigate("/") with one that only calls setShowJudgment(false) (keep
the existing navigation call tied to the explicit “Back to Home” button
handler). Locate the prop usage around setShowJudgment and navigate in
DebateRoom and adjust the callback so JudgementPopup.onClose only toggles
visibility while preserving the existing navigate("/") call on the dedicated
back/home control.
- Around line 224-231: The component currently returns early based on debateData
(from location.state) before calling later hooks (useAtom, useState, useEffect),
which can break hook ordering; instead, always call hooks and derive the initial
debate payload from location.state inside a hook-friendly construct (e.g.,
useMemo or useState initializer) so hooks like useAtom, useState, and useEffect
in this component are declared unconditionally, then conditionally render a
fallback UI when the derived debateData is null/invalid; update references to
debateData usage to use the derived value and keep the early-return removed to
preserve hook order.
---
Duplicate comments:
In `@frontend/src/Pages/DebateRoom.tsx`:
- Line 746: The Tailwind class "max-w-m" in the JSX div's className within the
DebateRoom component is a typo and not a valid utility; update that class to
"max-w-md" so the intended max-width applies (look for the div with className
starting "bg-white dark:bg-zinc-900 rounded-xl p-6 ... scale-105 shadow-2xl
border border-orange-200 dark:border-orange-400/30" inside DebateRoom.tsx and
replace max-w-m with max-w-md).
---
Nitpick comments:
In `@frontend/src/Pages/DebateRoom.tsx`:
- Line 10: The import JSON5 in DebateRoom.tsx is relying on a transitive
dependency; add "json5" as a direct dependency in frontend/package.json and run
yarn/npm install to lock it in. Update frontend/package.json dependencies to
include the latest compatible "json5" version, then reinstall (and regenerate
lockfile) so the import of JSON5 in DebateRoom.tsx resolves reliably.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 03eb086a-eaff-4012-835b-2c1db3265e0d
📒 Files selected for processing (3)
frontend/index.htmlfrontend/src/Pages/DebateRoom.tsxfrontend/tailwind.config.js
✅ Files skipped from review due to trivial changes (1)
- frontend/index.html
Comment on lines
+224
to
+231
| const debateData = location.state as unknown as DebateProps | null; | ||
| if (!debateData) { | ||
| return ( | ||
| <div className="flex h-screen items-center justify-center"> | ||
| Invalid access. Please start debate from dashboard. | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
p = Path("frontend/src/Pages/DebateRoom.tsx")
lines = p.read_text().splitlines()
guard = next((i+1 for i,l in enumerate(lines) if "if (!debateData)" in l), None)
later_hooks = []
if guard:
for i,l in enumerate(lines[guard:], start=guard+1):
if "useState(" in l or "useEffect(" in l or "useAtom(" in l:
later_hooks.append((i, l.strip()))
print("guard_line:", guard)
print("first_3_hooks_after_guard:", later_hooks[:3])
if guard and later_hooks:
print("Potential hook-order violation: conditional return precedes later hooks.")
PYRepository: AOSSIE-Org/DebateAI
Length of output: 347
🏁 Script executed:
cd frontend/src/Pages && sed -n '220,260p' DebateRoom.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 1498
🏁 Script executed:
cd frontend/src/Pages && grep -n "json5\|max-w-m\|JudgmentPopup" DebateRoom.tsx | head -20Repository: AOSSIE-Org/DebateAI
Length of output: 375
Conditional return before later hooks can break React hook ordering.
Line 225 returns before hooks declared from line 234 onward (useAtom, useState, useEffect). If location.state changes during this component instance's lifetime, React can hit "Rendered more hooks than during the previous render."
Suggested structure fix
-import { useLocation, useNavigate } from "react-router-dom";
+import { Navigate, useLocation, useNavigate } from "react-router-dom";
const DebateRoom: React.FC = () => {
- const judgingRef = useRef(false);
- const navigate = useNavigate();
const location = useLocation();
- const debateData = location.state as unknown as DebateProps | null;
+ const debateData = location.state as DebateProps | null;
if (!debateData) {
- return (
- <div className="flex h-screen items-center justify-center">
- Invalid access. Please start debate from dashboard.
- </div>
- );
+ return <Navigate to="/startDebate" replace />;
}
+
+ return <DebateRoomContent debateData={debateData} />;
+};
+
+const DebateRoomContent: React.FC<{ debateData: DebateProps }> = ({ debateData }) => {
+ const judgingRef = useRef(false);
+ const navigate = useNavigate();
// existing hook-heavy logic stays here🤖 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 224 - 231, The component
currently returns early based on debateData (from location.state) before calling
later hooks (useAtom, useState, useEffect), which can break hook ordering;
instead, always call hooks and derive the initial debate payload from
location.state inside a hook-friendly construct (e.g., useMemo or useState
initializer) so hooks like useAtom, useState, and useEffect in this component
are declared unconditionally, then conditionally render a fallback UI when the
derived debateData is null/invalid; update references to debateData usage to use
the derived value and keep the early-return removed to preserve hook order.
Comment on lines
+777
to
+780
| onClose={() => { | ||
| setShowJudgment(false); | ||
| navigate("/"); | ||
| }} |
There was a problem hiding this comment.
onClose now behaves like “Back to Home,” so the two popup buttons do the same thing.
In frontend/src/components/JudgementPopup.tsx, the “Close” button directly triggers onClose. With this handler navigating to /, users lose a close-only action.
Suggested behavior-preserving fix
onClose={() => {
setShowJudgment(false);
- navigate("/");
}}🤖 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 777 - 780, The current
onClose handler passed from DebateRoom (setShowJudgment and navigate("/")) makes
the JudgementPopup's Close button behave like “Back to Home”; change the handler
so closing the popup only hides it: update the prop passed to JudgementPopup by
replacing the current onClose that calls navigate("/") with one that only calls
setShowJudgment(false) (keep the existing navigation call tied to the explicit
“Back to Home” button handler). Locate the prop usage around setShowJudgment and
navigate in DebateRoom and adjust the callback so JudgementPopup.onClose only
toggles visibility while preserving the existing navigate("/") call on the
dedicated back/home control.
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.
Overview
This PR implements a unified branding update along with UI enhancements to improve consistency, discoverability, and user interaction across the frontend.
Changes Implemented
Unified Platform Naming
Replaced all legacy "Argue-Hub" references with "DebateAI"
Standardized naming across:
Header
Sidebar
Page titles
Navigation elements
AOSSIE Organization Discovery
Added an interactive AOSSIE logo across the platform
Logo is clickable and redirects to: https://aossie.org
Integrated in:
Home page
Start Debate page
Header
Sidebar
UI Micro-Interaction Enhancements
Added subtle hover animations for better user feedback
Implemented heart icon interaction:
Changes color to red on hover ❤️
Includes smooth scaling animation
Improved overall UI responsiveness and engagement
Benefits
Eliminates confusion from outdated branding
Ensures consistent platform identity (DebateAI)
Enhances discoverability of AOSSIE
Improves user experience with interactive UI elements
Testing
Verified all UI changes locally
Checked hover animations and responsiveness
Ensured no layout breaks across components
close #355
Summary by CodeRabbit
New Features
Bug Fixes
UI/Style
Chores