BUG: #133 - Fix frontend issues #134
Conversation
- Add missing clsx dependency to package.json - Fix TypeScript types: replace 'any' with proper RepositoryData interface - Remove console.log statements from production code - Rename ForgotPasswrdPage.tsx to ForgotPasswordPage.tsx (fix typo) - Consolidate utility functions: remove duplicate cn() function from helpers.ts - Update import statements to use correct filename Resolves AOSSIE-Org#133
WalkthroughAdds clsx dependency. Updates App.tsx to use ForgotPasswordPage, introduces RepositoryData interface, and narrows repoData typing. Renames exported component in ForgotPasswordPage.tsx and removes console error logs. Removes cn utility and a stray console.log from helpers.ts. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 0
🧹 Nitpick comments (4)
frontend/src/components/pages/ForgotPasswordPage.tsx (1)
36-46: Bug: InputField overrides caller’s className (e.g.,mb-7lost)Because
{...props}precedes a hardcodedclassName, anyclassNamepassed by the caller is ignored. Merge classes instead.Apply this diff:
-const InputField = ({ icon: Icon, ...props }: InputFieldProps) => ( +const InputField = ({ icon: Icon, className, ...props }: InputFieldProps) => ( <div className="relative"> <div className="absolute inset-y-0 left-0 pl-3 flex items-center pointer-events-none"> <Icon className="h-5 w-5 text-gray-400" /> </div> <input - {...props} - className="block w-full pl-10 pr-3 py-2 border border-gray-800 rounded-lg bg-gray-900 text-white placeholder-gray-400 focus:outline-none focus:ring-2 focus:ring-green-500 focus:border-transparent" + {...props} + className={twMerge( + "block w-full pl-10 pr-3 py-2 border border-gray-800 rounded-lg bg-gray-900 text-white placeholder-gray-400 focus:outline-none focus:ring-2 focus:ring-green-500 focus:border-transparent", + className + )} /> </div> );Also add the import at the top of this file:
import { twMerge } from "tailwind-merge";frontend/src/App.tsx (3)
22-38: RepositoryData inline type — consider centralizingLooks good. Consider moving this interface to a shared
typesmodule for reuse across pages.
56-56: Simplify Supabase auth subscription cleanupUse the canonical destructuring to avoid the double
.subscriptionaccess.Apply this diff:
- const { data: subscription } = supabase.auth.onAuthStateChange( + const { data: { subscription } } = supabase.auth.onAuthStateChange( (event, session) => { switch (event) { ... - return () => { - subscription.subscription.unsubscribe(); - }; + return () => { + subscription.unsubscribe(); + };Also applies to: 83-85
49-51: Avoidconsole.errorin production UI pathsYou already surface errors via toasts. Drop
console.erroror route it through a logger behind an env‑guard if needed.Apply this diff:
if (error) { toast.error('User Login Failed'); - console.error('Error checking session:', error); return; }if (error) { toast.error('Logout failed'); - console.error('Error during logout:', error); return; }Also applies to: 94-97
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/package.json(1 hunks)frontend/src/App.tsx(3 hunks)frontend/src/components/pages/ForgotPasswordPage.tsx(1 hunks)frontend/src/components/utils/helpers.ts(0 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/components/utils/helpers.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/App.tsx (1)
frontend/src/components/pages/ForgotPasswordPage.tsx (1)
ForgotPasswordPage(49-154)
🔇 Additional comments (3)
frontend/src/components/pages/ForgotPasswordPage.tsx (1)
49-49: Rename to ForgotPasswordPage — LGTMMatches route/import updates in App.tsx.
frontend/src/App.tsx (1)
19-19: Route/import and typing updates — LGTM
- ForgotPasswordPage import and route wiring look correct.
repoDatanarrowed toRepositoryData | nullis a solid improvement.Also applies to: 42-42, 146-146
frontend/package.json (1)
15-15: clsx added — cn helper present but no residual usages detectedFound frontend/src/lib/utils.ts exporting function cn(...): returns twMerge(clsx(inputs)); no other cn imports/usages across .ts/.tsx/.js/.jsx files; clsx and tailwind-merge are imported and used.
|
Merged, Cheers @Aditya30ag !! |
|
🎉 |
2 participants
🐛 Bug Fix: #133 - Fix frontend code quality and dependency issues
This PR resolves issue #133 by fixing multiple frontend code quality and dependency issues.
🔧 Changes Made
clsx@^2.1.1topackage.jsonanytype with properRepositoryDatainterfaceForgotPasswrdPage.tsx→ForgotPasswordPage.tsxcn()function fromhelpers.ts📝 Files Changed
frontend/package.json- Added clsx dependencyfrontend/src/App.tsx- Fixed types and removed console.logsfrontend/src/components/pages/ForgotPasswordPage.tsx- Renamed and cleaned upfrontend/src/components/utils/helpers.ts- Removed duplicate functionFixes #133
Summary by CodeRabbit
Bug Fixes
Refactor
Chores