Origin/implement login page/ login signup pages components #78
Conversation
reconfigured layout.tsx to avoid re-creating it on each render
login and sign up pages
isolated the components from the full page.tsx
Refactor the Signup page by isolating form-related logic and UI elements into modular components. Introduced components in the form, layout, and ui directories for better code organization.
update page.tsx for signup
There was a problem hiding this comment.
Help to remove empty files
There was a problem hiding this comment.
im not sure which empty file are you referring to. can you highlight it again
There was a problem hiding this comment.
not sure which empty file are you referring to
There was a problem hiding this comment.
these files are in your local machine but there are not commited and pushed to the branch on GitHub. You should check when you create a PR.
There was a problem hiding this comment.
Also, please fix all the red lines above. They are errors and even if your app works, they can cause probs in the future.
|
|
||
| return ( | ||
| <PageLayout title={loading ? "Processing" : "Login"} loading={loading}> | ||
| <LoginForm onLogin={onLogin} /> |
There was a problem hiding this comment.
Is this importing an empty component?
There was a problem hiding this comment.
No need to import this form component since you already have form input fields in the same file
There was a problem hiding this comment.
PageLayout is from flowforge\frontend\src\components\layouts\pageLayout.tsx
while LoginForm is from flowforge\frontend\src\components\form\loginForm.tsx
I've recommitted the changes to origin again. maybe the first commit did not register the saved changes.
There was a problem hiding this comment.
Your new commit only pushed inputField.tsx. The other files are still empty
| return ( | ||
| <PageLayout title={loading ? "Processing" : "Login"} loading={loading}> | ||
| <LoginForm onLogin={onLogin} /> | ||
| <InputField |
There was a problem hiding this comment.
inputField is a component I created under frontend\src\components\ui\inputField.tsx
There was a problem hiding this comment.
inputField.tsx, component under flowforge\frontend\src\components\ui\inputField.tsx
There was a problem hiding this comment.
shld I remove the components ive created and instead utilise the existing ones? thought its easier since its kind of different. lmk thanks!
There was a problem hiding this comment.
Yes please use the shadcn ui components we have for consistency. It is easier to implement with them as compared to vanilla HTML since its a library with pre-coded logic. We don't want to introduce other libs also.
| import Link from "next/link"; | ||
| import React, { useState } from "react"; | ||
| import axios from "axios"; | ||
| import { toast } from "react-hot-toast"; |
There was a problem hiding this comment.
Don't use another toast library. There is already an existing toast component you can use. Also in my opinion, no need to use toast here. Simply login on success or display error below the form field if any. This can be done with the existing shadcn ui form component (Guide)
| onChange={(value) => setUser({ ...user, password: value })} | ||
| placeholder="password" | ||
| /> | ||
| <button |
There was a problem hiding this comment.
Use our existing shadcn ui button component for consistent UI
updated some of the PR issues highlighted
updated according to comments in PR #78
| @@ -0,0 +1,19 @@ | |||
| import React from "react"; | |||
|
|
|||
| const InputField = ({ id, type, value, onChange, placeholder }) => { | |||
There was a problem hiding this comment.
We are using Typescript so this should flag out an error since the props type is not defined
| return ( | ||
| <div> | ||
| <label htmlFor={id}>{id}</label> | ||
| <input |
There was a problem hiding this comment.
try to use the shadcn ui components we have. Can look here for details on how to use.
There was a problem hiding this comment.
loginForm.tsx, signupForm.tsx and pageLayout.tsx are still empty files
|
Still an incomplete PR. Please check if your code works in your local machine too |
| const onLogin = async (user) => { | ||
| try { | ||
| setLoading(true); | ||
| const response = await axios.post("/api/users/login", user); |
There was a problem hiding this comment.
Why are you usin axios already when there is no backend api for login? /api/users/login does not exist. Please remove this to prevent invalid and unnecessary API calls.
created login page and signup page.tsx, isolated components under components directory
Closes #7