Improved homepage heading and auth UI#125
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the CommDesk subproject, modifies the copy on the SignUpPage, and updates styling and input types in the Input component and LoginPage. The review feedback highlights critical issues: hardcoding colors (black and white) in the reusable Input component breaks the application's theming system, and changing the password input type to "text" in the LoginPage introduces a severe security vulnerability by exposing passwords in plain text.
| <Input | ||
| name="password" | ||
| className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500" | ||
| className="w-full px-3 py-2 border border-gray-300 rounded-md text-black bg-white placeholder:text-gray-500 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500" | ||
| placeholder="Enter your password" | ||
| type="password" | ||
| type="text" | ||
| /> |
There was a problem hiding this comment.
Changing the password input type to "text" permanently exposes the user's password in plain text on the screen. This is a major security risk. If a password visibility toggle is desired, you should implement a state variable (e.g., showPassword) and a button to toggle the input type between "password" and "text" dynamically, rather than hardcoding it to "text".
| <Input | |
| name="password" | |
| className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500" | |
| className="w-full px-3 py-2 border border-gray-300 rounded-md text-black bg-white placeholder:text-gray-500 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500" | |
| placeholder="Enter your password" | |
| type="password" | |
| type="text" | |
| /> | |
| <Input | |
| name="password" | |
| className="w-full px-3 py-2 border border-gray-300 rounded-md text-black bg-white placeholder:text-gray-500 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500" | |
| placeholder="Enter your password" | |
| type="password" | |
| /> |
| htmlFor={name} | ||
| className="text-xs font-semibold uppercase tracking-wider" | ||
| style={{ color: theme.text.secondary }} | ||
| style={{ color: "black"}} |
There was a problem hiding this comment.
Hardcoding the color to 'black' inside a reusable component bypasses the application's theme configuration (theme.text.secondary). This breaks dark mode support and consistency across the app. If a specific color is needed for a particular instance, it should be passed via props or handled through Tailwind classes rather than hardcoded in the base component.
| style={{ color: "black"}} | |
| style={{ color: theme.text.secondary }} |
| className="flex items-center gap-2 rounded-lg px-3 py-2 border transition-all duration-150" | ||
| style={{ | ||
| backgroundColor: theme.bg.surface, | ||
| backgroundColor: "black", |
There was a problem hiding this comment.
| className={`flex-1 bg-black text-white outline-none text-sm ${inputClassName}`} | ||
| style={{ color: "white" }} |
There was a problem hiding this comment.
Hardcoding bg-black text-white and color: "white" overrides the theme settings (theme.text.primary) and prevents the input from rendering correctly in other themes (e.g., light mode).
| className={`flex-1 bg-black text-white outline-none text-sm ${inputClassName}`} | |
| style={{ color: "white" }} | |
| className={"flex-1 bg-transparent outline-none text-sm " + inputClassName} | |
| style={{ color: theme.text.primary }} |
Description:
Improved the homepage heading text and enhanced the authentication UI to make it more engaging and user-friendly.
Changes Made:
Screenshots:
Added before and after screenshots below.
BEFORE:


AFTER:
PASSWORD VISIBILITY:
