Improve Home page UI/UX and readability#190
Improve Home page UI/UX and readability#190ankitkr104 wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR adds backend CLI tools for user management (adduser, updateuser), modifies the signup flow to auto-verify users in development mode while maintaining email verification in production, and comprehensively redesigns the frontend with improved typography, spacing, header navigation with theming support, and custom Tailwind utilities for text shadows. Changes
Sequence DiagramssequenceDiagram
participant User
participant SignupFlow as Auth Controller
participant Config as Configuration
participant DB as MongoDB
participant Mailer as SMTP Service
User->>SignupFlow: POST /signup (email, password, name)
SignupFlow->>Config: Check SMTP configuration
alt SMTP Configured (Production)
SignupFlow->>DB: Hash password & create user
SignupFlow->>DB: Generate verification code
SignupFlow->>Mailer: Send verification email
SignupFlow-->>User: Return success message
else SMTP Not Configured (Development)
SignupFlow->>DB: Hash password & create user
SignupFlow->>DB: Set isVerified = true
SignupFlow->>SignupFlow: Generate JWT token
SignupFlow-->>User: Return accessToken + user data
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/Header.tsx (1)
258-311: Missing dark mode styling in notification popover content.The notification popover uses hardcoded light theme colors (
text-gray-900,bg-gray-50,border-gray-100, etc.) withoutdark:variants, causing poor visibility in dark mode.🎨 Add dark mode variants
Key lines to update:
- <div className="flex items-center justify-between p-4 border-b border-gray-100"> - <h4 className="font-semibold text-gray-900">Notifications</h4> + <div className="flex items-center justify-between p-4 border-b border-gray-100 dark:border-gray-700"> + <h4 className="font-semibold text-gray-900 dark:text-white">Notifications</h4> - <div className="divide-y divide-gray-100"> + <div className="divide-y divide-gray-100 dark:divide-gray-700"> - className={`p-4 hover:bg-gray-50 cursor-pointer transition-colors relative group ${!notification.isRead ? 'bg-blue-50/50' : ''}`} + className={`p-4 hover:bg-gray-50 dark:hover:bg-gray-700 cursor-pointer transition-colors relative group ${!notification.isRead ? 'bg-blue-50/50 dark:bg-blue-900/30' : ''}`} - <p className="text-sm font-medium leading-none text-gray-900"> + <p className="text-sm font-medium leading-none text-gray-900 dark:text-white">
🤖 Fix all issues with AI agents
In @backend/cmd/adduser/main.go:
- Around line 66-85: The CLI user creation sets Rating: 1500.0 on the newUser
struct; change it to match the SignUp controller's production default of Rating:
1200.0 (or add a clear comment documenting why the CLI would intentionally use
1500.0). Update the Rating field in the newUser literal in
backend/cmd/adduser/main.go to 1200.0 (or add a comment next to Rating
explaining the deliberate deviation).
- Around line 19-33: The main() function currently sets configPath to
"config/config.prod.yml" which risks accidental production use; change the
default to a development config (e.g., "config/config.dev.yml") or make the flag
required by setting configPath := flag.String("config", "", "Path to config
file") and adding a validation check similar to the email/password/name check
that prints usage and exits if *configPath == ""; update any help text
accordingly to reference the dev default if you choose the default-change
option.
In @backend/cmd/updateuser/main.go:
- Around line 19-33: The default configPath in main currently uses
"config/config.prod.yml", risking accidental production updates; change its
default to a development config (e.g., "config/config.dev.yml") or make the flag
required by setting default to "" and adding a validation that prints an error
and exits if *configPath == ""; update the flag declaration (configPath
variable) and the validation block in main to enforce the new
default/requirement and ensure the help text reflects the change.
In @frontend/src/components/Sidebar.tsx:
- Around line 18-23: The Sidebar component uses hardcoded light-theme classes
and unsafe icon cloning: replace instances of 'bg-white' and 'border-gray-200'
in the Sidebar JSX with the app's design tokens (e.g., 'bg-background' and
'border-border') so the component respects dark mode, and make icon rendering
safe by checking React.isValidElement(icon) before calling
React.cloneElement(icon as ReactElement, ...) — if the check fails, render a
fallback/null; update the same fix for the other occurrence noted around the
cloneElement use (the one at lines ~90-92).
In @frontend/src/Pages/Home.tsx:
- Around line 59-67: The heading h3 uses a span with className
"whitespace-nowrap" which causes overflow inside the right column (container
with md:w-1/3) at md/lg font sizes; fix by making the no-wrap behavior
responsive or adjusting sizing—either remove "whitespace-nowrap" from the span,
replace it with "md:whitespace-normal" (or "whitespace-normal
md:whitespace-nowrap" depending on desired breakpoints), or alternatively reduce
the heading font at md/lg (change md:text-6xl/lg:text-7xl to smaller classes) or
increase the column width (e.g., change md:w-1/3 to md:w-1/2); pick one approach
and update the h3/span className and/or the parent div className accordingly.
🧹 Nitpick comments (10)
backend/cmd/adduser/main.go (1)
41-45: Handle the disconnect error.The deferred
Disconnectcall should check for errors to ensure proper cleanup logging.♻️ Suggested improvement
- defer db.MongoClient.Disconnect(context.Background()) + defer func() { + if err := db.MongoClient.Disconnect(context.Background()); err != nil { + log.Printf("Warning: failed to disconnect from MongoDB: %v", err) + } + }()backend/cmd/updateuser/main.go (1)
41-45: Handle the disconnect error.The deferred
Disconnectcall should check for errors to ensure proper cleanup logging.♻️ Suggested improvement
- defer db.MongoClient.Disconnect(context.Background()) + defer func() { + if err := db.MongoClient.Disconnect(context.Background()); err != nil { + log.Printf("Warning: failed to disconnect from MongoDB: %v", err) + } + }()backend/controllers/auth.go (1)
190-227: Consider explicit environment detection for the signup flow.The current implementation uses SMTP credential presence to determine whether to auto-verify users (lines 191-201 for production vs. 201-227 for development). While functional, this approach is implicit and could be fragile if SMTP credentials are temporarily misconfigured.
Consider using an explicit environment variable for clearer intent:
♻️ Suggested improvement
+ isDevelopment := os.Getenv("ENV") == "development" || os.Getenv("ENV") == "dev" + // Send verification email (skip in development if SMTP not configured) - if cfg.SMTP.Username != "" && cfg.SMTP.Password != "" { + if !isDevelopment && cfg.SMTP.Username != "" && cfg.SMTP.Password != "" { err = utils.SendVerificationEmail(request.Email, verificationCode) if err != nil { ctx.JSON(500, gin.H{"error": "Failed to send verification email", "message": err.Error()}) return } // Return success response - email verification required ctx.JSON(200, gin.H{ "message": "Sign-up successful. Please verify your email.", }) - } else { + } else if isDevelopment { // Development mode: auto-verify userThis makes the environment mode explicit and prevents unintended auto-verification if SMTP is simply misconfigured in production.
frontend/tailwind.config.js (1)
74-83: Verify Tailwind plugin API + consider usingtailwindcss/pluginfor clarity/compatibility.
matchUtilitiesis correct here, but this is Tailwind-version-sensitive; please confirm the project’stailwindcssversion supports this usage and that the config loader accepts it as written.Proposed refactor (more idiomatic plugin wrapper)
+import plugin from 'tailwindcss/plugin' + export default { darkMode: ["class"], content: [ "./index.html", "./src/**/*.{js,ts,jsx,tsx}", ], theme: { extend: { @@ }, }, plugins: [ require("tailwindcss-animate"), require("tailwind-scrollbar-hide"), - function({ matchUtilities, theme }) { - matchUtilities( - { - 'text-shadow': (value) => ({ - textShadow: value, - }), - }, - { values: theme('textShadow') } - ) - }, + plugin(function ({ matchUtilities, theme }) { + matchUtilities( + { 'text-shadow': (value) => ({ textShadow: value }) }, + { values: theme('textShadow') } + ) + }), ], }frontend/src/Pages/Home.tsx (1)
65-91: Reduce inline style duplication + verify “Practice with Bot” route/wording matches PR objective.
- The repeated
style={{ textShadow: ... }}/style={{ boxShadow: ... }}blocks are hard to maintain; consider using the newtext-shadow-*utilities + Tailwind shadows (or extract shared constants/components).- Also please confirm the “no content change” constraint (button labels/headline copy appear updated), and that the Bot CTA navigates to the intended flow (its label now reads “Practice with Bot”).
frontend/src/Pages/StartDebate.tsx (1)
34-77: Consider extracting a shared “Hero + CTA stack” component to avoid Home/StartDebate drift.These sections are nearly identical (heading + two CTA buttons + shadows). A shared component (and/or shared style tokens using
text-shadow-*) will reduce duplication and keep the redesign consistent.frontend/src/components/Header.tsx (4)
61-66: Inefficient theme switching via repeated toggleTheme calls.Calling
toggleTheme()in a loop causes multiple state updates and potential re-renders. This approach also assumes the toggle cycles themes in a specific order.Consider extending
ThemeContextto expose a directsetTheme(theme: ThemeOptions)method instead of computing clicks and toggling repeatedly.♻️ Suggested approach
If modifying the context is feasible:
// In theme-provider.tsx, expose setTheme directly const setTheme = (targetTheme: ThemeOptions) => { // directly set the theme state }; // Then in Header.tsx, simply: const { theme, setTheme } = useContext(ThemeContext); // ... onClick={() => setTheme(option.id)}If you must keep the current context API, at minimum batch the updates or use a ref to avoid cascading renders.
76-81: Consider memoizing fetchNotifications or adding to deps.The
fetchNotificationsfunction is defined inside the component but not included in the dependency array. While functionally correct here (sinceuseris the key dependency), ESLint's exhaustive-deps rule may warn.♻️ Option: wrap with useCallback
+ const fetchNotifications = useCallback(async () => { + if (user) { + const data = await getNotifications(); + setNotifications(data); + setUnreadCount(data.filter(n => !n.isRead).length); + } + }, [user]); useEffect(() => { fetchNotifications(); const interval = setInterval(fetchNotifications, 60000); return () => clearInterval(interval); - }, [user]); + }, [fetchNotifications]);
154-154: Redundant shadow styling - inline style overrides Tailwind class.The inline
style={{ boxShadow: '...' }}overrides the Tailwindshadow-2xlclass, making it ineffective. Consider using only one approach for consistency.♻️ Remove redundant class
- <header className="flex items-center justify-between h-20 px-6 border-b border-gray-200 dark:border-gray-700 shadow-2xl bg-white dark:bg-gray-900" style={{ boxShadow: '0 10px 25px rgba(0, 0, 0, 0.15), 0 4px 10px rgba(0, 0, 0, 0.1)' }}> + <header className="flex items-center justify-between h-20 px-6 border-b border-gray-200 dark:border-gray-700 bg-white dark:bg-gray-900" style={{ boxShadow: '0 10px 25px rgba(0, 0, 0, 0.15), 0 4px 10px rgba(0, 0, 0, 0.1)' }}>Or define a custom shadow utility in Tailwind config to avoid inline styles entirely.
166-245: Consider extracting navigation items to reduce duplication.The six
NavLinkcomponents share identical className logic. A data-driven approach would improve maintainability.♻️ Suggested refactor
const navItems = [ { to: "/startDebate", icon: MessageSquare, label: "Start Debate" }, { to: "/tournaments", icon: Trophy, label: "Tournaments" }, { to: "/team-builder", icon: Users, label: "Team Debates" }, { to: "/leaderboard", icon: BarChart, label: "Leaderboard" }, { to: "/community", icon: MessageCircle, label: "Community" }, { to: "/about", icon: Info, label: "About" }, ]; const navLinkClass = ({ isActive }: { isActive: boolean }) => `flex items-center gap-2 px-4 py-3 text-lg font-medium rounded-md transition-colors ${ isActive ? "bg-secondary text-secondary-foreground" : "text-gray-700 dark:text-gray-200 hover:bg-gray-100 dark:hover:bg-gray-800 hover:text-gray-900 dark:hover:text-white" }`; // In JSX: <nav className="hidden lg:flex items-center justify-center space-x-8 flex-1"> {navItems.map(({ to, icon: Icon, label }) => ( <NavLink key={to} to={to} className={navLinkClass}> <Icon className="h-5 w-5" /> {label} </NavLink> ))} </nav>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
backend/cmd/adduser/main.gobackend/cmd/updateuser/main.gobackend/controllers/auth.gofrontend/src/Pages/Home.tsxfrontend/src/Pages/StartDebate.tsxfrontend/src/components/Header.tsxfrontend/src/components/Layout.tsxfrontend/src/components/Sidebar.tsxfrontend/tailwind.config.js
🧰 Additional context used
🧬 Code graph analysis (5)
frontend/src/Pages/StartDebate.tsx (1)
frontend/src/components/ui/button.tsx (1)
Button(57-57)
frontend/src/components/Header.tsx (2)
frontend/src/context/theme-provider.tsx (1)
ThemeContext(18-18)frontend/src/components/ui/popover.tsx (3)
Popover(31-31)PopoverTrigger(31-31)PopoverContent(31-31)
backend/controllers/auth.go (2)
backend/utils/email.go (1)
SendVerificationEmail(29-55)backend/db/db.go (1)
MongoDatabase(18-18)
backend/cmd/adduser/main.go (2)
backend/config/config.go (1)
LoadConfig(60-92)backend/db/db.go (3)
ConnectMongoDB(40-61)MongoClient(17-17)MongoDatabase(18-18)
frontend/src/Pages/Home.tsx (1)
frontend/src/components/ui/button.tsx (1)
Button(57-57)
🔇 Additional comments (8)
backend/cmd/updateuser/main.go (1)
60-77: LGTM: Clean conditional update logic.The update document construction correctly handles optional fields—only updating password when provided and only setting verification status when the flag is true. The mandatory
updatedAttimestamp ensures audit trail consistency.backend/controllers/auth.go (1)
203-218: LGTM: Proper error handling in development mode auto-verification.The implementation correctly handles potential failures in both the database update (lines 203-211) and JWT generation (lines 213-218), returning appropriate error responses to the client.
frontend/tailwind.config.js (1)
13-21: Nice addition: centralizedtextShadowtokens.This is a clean way to standardize shadow depth across pages/components (especially since the PR aims for consistent hierarchy/readability).
frontend/src/components/Layout.tsx (1)
7-11: Potential UX/regression risk:h-screen overflow-hidden+ inner scroll container.Please verify overlays (dialogs/popups/toasts) and mobile Safari viewport behavior still work as expected when the page scroll is confined to
<main>.frontend/src/components/Header.tsx (4)
3-23: LGTM!Imports are well-organized and all appear to be utilized in the component.
411-421: Potential overflow in mobile drawer header.The drawer is
w-64(256px) but containstext-3xlbranding text plus a logo image. This may cause horizontal overflow or cramped layout on smaller screens.Consider using smaller text (
text-xlortext-2xl) in the drawer header, or verify the layout renders correctly across device widths.
350-390: LGTM!Theme selection UI is well-implemented with proper dark mode support and clear visual indicators for the current theme selection.
488-505: LGTM!The
NavItemcomponent is well-structured with proper TypeScript typing and comprehensive dark mode support.
| func main() { | ||
| // Parse command line flags | ||
| email := flag.String("email", "", "User email (required)") | ||
| password := flag.String("password", "", "User password (required)") | ||
| name := flag.String("name", "", "User display name (required)") | ||
| configPath := flag.String("config", "config/config.prod.yml", "Path to config file") | ||
| flag.Parse() | ||
|
|
||
| // Validate required fields | ||
| if *email == "" || *password == "" || *name == "" { | ||
| fmt.Println("Error: email, password, and name are required") | ||
| fmt.Println("\nUsage:") | ||
| flag.PrintDefaults() | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
Reconsider the default config path for a development tool.
The tool defaults to config/config.prod.yml (line 24) but the comment on line 79 indicates this is for development use ("Auto-verify for development"). Consider defaulting to a development config or making the config flag required to prevent accidental operations against production databases.
🔧 Suggested fix
- configPath := flag.String("config", "config/config.prod.yml", "Path to config file")
+ configPath := flag.String("config", "config/config.dev.yml", "Path to config file")Or make it required:
configPath := flag.String("config", "config/config.prod.yml", "Path to config file")
flag.Parse()
// Validate required fields
- if *email == "" || *password == "" || *name == "" {
- fmt.Println("Error: email, password, and name are required")
+ if *email == "" || *password == "" || *name == "" || *configPath == "config/config.prod.yml" {
+ fmt.Println("Error: email, password, name, and config are required")
fmt.Println("\nUsage:")
flag.PrintDefaults()
os.Exit(1)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func main() { | |
| // Parse command line flags | |
| email := flag.String("email", "", "User email (required)") | |
| password := flag.String("password", "", "User password (required)") | |
| name := flag.String("name", "", "User display name (required)") | |
| configPath := flag.String("config", "config/config.prod.yml", "Path to config file") | |
| flag.Parse() | |
| // Validate required fields | |
| if *email == "" || *password == "" || *name == "" { | |
| fmt.Println("Error: email, password, and name are required") | |
| fmt.Println("\nUsage:") | |
| flag.PrintDefaults() | |
| os.Exit(1) | |
| } | |
| func main() { | |
| // Parse command line flags | |
| email := flag.String("email", "", "User email (required)") | |
| password := flag.String("password", "", "User password (required)") | |
| name := flag.String("name", "", "User display name (required)") | |
| configPath := flag.String("config", "config/config.dev.yml", "Path to config file") | |
| flag.Parse() | |
| // Validate required fields | |
| if *email == "" || *password == "" || *name == "" { | |
| fmt.Println("Error: email, password, and name are required") | |
| fmt.Println("\nUsage:") | |
| flag.PrintDefaults() | |
| os.Exit(1) | |
| } |
🤖 Prompt for AI Agents
In @backend/cmd/adduser/main.go around lines 19 - 33, The main() function
currently sets configPath to "config/config.prod.yml" which risks accidental
production use; change the default to a development config (e.g.,
"config/config.dev.yml") or make the flag required by setting configPath :=
flag.String("config", "", "Path to config file") and adding a validation check
similar to the email/password/name check that prints usage and exits if
*configPath == ""; update any help text accordingly to reference the dev default
if you choose the default-change option.
| // Create new user | ||
| now := time.Now() | ||
| newUser := models.User{ | ||
| Email: *email, | ||
| Password: string(hashedPassword), | ||
| DisplayName: *name, | ||
| Nickname: *name, | ||
| Bio: "Test user account", | ||
| Rating: 1500.0, | ||
| RD: 350.0, | ||
| Volatility: 0.06, | ||
| LastRatingUpdate: now, | ||
| AvatarURL: "https://avatar.iran.liara.run/public/10", | ||
| IsVerified: true, // Auto-verify for development | ||
| Score: 0, | ||
| Badges: []string{}, | ||
| CurrentStreak: 0, | ||
| CreatedAt: now, | ||
| UpdatedAt: now, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all Rating initializations in Go files
rg -n 'Rating:\s*[0-9]+' --type=go -C 2Repository: AOSSIE-Org/DebateAI
Length of output: 8539
🏁 Script executed:
#!/bin/bash
# Also search with decimal notation
rg -n 'Rating:\s*[0-9]+\.[0-9]+' --type=go -C 2Repository: AOSSIE-Org/DebateAI
Length of output: 995
🏁 Script executed:
#!/bin/bash
# Check the auth.go file specifically around line 167
if [ -f "backend/controllers/auth.go" ]; then
sed -n '160,175p' backend/controllers/auth.go
fiRepository: AOSSIE-Org/DebateAI
Length of output: 625
🏁 Script executed:
#!/bin/bash
# Get a broader view of the file to understand the structure
fd 'auth.go' --type=f | head -5Repository: AOSSIE-Org/DebateAI
Length of output: 187
Align user creation defaults across CLI and signup controller.
The CLI tool (backend/cmd/adduser/main.go line 74) sets Rating: 1500.0, while the SignUp controller (backend/controllers/auth.go lines 82 and 167) consistently uses Rating: 1200.0. The signup flow is the production standard; the CLI should use the same default value for consistency. If the higher rating in the CLI is intentional for development purposes, it should be documented.
🤖 Prompt for AI Agents
In @backend/cmd/adduser/main.go around lines 66 - 85, The CLI user creation sets
Rating: 1500.0 on the newUser struct; change it to match the SignUp controller's
production default of Rating: 1200.0 (or add a clear comment documenting why the
CLI would intentionally use 1500.0). Update the Rating field in the newUser
literal in backend/cmd/adduser/main.go to 1200.0 (or add a comment next to
Rating explaining the deliberate deviation).
| func main() { | ||
| // Parse command line flags | ||
| email := flag.String("email", "", "User email (required)") | ||
| password := flag.String("password", "", "New password (optional)") | ||
| verify := flag.Bool("verify", false, "Set user as verified") | ||
| configPath := flag.String("config", "config/config.prod.yml", "Path to config file") | ||
| flag.Parse() | ||
|
|
||
| // Validate required fields | ||
| if *email == "" { | ||
| fmt.Println("Error: email is required") | ||
| fmt.Println("\nUsage:") | ||
| flag.PrintDefaults() | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
Reconsider the default config path for a development tool.
Similar to the adduser tool, this defaults to config/config.prod.yml (line 24), which poses a risk of unintended updates to production data. Consider using a development config by default or requiring the config flag explicitly.
🔧 Suggested fix
- configPath := flag.String("config", "config/config.prod.yml", "Path to config file")
+ configPath := flag.String("config", "config/config.dev.yml", "Path to config file")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func main() { | |
| // Parse command line flags | |
| email := flag.String("email", "", "User email (required)") | |
| password := flag.String("password", "", "New password (optional)") | |
| verify := flag.Bool("verify", false, "Set user as verified") | |
| configPath := flag.String("config", "config/config.prod.yml", "Path to config file") | |
| flag.Parse() | |
| // Validate required fields | |
| if *email == "" { | |
| fmt.Println("Error: email is required") | |
| fmt.Println("\nUsage:") | |
| flag.PrintDefaults() | |
| os.Exit(1) | |
| } | |
| func main() { | |
| // Parse command line flags | |
| email := flag.String("email", "", "User email (required)") | |
| password := flag.String("password", "", "New password (optional)") | |
| verify := flag.Bool("verify", false, "Set user as verified") | |
| configPath := flag.String("config", "config/config.dev.yml", "Path to config file") | |
| flag.Parse() | |
| // Validate required fields | |
| if *email == "" { | |
| fmt.Println("Error: email is required") | |
| fmt.Println("\nUsage:") | |
| flag.PrintDefaults() | |
| os.Exit(1) | |
| } |
🤖 Prompt for AI Agents
In @backend/cmd/updateuser/main.go around lines 19 - 33, The default configPath
in main currently uses "config/config.prod.yml", risking accidental production
updates; change its default to a development config (e.g.,
"config/config.dev.yml") or make the flag required by setting default to "" and
adding a validation that prints an error and exits if *configPath == ""; update
the flag declaration (configPath variable) and the validation block in main to
enforce the new default/requirement and ensure the help text reflects the
change.
| <aside className='hidden md:flex flex-col w-64 border-r border-gray-200 shadow-md bg-white'> | ||
| {/* Logo / Brand */} | ||
| <div className='flex items-center h-16 px-4 border-b border-gray-200'> | ||
| <div className='flex items-center h-16 px-4 border-b border-gray-200 shadow-sm bg-white'> | ||
| <div className='flex items-center gap-2'> | ||
| <span className='text-xl font-bold'>DebateAI by</span> | ||
| <span className='text-2xl font-bold'>DebateAI by</span> | ||
| <img |
There was a problem hiding this comment.
Fix dark-mode theming + make icon rendering safe (current cloneElement can throw).
bg-white/border-gray-200will look wrong in dark mode; prefer the app’s tokenized colors (e.g.,bg-background,border-border).React.cloneElement(icon as React.ReactElement, ...)will crash ificonis ever omitted or not a valid element (andiconis currently optional).
Suggested patch (theme tokens + guarded icon cloning)
function Sidebar() {
return (
- <aside className='hidden md:flex flex-col w-64 border-r border-gray-200 shadow-md bg-white'>
+ <aside className='hidden md:flex flex-col w-64 border-r border-border shadow-md bg-background'>
{/* Logo / Brand */}
- <div className='flex items-center h-16 px-4 border-b border-gray-200 shadow-sm bg-white'>
+ <div className='flex items-center h-16 px-4 border-b border-border shadow-sm bg-background'>
<div className='flex items-center gap-2'>
- <span className='text-2xl font-bold'>DebateAI by</span>
+ <span className='text-2xl font-bold text-foreground'>DebateAI by</span>
@@
interface NavItemProps {
to: string;
label: string;
- icon?: React.ReactNode;
+ icon?: React.ReactElement;
}
function NavItem({ to, label, icon }: NavItemProps) {
return (
@@
- <span className="mr-3 h-5 w-5 flex items-center justify-center">
- {React.cloneElement(icon as React.ReactElement, { className: 'h-5 w-5' })}
- </span>
+ {icon && (
+ <span className="mr-3 h-5 w-5 flex items-center justify-center">
+ {React.cloneElement(icon, { className: 'h-5 w-5' })}
+ </span>
+ )}
{label}
</NavLink>
);
}Also applies to: 90-92
🤖 Prompt for AI Agents
In @frontend/src/components/Sidebar.tsx around lines 18 - 23, The Sidebar
component uses hardcoded light-theme classes and unsafe icon cloning: replace
instances of 'bg-white' and 'border-gray-200' in the Sidebar JSX with the app's
design tokens (e.g., 'bg-background' and 'border-border') so the component
respects dark mode, and make icon rendering safe by checking
React.isValidElement(icon) before calling React.cloneElement(icon as
ReactElement, ...) — if the check fails, render a fallback/null; update the same
fix for the other occurrence noted around the cloneElement use (the one at lines
~90-92).
| <div className="flex items-start justify-center pt-8"> | ||
| <div className="flex flex-wrap items-start justify-center w-full px-2 md:px-16"> | ||
| <div className="w-full md:w-2/3 p-4 md:p-16"> | ||
| <img src={DebateCover} alt="Debate Cover" className="w-full object-cover" /> | ||
| </div> | ||
| <div className="flex w-full md:w-1/3 flex-col items-center justify-center space-y-4 p-4"> | ||
| <h3 className="text-xl md:text-4xl font-bold text-center"> | ||
| Play Debate Online on the <span className="text-primary">#1</span> Site! | ||
| <div className="flex w-full md:w-1/3 flex-col items-center justify-center space-y-8 p-4 pt-16 min-h-[600px]"> | ||
| <h3 className="text-2xl md:text-6xl lg:text-7xl font-extrabold text-center leading-tight tracking-tight drop-shadow-2xl shadow-black/50" style={{ textShadow: '4px 4px 8px rgba(0, 0, 0, 0.5), 2px 2px 4px rgba(0, 0, 0, 0.3)' }}> | ||
| <span className="whitespace-nowrap">Play Debate Online</span><br />on the <span className="text-primary font-black drop-shadow-xl shadow-black/60">#1</span> Site! | ||
| </h3> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "Home.tsx" -path "*/frontend/*"Repository: AOSSIE-Org/DebateAI
Length of output: 92
🏁 Script executed:
cd frontend && find . -name "Home.tsx" | head -5Repository: AOSSIE-Org/DebateAI
Length of output: 83
🏁 Script executed:
git ls-files "frontend/src/Pages/Home.tsx"Repository: AOSSIE-Org/DebateAI
Length of output: 90
🏁 Script executed:
cat -n frontend/src/Pages/Home.tsx | sed -n '55,100p'Repository: AOSSIE-Org/DebateAI
Length of output: 3401
🏁 Script executed:
wc -l frontend/src/Pages/Home.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 94
Fix whitespace-nowrap overflow on medium+ screens where heading text exceeds 1/3-width container.
The heading "Play Debate Online" with whitespace-nowrap at md:text-6xl lg:text-7xl will overflow its md:w-1/3 container. At md breakpoint (768px), the content div is ~213px wide; at lg (1024px), ~298px—insufficient for a 60-72px font size without wrapping. Consider removing whitespace-nowrap or adjusting the responsive width/font scaling to prevent horizontal overflow.
🤖 Prompt for AI Agents
In @frontend/src/Pages/Home.tsx around lines 59 - 67, The heading h3 uses a span
with className "whitespace-nowrap" which causes overflow inside the right column
(container with md:w-1/3) at md/lg font sizes; fix by making the no-wrap
behavior responsive or adjusting sizing—either remove "whitespace-nowrap" from
the span, replace it with "md:whitespace-normal" (or "whitespace-normal
md:whitespace-nowrap" depending on desired breakpoints), or alternatively reduce
the heading font at md/lg (change md:text-6xl/lg:text-7xl to smaller classes) or
increase the column width (e.g., change md:w-1/3 to md:w-1/2); pick one approach
and update the h3/span className and/or the parent div className accordingly.
|
Thanks for the automated review! |
|
Hey @ankitkr104
|
|
hey @ayush00git , Regarding tailwind.config.js, that was added to experiment with a consistent shadow style, but I understand your point. I’ll move that styling to Tailwind utility classes instead of defining it globally to keep the design simpler and more maintainable. Thanks again for pointing this out — it definitely helps improve the quality of the PR 🙂 |
2 participants
Summary
This PR improves the UI/UX of the Home / Start Debate page while keeping all existing content, theme, icons, and functionality unchanged.
What changed
What did NOT change
Screenshots
Before
After
Related Issue
Closes #185
Summary by CodeRabbit
Release Notes
New Features
UI/Styling
✏️ Tip: You can customize this high-level summary in your review settings.