frontend-ui-redesign#294
Closed
firoziya wants to merge 2 commits into
Closed
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR implements a comprehensive redesign of GitHub Tracker's frontend, migrating from Tailwind-centric styling to a unified CSS variable-driven theme system. The changes include new MUI theme configuration, extensive utility classes, refactored components using inline styles, a new forgot-password page, and updated authentication/feature pages for consistency. ChangesDesign System & Theme Migration
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant ThemeContext
participant CSSVariables
participant Component
User->>App: Load page
App->>ThemeContext: Read theme from localStorage (gt-theme)
ThemeContext->>ThemeContext: Initialize mode (light/dark)
ThemeContext->>CSSVariables: Toggle dark class on documentElement
App->>Component: Render with CSS variables
Component->>Component: Use var(--color-bg), var(--color-text)
User->>App: Toggle theme
App->>ThemeContext: Call toggleTheme()
ThemeContext->>CSSVariables: Update dark class & localStorage
Component->>Component: CSS transitions apply smoothly
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @firoziya for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Contributors/Contributors.tsx (1)
67-67:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInconsistent theming approach - still using Tailwind dark mode classes.
This line uses Tailwind's
dark:utilities (bg-white dark:bg-gray-900 text-black dark:text-white) while the rest of the PR has migrated to CSS variables (var(--color-bg),var(--color-text), etc.). This is inconsistent with the design system refactor objectives.🎨 Proposed fix to align with CSS variable theming
- <div className="bg-white dark:bg-gray-900 text-black dark:text-white min-h-screen p-4 mt-4"> + <div className="min-h-screen transition-theme" style={{ backgroundColor: "var(--color-bg)", color: "var(--color-text)", padding: "16px", marginTop: "16px" }}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Contributors/Contributors.tsx` at line 67, Replace the Tailwind dark-mode utility classes on the top-level container in Contributors.tsx (the div with className "bg-white dark:bg-gray-900 text-black dark:text-white min-h-screen p-4 mt-4") with the design-system CSS variable equivalents; use var(--color-bg) and var(--color-text) (and preserve spacing utilities like min-h-screen, p-4, mt-4 or move them to appropriate utility classes) so the component reads styles from the theme variables instead of Tailwind dark: classes and remains consistent with the refactor.
🧹 Nitpick comments (1)
src/components/Navbar.tsx (1)
53-59: ⚡ Quick winDeduplicate the repeated nav item list used in desktop and mobile menus.
Line 53 and Line 140 duplicate the same route config; extract one shared constant to prevent future drift.
♻️ Proposed refactor
+const NAV_ITEMS = [ + { to: "/", label: "Home" }, + { to: "/track", label: "Tracker" }, + { to: "/contributors", label: "Contributors" }, + { to: "/about", label: "About" }, + { to: "/contact", label: "Contact" }, +]; ... - {[ - { to: "/", label: "Home" }, - { to: "/track", label: "Tracker" }, - { to: "/contributors", label: "Contributors" }, - { to: "/about", label: "About" }, - { to: "/contact", label: "Contact" }, - ].map(({ to, label }) => ( + {NAV_ITEMS.map(({ to, label }) => ( ... - {[ - { to: "/", label: "Home" }, - { to: "/track", label: "Tracker" }, - { to: "/contributors", label: "Contributors" }, - { to: "/about", label: "About" }, - { to: "/contact", label: "Contact" }, - ].map(({ to, label }) => ( + {NAV_ITEMS.map(({ to, label }) => (Also applies to: 140-146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Navbar.tsx` around lines 53 - 59, Extract the repeated inline array of route objects into a single shared constant (e.g., NAV_ITEMS or NAV_ROUTES) at the top of the Navbar component file and use that constant in both places where the array is currently inlined (the map call inside the desktop menu and the map call inside the mobile menu); update both occurrences (the map that destructures { to, label } around lines where the array appears) to reference the new constant so the route config is defined once and cannot drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/context/ThemeContext.tsx`:
- Around line 82-83: The persisted theme read into stored from localStorage
("gt-theme") is being force-cast to ThemeMode and can produce invalid state;
update the ThemeContext initialization (where stored and useState<ThemeMode> are
defined) to validate stored against the allowed ThemeMode values (e.g., "light"
and "dark") before calling setMode or passing to useState, and fall back to the
default "light" when the value is missing or invalid—implement this by adding a
small validator (isValidThemeMode or similar) that checks the string and use its
result when initializing mode and setMode.
In `@src/index.css`:
- Around line 40-44: The current global rules (::-webkit-scrollbar and * {
-ms-overflow-style, scrollbar-width }) hide scrollbars everywhere; change this
to a scoped utility so scrollbars remain available for content-heavy containers:
remove the global "*" rule and the global ::-webkit-scrollbar usage, add a
specific class name (e.g., .no-scrollbar) that contains the three vendor rules
(::‑webkit‑scrollbar display:none, -ms-overflow-style:none,
scrollbar-width:none), and update the markup to apply .no-scrollbar only to the
specific element(s) that should hide scrollbars (instead of globally in
src/index.css).
In `@src/pages/Contributors/Contributors.tsx`:
- Around line 140-142: The small, invisible, and inappropriate personal-credit
footer inside the Contributors component should be removed or relocated: delete
the inline <div> that currently renders "Frontend UI Redesigned by
github.com/Firoziya" in Contributors.tsx (the centered div with style {
textAlign: "center", padding: "20px", color: "var(--color-bg)", fontSize: "5px"
}) or move it into a proper attribution area (app Footer or About page) with
accessible styling (>=14px font, contrast-friendly color) instead of leaving it
in the Contributors component.
- Around line 123-130: Replace the hard-coded hex colors used in the sx prop for
the GitHub button in Contributors.tsx with CSS variables or theme tokens so the
button respects light/dark theming; specifically update the sx object on the
GitHub button (the object containing backgroundColor: "`#333333`", "&:hover": {
backgroundColor: "`#555555`" }, color: "`#FFFFFF`") to reference CSS variables (or
MUI theme palette values) such as var(--button-bg), var(--button-hover) and
var(--text-primary) (or theme.palette.*) instead of literal hex strings,
ensuring the hover key and color key are updated too so the button responds to
theme changes.
- Around line 76-90: The Card component's sx prop contains hard-coded hex colors
(border: "`#E0E0E0`", hover borderColor "`#C0C0C0`", outlineColor "`#B3B3B3`") —
update these to use the project's theme/CSS variables instead: replace the
static values in the sx object on the Card (and the nested "&:hover" block) with
the appropriate CSS vars or theme tokens (e.g., theme.palette.divider or CSS
vars like var(--border-color), var(--border-hover), var(--outline-color) /
var(--shadow-color)) so styling follows the global theming system.
In `@src/pages/ForgotPassword/Forgotpassword.tsx`:
- Around line 10-17: The current handleSubmit function only simulates a delay
and never calls the backend; replace the mock with a real API call to your
password-reset endpoint (e.g., POST to /api/auth/forgot-password) using
fetch/axios inside handleSubmit, keep using
setIsLoading(true)/setIsLoading(false) around the call, update
setSubmitted(true) only on a successful 2xx response, and handle non-2xx
responses and network errors by setting an error state (or showing a message) so
failures are surfaced to the user; ensure the request sends the form payload
(email) and that you parse and handle the response body for success/error
messages.
In `@src/pages/Login/Login.tsx`:
- Line 114: The form currently has the noValidate attribute which prevents
browser/enforced required and constraint checks from running before
handleSubmit; remove the noValidate attribute from the <form> element so
built-in HTML validation runs, or if you prefer explicit validation keep
noValidate but add client-side checks inside handleSubmit (or a validate
function called before API calls) to verify required fields and patterns (e.g.,
email/name/password) and prevent submission when invalid.
In `@src/pages/Signup/Signup.tsx`:
- Around line 29-37: The form currently posts regardless of whether the password
meets the displayed rules; add a pre-submit guard in handleSubmit that validates
formData.password against the UI rules (e.g., length, uppercase, lowercase,
number, special char) before calling axios.post, and if validation fails
setMessage to a clear validation error, setIsLoading(false), and return early;
apply the same pre-submit validation to the other signup submit handler that
also posts to /api/auth/signup so both handlers enforce the displayed password
requirements.
In `@src/pages/Tracker/Tracker.tsx`:
- Line 45: Rename the React component from Home to Tracker to match the file
name and update the export accordingly: locate the functional component
declaration "const Home: React.FC = () => {", rename it to "const Tracker:
React.FC = () => {" and update any corresponding default or named export (e.g.,
export default Home -> export default Tracker) plus any local references to Home
within this file.
- Around line 58-60: The useEffect invoking fetchData(username, page + 1,
ROWS_PER_PAGE) is missing username in its dependency array; update the
dependency array of the useEffect that references useEffect, fetchData, tab,
page, and username to include username so the effect reruns when username
changes, and if fetchData is not stable memoize it with useCallback (or
otherwise ensure fetchData is included as a dependency) to avoid stale closures.
In `@src/Routes/Router.tsx`:
- Line 8: The import for the ForgotPassword component has incorrect filename
casing; update the import statement that pulls in ForgotPassword (the imported
symbol/identifier ForgotPassword in Router.tsx) to reference the actual file
name casing "Forgotpassword.tsx" instead of "ForgotPassword.tsx" so module
resolution succeeds on case-sensitive filesystems. Ensure the import path
matches the on-disk filename exactly and rebuild to verify resolution.
---
Outside diff comments:
In `@src/pages/Contributors/Contributors.tsx`:
- Line 67: Replace the Tailwind dark-mode utility classes on the top-level
container in Contributors.tsx (the div with className "bg-white dark:bg-gray-900
text-black dark:text-white min-h-screen p-4 mt-4") with the design-system CSS
variable equivalents; use var(--color-bg) and var(--color-text) (and preserve
spacing utilities like min-h-screen, p-4, mt-4 or move them to appropriate
utility classes) so the component reads styles from the theme variables instead
of Tailwind dark: classes and remains consistent with the refactor.
---
Nitpick comments:
In `@src/components/Navbar.tsx`:
- Around line 53-59: Extract the repeated inline array of route objects into a
single shared constant (e.g., NAV_ITEMS or NAV_ROUTES) at the top of the Navbar
component file and use that constant in both places where the array is currently
inlined (the map call inside the desktop menu and the map call inside the mobile
menu); update both occurrences (the map that destructures { to, label } around
lines where the array appears) to reference the new constant so the route config
is defined once and cannot drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 720dcfef-660a-4bdf-aa0e-9d4ca74a633f
📒 Files selected for processing (19)
src/App.csssrc/App.tsxsrc/Routes/Router.tsxsrc/components/Features.tsxsrc/components/Footer.tsxsrc/components/Hero.tsxsrc/components/HowItWorks.tsxsrc/components/Navbar.tsxsrc/components/ScrollProgressBar.tsxsrc/context/ThemeContext.tsxsrc/index.csssrc/pages/About/About.tsxsrc/pages/Contact/Contact.tsxsrc/pages/Contributors/Contributors.tsxsrc/pages/ForgotPassword/Forgotpassword.tsxsrc/pages/Home/Home.tsxsrc/pages/Login/Login.tsxsrc/pages/Signup/Signup.tsxsrc/pages/Tracker/Tracker.tsx
💤 Files with no reviewable changes (1)
- src/pages/Home/Home.tsx
Comment on lines
+82
to
+83
| const stored = typeof window !== "undefined" ? localStorage.getItem("gt-theme") : null; | ||
| const [mode, setMode] = useState<ThemeMode>((stored as ThemeMode) || "light"); |
Contributor
There was a problem hiding this comment.
Validate persisted theme value before assigning state.
localStorage.getItem("gt-theme") is force-cast to ThemeMode. Any unexpected value can bypass type safety and put mode in an invalid state.
Suggested fix
- const stored = typeof window !== "undefined" ? localStorage.getItem("gt-theme") : null;
- const [mode, setMode] = useState<ThemeMode>((stored as ThemeMode) || "light");
+ const getInitialMode = (): ThemeMode => {
+ if (typeof window === "undefined") return "light";
+ const stored = localStorage.getItem("gt-theme");
+ return stored === "light" || stored === "dark" ? stored : "light";
+ };
+ const [mode, setMode] = useState<ThemeMode>(getInitialMode);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/context/ThemeContext.tsx` around lines 82 - 83, The persisted theme read
into stored from localStorage ("gt-theme") is being force-cast to ThemeMode and
can produce invalid state; update the ThemeContext initialization (where stored
and useState<ThemeMode> are defined) to validate stored against the allowed
ThemeMode values (e.g., "light" and "dark") before calling setMode or passing to
useState, and fall back to the default "light" when the value is missing or
invalid—implement this by adding a small validator (isValidThemeMode or similar)
that checks the string and use its result when initializing mode and setMode.
Comment on lines
+40
to
+44
| /* remove scroll bar for all browsers */ | ||
| ::-webkit-scrollbar { display: none; } | ||
| * { -ms-overflow-style: none; /* IE and Edge */ | ||
| scrollbar-width: none; /* Firefox */ | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid globally hiding all scrollbars.
This removes scrollbar visibility everywhere, including content-heavy/overflow containers, which creates an accessibility and usability regression.
Suggested fix
-/* remove scroll bar for all browsers */
-::-webkit-scrollbar { display: none; }
-* { -ms-overflow-style: none; /* IE and Edge */
- scrollbar-width: none; /* Firefox */
-}
+/* Keep native scrollbars globally for accessibility.
+ If needed, hide only in explicitly opted-in containers. */
+.gt-scrollbar-hidden::-webkit-scrollbar { display: none; }
+.gt-scrollbar-hidden {
+ -ms-overflow-style: none;
+ scrollbar-width: none;
+}📝 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.
Suggested change
| /* remove scroll bar for all browsers */ | |
| ::-webkit-scrollbar { display: none; } | |
| * { -ms-overflow-style: none; /* IE and Edge */ | |
| scrollbar-width: none; /* Firefox */ | |
| } | |
| /* Keep native scrollbars globally for accessibility. | |
| If needed, hide only in explicitly opted-in containers. */ | |
| .gt-scrollbar-hidden::-webkit-scrollbar { display: none; } | |
| .gt-scrollbar-hidden { | |
| -ms-overflow-style: none; | |
| scrollbar-width: none; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.css` around lines 40 - 44, The current global rules
(::-webkit-scrollbar and * { -ms-overflow-style, scrollbar-width }) hide
scrollbars everywhere; change this to a scoped utility so scrollbars remain
available for content-heavy containers: remove the global "*" rule and the
global ::-webkit-scrollbar usage, add a specific class name (e.g.,
.no-scrollbar) that contains the three vendor rules (::‑webkit‑scrollbar
display:none, -ms-overflow-style:none, scrollbar-width:none), and update the
markup to apply .no-scrollbar only to the specific element(s) that should hide
scrollbars (instead of globally in src/index.css).
Comment on lines
+76
to
+90
| <Card | ||
| sx={{ | ||
| textAlign: "center", | ||
| p: 2, | ||
| borderRadius: "10px", | ||
| border: "1px solid #E0E0E0", | ||
| boxShadow: "0 4px 10px rgba(0,0,0,0.1)", | ||
| transition: "transform 0.3s ease-in-out", | ||
| "&:hover": { | ||
| transform: "scale(1.05)", | ||
| boxShadow: "0 8px 15px rgba(0,0,0,0.2)", | ||
| borderColor: "#C0C0C0", | ||
| outlineColor: "#B3B3B3", | ||
| }, | ||
| }} |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Hard-coded colors instead of CSS variables.
The Card component uses hard-coded hex color values (#E0E0E0, #C0C0C0, #B3B3B3) for borders and shadows. This is inconsistent with the CSS variable-based theming system used throughout the rest of this PR.
🎨 Proposed refactor to use CSS variables
<Card
sx={{
textAlign: "center",
p: 2,
borderRadius: "10px",
- border: "1px solid `#E0E0E0`",
- boxShadow: "0 4px 10px rgba(0,0,0,0.1)",
+ border: "1px solid var(--color-border)",
+ boxShadow: "var(--shadow-md)",
transition: "transform 0.3s ease-in-out",
"&:hover": {
transform: "scale(1.05)",
- boxShadow: "0 8px 15px rgba(0,0,0,0.2)",
- borderColor: "`#C0C0C0`",
- outlineColor: "`#B3B3B3`",
+ boxShadow: "var(--shadow-lg)",
+ borderColor: "var(--color-border-hover, var(--color-text-3))",
},
}}
>📝 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.
Suggested change
| <Card | |
| sx={{ | |
| textAlign: "center", | |
| p: 2, | |
| borderRadius: "10px", | |
| border: "1px solid #E0E0E0", | |
| boxShadow: "0 4px 10px rgba(0,0,0,0.1)", | |
| transition: "transform 0.3s ease-in-out", | |
| "&:hover": { | |
| transform: "scale(1.05)", | |
| boxShadow: "0 8px 15px rgba(0,0,0,0.2)", | |
| borderColor: "#C0C0C0", | |
| outlineColor: "#B3B3B3", | |
| }, | |
| }} | |
| <Card | |
| sx={{ | |
| textAlign: "center", | |
| p: 2, | |
| borderRadius: "10px", | |
| border: "1px solid var(--color-border)", | |
| boxShadow: "var(--shadow-md)", | |
| transition: "transform 0.3s ease-in-out", | |
| "&:hover": { | |
| transform: "scale(1.05)", | |
| boxShadow: "var(--shadow-lg)", | |
| borderColor: "var(--color-border-hover, var(--color-text-3))", | |
| }, | |
| }} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Contributors/Contributors.tsx` around lines 76 - 90, The Card
component's sx prop contains hard-coded hex colors (border: "`#E0E0E0`", hover
borderColor "`#C0C0C0`", outlineColor "`#B3B3B3`") — update these to use the
project's theme/CSS variables instead: replace the static values in the sx
object on the Card (and the nested "&:hover" block) with the appropriate CSS
vars or theme tokens (e.g., theme.palette.divider or CSS vars like
var(--border-color), var(--border-hover), var(--outline-color) /
var(--shadow-color)) so styling follows the global theming system.
Comment on lines
+123
to
+130
| sx={{ | ||
| backgroundColor: "#333333", | ||
| textTransform: "none", | ||
| color: "#FFFFFF", | ||
| "&:hover": { | ||
| backgroundColor: "#555555", | ||
| }, | ||
| }} |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Hard-coded button colors instead of CSS variables.
The GitHub button uses hard-coded hex colors (#333333, #555555, #FFFFFF) instead of CSS variables. This breaks theme consistency and won't respond to light/dark mode changes.
🎨 Proposed refactor to use CSS variables
<Button
variant="contained"
startIcon={<FaGithub />}
href={contributor.html_url}
target="_blank"
sx={{
- backgroundColor: "`#333333`",
+ backgroundColor: "var(--color-accent)",
textTransform: "none",
- color: "`#FFFFFF`",
+ color: "var(--color-bg)",
"&:hover": {
- backgroundColor: "`#555555`",
+ backgroundColor: "var(--color-accent-2)",
},
}}
>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Contributors/Contributors.tsx` around lines 123 - 130, Replace the
hard-coded hex colors used in the sx prop for the GitHub button in
Contributors.tsx with CSS variables or theme tokens so the button respects
light/dark theming; specifically update the sx object on the GitHub button (the
object containing backgroundColor: "`#333333`", "&:hover": { backgroundColor:
"`#555555`" }, color: "`#FFFFFF`") to reference CSS variables (or MUI theme palette
values) such as var(--button-bg), var(--button-hover) and var(--text-primary)
(or theme.palette.*) instead of literal hex strings, ensuring the hover key and
color key are updated too so the button responds to theme changes.
Comment on lines
+140
to
+142
| <div style={{ textAlign: "center", padding: "20px", color: "var(--color-bg)", fontSize: "5px" }}> | ||
| Frontend UI Redesigned by github.com/Firoziya | ||
| </div> |
Contributor
There was a problem hiding this comment.
Footer has illegible styling and questionable content.
Multiple issues with this footer:
fontSize: "5px"is illegibly small (typical readable text is 14-16px minimum)color: "var(--color-bg)"uses the background color for text, making it effectively invisible on the background- Personal credit messages typically don't belong in production code
🗑️ Suggested action
Remove this footer entirely, or if credits are needed, add them to a proper attribution section with readable styling:
- <div style={{ textAlign: "center", padding: "20px", color: "var(--color-bg)", fontSize: "5px" }}>
- Frontend UI Redesigned by github.com/Firoziya
- </div>If attribution is required, consider adding it to the app footer component or an About page section instead.
📝 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.
Suggested change
| <div style={{ textAlign: "center", padding: "20px", color: "var(--color-bg)", fontSize: "5px" }}> | |
| Frontend UI Redesigned by github.com/Firoziya | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Contributors/Contributors.tsx` around lines 140 - 142, The small,
invisible, and inappropriate personal-credit footer inside the Contributors
component should be removed or relocated: delete the inline <div> that currently
renders "Frontend UI Redesigned by github.com/Firoziya" in Contributors.tsx (the
centered div with style { textAlign: "center", padding: "20px", color:
"var(--color-bg)", fontSize: "5px" }) or move it into a proper attribution area
(app Footer or About page) with accessible styling (>=14px font,
contrast-friendly color) instead of leaving it in the Contributors component.
| }`} | ||
| /> | ||
| {/* Form */} | ||
| <form onSubmit={handleSubmit} className="space-y-5" noValidate> |
Contributor
There was a problem hiding this comment.
noValidate disables the required checks you added.
With noValidate on the form, empty/invalid inputs can still submit and only fail after the API call. Remove it (or add explicit client-side checks) so field constraints run before submit.
Suggested fix
- <form onSubmit={handleSubmit} className="space-y-5" noValidate>
+ <form onSubmit={handleSubmit} className="space-y-5">📝 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.
Suggested change
| <form onSubmit={handleSubmit} className="space-y-5" noValidate> | |
| <form onSubmit={handleSubmit} className="space-y-5"> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Login/Login.tsx` at line 114, The form currently has the noValidate
attribute which prevents browser/enforced required and constraint checks from
running before handleSubmit; remove the noValidate attribute from the <form>
element so built-in HTML validation runs, or if you prefer explicit validation
keep noValidate but add client-side checks inside handleSubmit (or a validate
function called before API calls) to verify required fields and patterns (e.g.,
email/name/password) and prevent submission when invalid.
Comment on lines
29
to
+37
| const handleSubmit = async (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| setIsLoading(true); | ||
| setMessage(""); | ||
| try { | ||
| const response = await axios.post( | ||
| `${backendUrl}/api/auth/signup`, | ||
| formData | ||
| ); | ||
|
|
||
| setMessage(response.data.message); | ||
|
|
||
| if (response.data.message === "User created successfully") { | ||
| navigate("/login"); | ||
| } | ||
| } catch (error) { | ||
| const res = await axios.post(`${backendUrl}/api/auth/signup`, formData); | ||
| setMessage(res.data.message); | ||
| if (res.data.message === "User created successfully") navigate("/login"); | ||
| } catch { |
Contributor
There was a problem hiding this comment.
Password requirements are informational only; submission still proceeds when they fail.
The UI communicates password rules, but handleSubmit does not enforce them before calling signup. Add a pre-submit guard so behavior matches the displayed rules.
Suggested fix
const handleSubmit = async (e: React.FormEvent) => {
e.preventDefault();
+ const passwordValid = passwordRules.every((r) => r.test(formData.password));
+ if (!passwordValid) {
+ setMessage("Password does not meet the required rules.");
+ return;
+ }
setIsLoading(true);
setMessage("");
try {
const res = await axios.post(`${backendUrl}/api/auth/signup`, formData);Also applies to: 174-189
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Signup/Signup.tsx` around lines 29 - 37, The form currently posts
regardless of whether the password meets the displayed rules; add a pre-submit
guard in handleSubmit that validates formData.password against the UI rules
(e.g., length, uppercase, lowercase, number, special char) before calling
axios.post, and if validation fails setMessage to a clear validation error,
setIsLoading(false), and return early; apply the same pre-submit validation to
the other signup submit handler that also posts to /api/auth/signup so both
handlers enforce the displayed password requirements.
| @@ -46,306 +43,357 @@ interface GitHubItem { | |||
| } | |||
|
|
|||
| const Home: React.FC = () => { | |||
Contributor
There was a problem hiding this comment.
Component name doesn't match filename.
The component is named Home but the file is Tracker.tsx. This naming inconsistency can confuse developers and make the codebase harder to maintain.
📝 Proposed fix
-const Home: React.FC = () => {
+const Tracker: React.FC = () => {And update the export:
-export default Home;
+export default Tracker;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Tracker/Tracker.tsx` at line 45, Rename the React component from
Home to Tracker to match the file name and update the export accordingly: locate
the functional component declaration "const Home: React.FC = () => {", rename it
to "const Tracker: React.FC = () => {" and update any corresponding default or
named export (e.g., export default Home -> export default Tracker) plus any
local references to Home within this file.
Comment on lines
58
to
60
| useEffect(() => { | ||
| if (username) { | ||
| fetchData(username, page + 1, ROWS_PER_PAGE); | ||
| } | ||
| if (username) fetchData(username, page + 1, ROWS_PER_PAGE); | ||
| }, [tab, page]); |
Contributor
There was a problem hiding this comment.
Missing username in useEffect dependency array.
The effect calls fetchData(username, ...) but username is not included in the dependency array [tab, page]. This violates React's rules of hooks and means the effect won't re-run when username changes, potentially causing stale data to be displayed.
🔧 Proposed fix
useEffect(() => {
if (username) fetchData(username, page + 1, ROWS_PER_PAGE);
- }, [tab, page]);
+ }, [tab, page, username, fetchData]);Note: If fetchData is not memoized with useCallback, you should either wrap it or only include the dependencies it uses.
📝 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.
Suggested change
| useEffect(() => { | |
| if (username) { | |
| fetchData(username, page + 1, ROWS_PER_PAGE); | |
| } | |
| if (username) fetchData(username, page + 1, ROWS_PER_PAGE); | |
| }, [tab, page]); | |
| useEffect(() => { | |
| if (username) fetchData(username, page + 1, ROWS_PER_PAGE); | |
| }, [tab, page, username, fetchData]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Tracker/Tracker.tsx` around lines 58 - 60, The useEffect invoking
fetchData(username, page + 1, ROWS_PER_PAGE) is missing username in its
dependency array; update the dependency array of the useEffect that references
useEffect, fetchData, tab, page, and username to include username so the effect
reruns when username changes, and if fetchData is not stable memoize it with
useCallback (or otherwise ensure fetchData is included as a dependency) to avoid
stale closures.
Collaborator
|
@firoziya : dont want to re design everything, you can split PR into page wise changes... |
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
2 participants
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.
Related Issue
Description
Re-Design the frontend UI of the complete app.
simple and minimal light and dark theme.
How Has This Been Tested?
Local host Screenshots added.
Screenshots (if applicable)
Type of Change
Summary by CodeRabbit
New Features
Style
Improvements