-
Notifications
You must be signed in to change notification settings - Fork 0
divie between register to sort day and regesteration to the website #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
|
|
||
|
|
||
| export let analytics; | ||
| if (typeof window !== 'undefined') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to convert this code into async await just like we are doing inside the entire code base
| // validator: passwordValidation, | ||
| // }, | ||
| { | ||
| label: "Password", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check with one of the other mentors that this is ok to un-comment
| return parsedData; | ||
| } catch (error) {} | ||
| const parsed = parseCsv(csvData); | ||
| console.log('[useSheets] parsed rows:', parsed.length, parsed.slice(0, 3)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the console
| console.log('[useSheets] parsed rows:', parsed.length, parsed.slice(0, 3)); | ||
| return parsed; | ||
| } catch (error) { | ||
| console.error('[useSheets] fetch failed:', error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the console
|
|
||
| const parseCsv = (csvData) => { | ||
| return csvData.split('\n').map((row) => row.split(/","/)); | ||
| return csvData |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if we can parse the file something like this :
return data.split(/n).map((row) => row.split(/","/)| queryKey: ['googleSheetsData'], | ||
| queryFn: fetchDataFromCsv, | ||
| refetchOnWindowFocus: false, | ||
| staleTime: 5 * 60 * 1000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number should be saved inside a const variable
For example:
const VARIABLE_NAME = 5
And so on
| import Avatar from '@mui/material/Avatar'; | ||
| import { useCreateUser } from 'src/hooks/firebase.hooks'; | ||
|
|
||
| // ✅ ייבוא ישיר לבדיקה יעילה ב-Firestore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
|
|
||
| const createUser = useCreateUser(); | ||
|
|
||
| const onSignupHandler = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if you can push your changed into smaller helper function
| if (!rules) return; | ||
|
|
||
| try { | ||
| const emailToCheck = String(formValues.email || '').trim().toLowerCase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use existing validations, also the formValues should already contain valid email because otherwise we cannot validate the form - check it
| if (!emailToCheck) return; | ||
|
|
||
| if (validationState[key]) { | ||
| const q = query( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire part could be extracted into a service that handles all of the users.
| paddingBottom: '3rem', | ||
|
|
||
| }} | ||
| sx={{ paddingTop: '3rem', paddingBottom: '3rem' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally its not recommended to mix inline style into the jsx.
You can use external file to save those styles.
| })} | ||
| </Box> | ||
|
|
||
| {/* מודל הצלחה */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comment
| fieldOfStudy: '', | ||
| }); | ||
|
|
||
| const setField = (key) => (e) => setForm((p) => ({ ...p, [key]: e.target.value })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we can have a better name for this ? p is a bit confusing in this context
| const q = query( | ||
| collection(db, COLLECTION), | ||
| where('email', '==', normalized.email), | ||
| limit(1) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should come from the user service that we talked about before
Creating this pr for noam for cr