Signup/login/page #173
Signup/login/page #173
Conversation
|
@samar12-rad is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces updates to the PlayCafe website project, including modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. In the meantime, please ensure that your changes align with our CONTRIBUTING.md. If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
frontend/src/router/index.jsx (1)
32-33: LGTM: New routes added correctly. Consider route organization and error handling.The new routes for signup and login are correctly implemented. However, consider the following suggestions:
Group related routes together for better organization. For example, you might want to place login and signup routes near each other or near other user-related routes.
Ensure there's a catch-all route to handle undefined paths. This could improve user experience by redirecting to a 404 page or the home page for mistyped URLs.
Example of grouping and adding a catch-all route:
<Route path="/" element={<App />}> <Route index={true} path="/" element={<Home />} /> <Route path="/about" element={<About />} /> <Route path="/menu" element={<Menu />} /> <Route path="/boardgame" element={<Boardgame />} /> <Route path="/events" element={<Event />} /> <Route path="/book" element={<MyBook />} /> <Route path="/reservation" element={<Register />} /> {/* User-related routes grouped together */} <Route path="/signup" element={<Signup />} /> <Route path="/login" element={<Login />} /> {/* Catch-all route for undefined paths */} <Route path="*" element={<NotFound />} /> </Route>frontend/src/components/Pages/Login.jsx (1)
6-13: Consider simplifying the handleChange function.The state management looks good, but we can make the handleChange function more concise by destructuring the event object.
Here's a suggested improvement:
- const handleChange = (e) => { - setData({ ...data, [e.target.name]: e.target.value }); + const handleChange = ({ target: { name, value } }) => { + setData(prevData => ({ ...prevData, [name]: value })); };This change also ensures that we're using the most up-to-date state when updating, which is a best practice when the new state depends on the previous state.
frontend/src/components/Shared/Navbar.jsx (2)
47-53: LGTM: Improved readability of color class assignmentsThe refactoring of text color class assignments enhances code readability without altering the logic. Good use of template literals for class names.
For consistency, consider using template literals for all class assignments in this section. For example:
const baseTextColorClass = isScrolled ? `text-gray-800` : `text-gray-900`;
79-82: LGTM: Improved menu structure and navigationThe changes to the desktop menu items enhance the structure and navigation:
- The addition of the
lielement improves the structure and styling capabilities of menu items.- The use of the
navigatefunction for "Log In" and "Sign Up" buttons aligns with the PR objectives.- The new "Sign Up" button is a valuable addition to the navigation.
Consider extracting the common button styles into a separate constant to reduce repetition:
const buttonClass = `${baseTextColorClass} ${hoverTextColorClass} transform hover:scale-110 hover:-translate-y-1 transition`;Then, you can use it like this:
<button className={buttonClass} type="button" onClick={() => navigate("/login")}> Log In </button>Also applies to: 93-95, 101-103, 108-114
README.md (2)
216-222: LGTM! Minor suggestion for consistency.The addition of Jay Shah as a new contributor is well-formatted and correctly placed within the table structure. Good job on maintaining the layout consistency!
For even better consistency, consider aligning the indentation of these new lines with the existing entries in the table.
You can adjust the indentation like this:
- <td align="center"> - <a href="https://github.com/Jay-1409"> - <img src="https://avatars.githubusercontent.com/u/166749819?v=4" width="100;" alt="Jay-1409"/> - <br /> - <sub><b>Jay shah</b></sub> - </a> - </td> + <td align="center"> + <a href="https://github.com/Jay-1409"> + <img src="https://avatars.githubusercontent.com/u/166749819?v=4" width="100;" alt="Jay-1409"/> + <br /> + <sub><b>Jay shah</b></sub> + </a> + </td>
223-224: Replace hard tabs with spaces for better compatibility.Markdownlint has flagged the use of hard tabs on lines 223 and 224. To ensure consistent rendering across different platforms and editors, it's recommended to use spaces for indentation in Markdown files.
Replace the hard tabs with spaces. You can use 4 spaces for each level of indentation. For example:
- </tr> - <tr> + </tr> + <tr>Consider using a Markdown linter or editor configuration to automatically convert tabs to spaces in Markdown files.
🧰 Tools
🪛 Markdownlint
223-223: Column: 1
Hard tabs(MD010, no-hard-tabs)
224-224: Column: 1
Hard tabs(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
frontend/src/assets/login.pngis excluded by!**/*.png
📒 Files selected for processing (5)
- README.md (1 hunks)
- frontend/src/components/Pages/Login.jsx (1 hunks)
- frontend/src/components/Pages/Signup.jsx (1 hunks)
- frontend/src/components/Shared/Navbar.jsx (7 hunks)
- frontend/src/router/index.jsx (2 hunks)
🧰 Additional context used
🪛 Markdownlint
README.md
223-223: Column: 1
Hard tabs(MD010, no-hard-tabs)
224-224: Column: 1
Hard tabs(MD010, no-hard-tabs)
🔇 Additional comments (13)
frontend/src/router/index.jsx (2)
17-18: LGTM: New component imports added correctly.The Signup and Login component imports are correctly added and follow the existing import style. These imports are necessary for the new routes added later in the file.
Line range hint
1-37: Overall, the changes look good and align with the PR objectives.The new routes for signup and login pages have been correctly implemented, and the necessary component imports have been added. These changes enhance the routing capabilities of the application as intended. The suggestions provided are for further improvement and don't affect the core functionality of the changes.
frontend/src/components/Pages/Login.jsx (4)
1-4: LGTM: Imports and component declaration look good.The imports are appropriate for the component's needs, and the component is correctly declared as a functional component using arrow function syntax.
67-67: LGTM: Component export is correct.The Login component is properly exported as the default export, which is the standard practice for exporting a single component from a file.
29-63:⚠️ Potential issueEnhance accessibility and user experience of the login form.
While the form structure and styling look good, there are some accessibility improvements we can make:
- Add proper labels for input fields.
- Ensure sufficient color contrast for text and background.
- Provide more informative alt text for the image.
- Add aria-labels to improve screen reader experience.
Here are some suggested improvements:
- <img src={photo} alt="login" className=" w-3/4 absolute" /> + <img src={photo} alt="Decorative image for login page" className="w-3/4 absolute" /> <form className="form z-10 p-16 bg-lightblue flex flex-col items-start justify-center gap-5 rounded-lg border-2 border-black shadow-[4px_4px_0px_0px_black] bg-[#f1e9dc]"> <div className="title text-[#323232] font-black text-7xl mb-6"> Welcome, <br /> <span className="block text-[#666] font-semibold text-2xl "> Log in to continue </span> </div> + <label htmlFor="email" className="sr-only">Email</label> <input className="input w-full h-10 rounded-md border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[15px] font-semibold text-[#323232] p-2.5 focus:outline-none focus:border-[#2d8cf0] placeholder-[#666] placeholder-opacity-80" name="email" + id="email" placeholder="Email" type="email" - onChange={(e) => handleChange(e)} + onChange={handleChange} + aria-label="Enter your email" /> + <label htmlFor="password" className="sr-only">Password</label> <input className="input w-full h-10 rounded-md border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[15px] font-semibold text-[#323232] p-2.5 focus:outline-none focus:border-[#2d8cf0] placeholder-[#666] placeholder-opacity-80" name="password" + id="password" placeholder="Password" type="password" - onChange={(e) => handleChange(e)} + onChange={handleChange} + aria-label="Enter your password" /> <button className="button-confirm mx-auto mt-12 px-4 w-30 h-10 rounded-md border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[17px] font-semibold text-[#323232] cursor-pointer active:shadow-none active:translate-x-[3px] active:translate-y-[3px]" - onClick={(e) => handleSubmit(e)} + onClick={handleSubmit} + aria-label="Submit login form" > Let's go → </button>Also, please review the color contrast of your text and background colors to ensure they meet WCAG 2.1 Level AA standards. You can use online tools like WebAIM's Contrast Checker to verify this.
To check for potential accessibility issues, you can run:
npx eslint --ext .jsx,.js src/ --rule 'jsx-a11y/alt-text: error'This will help identify any missing alt text for images in your React components.
15-27:⚠️ Potential issueImprove handleSubmit function for security and robustness.
The handleSubmit function has several areas for improvement:
- Remove console logging of sensitive data (password).
- Use environment variables for the API endpoint.
- Add error handling for the API call.
- Implement proper handling of the API response (e.g., setting auth token, redirecting user).
Here's a suggested improvement:
const handleSubmit = async (e) => { e.preventDefault(); try { const response = await fetch(process.env.REACT_APP_LOGIN_API_URL, { method: "POST", headers: { "Content-Type": "application/json", }, body: JSON.stringify(data), }); if (!response.ok) { throw new Error('Login failed'); } const result = await response.json(); // Handle successful login (e.g., set auth token, redirect) console.log("Login successful"); } catch (error) { console.error("Login error:", error); // Handle login error (e.g., show error message to user) } };Remember to set the
REACT_APP_LOGIN_API_URLenvironment variable in your project configuration.To ensure the API endpoint is correctly set, you can run:
This will help verify if the environment variable is properly defined in your project files.
frontend/src/components/Pages/Signup.jsx (2)
1-4: LGTM: Imports and component declaration are appropriate.The imports and component declaration follow React best practices. The useState hook is correctly imported, and the photo asset is imported for use in the component.
75-75: LGTM: Component export is correct.The Signup component is correctly exported as the default export, following React conventions.
frontend/src/components/Shared/Navbar.jsx (4)
3-3: LGTM: Proper implementation of useNavigate hookThe addition of
useNavigateto the imports and its initialization are correct. This change aligns with the PR objectives of implementing navigation for login and signup pages.Also applies to: 10-10
59-61: LGTM: Improved readability of nav element's classNameThe refactoring of the nav element's className assignment enhances code readability by breaking down the long line into multiple lines. The logic for applying classes based on the
isScrolledstate remains intact.
67-71: LGTM: Improved readability of logo image elementThe refactoring of the logo image element enhances code readability by breaking it down into multiple lines. The attributes and styling of the logo image remain unchanged, maintaining the original design.
120-123: LGTM: Improved readability of mobile menu buttonThe refactoring of the mobile menu button and its SVG icons enhances code readability by breaking down the elements into multiple lines. The functionality and appearance of the mobile menu button remain unchanged, maintaining the original design and user experience.
Also applies to: 132-137, 147-152
README.md (1)
Line range hint
1-224: Overall, the changes look good with minor suggestions for improvement.The update to the contributors section is well-implemented, maintaining the structure and consistency of the README. The addition of Jay Shah and the removal of Bashua Mutiat have been handled correctly.
I've suggested a few minor improvements:
- Adjusting the indentation of the new contributor entry for better consistency.
- Replacing hard tabs with spaces on lines 223-224 to ensure better compatibility across different platforms and editors.
These small changes will further enhance the overall quality and consistency of the README file.
🧰 Tools
🪛 Markdownlint
223-223: Column: 1
Hard tabs(MD010, no-hard-tabs)
224-224: Column: 1
Hard tabs(MD010, no-hard-tabs)
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@samar12-rad add only login button there |
|
Cool |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- frontend/src/components/Pages/Login.jsx (1 hunks)
- frontend/src/components/Shared/Navbar.jsx (8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/Shared/Navbar.jsx
🧰 Additional context used
🔇 Additional comments (2)
frontend/src/components/Pages/Login.jsx (2)
1-5: LGTM: Imports and component declaration are appropriate.The imports cover all necessary dependencies for the component's functionality, including React hooks and routing. The Login component is correctly declared as a functional component using an arrow function.
74-74: LGTM: Component export is correct.The Login component is properly exported as the default export, which is a common practice in React and allows for easy importing in other parts of the application.
RamakrushnaBiswal
added
enhancement
Created frontend pages Login And Signup
Created navbar buttons
Created api calls for integration
Tested api calls
For issue #171
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style