Skip to content

Feature/auth#92

Open
mariam123hagras wants to merge 5 commits intoplatform/webfrom
feature/auth
Open

Feature/auth#92
mariam123hagras wants to merge 5 commits intoplatform/webfrom
feature/auth

Conversation

@mariam123hagras
Copy link
Collaborator

No description provided.

@mariam123hagras mariam123hagras changed the base branch from development to platform/web March 1, 2026 19:32
@Salint
Copy link
Member

Salint commented Mar 2, 2026

The project seems to not run because of react-hook-form

image

Also, why do we need it?

Copy link
Member

@Salint Salint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not put atoms, molecules, organisms, templates, and pages directly in the root of src/. Instead, put them in components/ or ui/ to clean up the code.

Additionally, a lot of the code was overengineered, and was written by AI. There's much simpler and easier approaches.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why create a definitions file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change the routing? Use createBrowserRouter. It's the newer and recommended way to do routing using react-router.

<Page>
<Card>
<Logo subtitle={subtitle} />
<ContentArea>{children}</ContentArea>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use <Outlet /> with layouts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this file deleted?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a global index.css file with our colors like in index.css.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A form is not a molecule, its an organism.

Comment on lines +9 to +18
export interface AuthContextType {
user: User | null
loading: boolean
error: string | null
loginWithEmail: (email: string, password: string) => Promise<void>
registerWithEmail: (email: string, password: string) => Promise<void>
loginWithGoogle: () => Promise<void>
logout: () => Promise<void>
clearError: () => void
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does a context have a type?

export const auth = getAuth(app);
export const db = getFirestore(app);
export const analytics = getAnalytics(app);
export const googleProvider = new GoogleAuthProvider();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The google provider does not belong here. Imagine if we had a lot of providers.

export const PrivacyNote: React.FC = () => (
<Text>
By continuing, you agree to our{' '}
<a href="#" target="_blank" rel="noopener noreferrer">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +15 to +34
function mapFirebaseError(error: FirebaseError): string {
switch (error.code) {
case 'auth/user-not-found':
case 'auth/wrong-password':
case 'auth/invalid-credential':
return 'Invalid email or password.'
case 'auth/email-already-in-use':
return 'This email is already registered.'
case 'auth/weak-password':
return 'Password must be at least 6 characters.'
case 'auth/invalid-email':
return 'Please enter a valid email address.'
case 'auth/too-many-requests':
return 'Too many attempts. Please try again later.'
case 'auth/popup-closed-by-user':
return 'Google sign-in was cancelled.'
default:
return 'An unexpected error occurred. Please try again.'
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not be handled in the context.

@mariam123hagras
Copy link
Collaborator Author

no promlem , i will maintain what u requested

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants