Skip to content
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.

Sign up for GitHub

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

Alertmessage : Alert pop up message from antd added #360 done #376

Merged
merged 5 commits into from
Oct 22, 2024
Merged

Alertmessage : Alert pop up message from antd added #360 done #376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5052f84
docs(contributor): contrib-readme-action has updated readme
github-actions[bot] Oct 21, 2024
0c7608a
antd alert message added
VinayLodhi1712 Oct 21, 2024
fbcdbb5
Merge branch 'main' of https://github.com/VinayLodhi1712/PlayCafe int…
VinayLodhi1712 Oct 22, 2024
60fc4c6
remove css file
VinayLodhi1712 Oct 22, 2024
054b7ce
docs(contributor): contrib-readme-action has updated readme
github-actions[bot] Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ require("dotenv").config();
const cors = require("cors");
const mongoose = require("mongoose");
const logger = require("./config/logger");
const errorMiddleware = require("./middlewares/errorMiddleware"); // Corrected typo
const errorMiddleware = require("./middlewares/errrorMiddleware"); // Corrected typo
RamakrushnaBiswal marked this conversation as resolved.
Show resolved Hide resolved
const passport = require("passport");
const { handleGoogleOAuth } = require("./controller/googleOAuth.controller");
const app = express();
Expand Down
2 changes: 1 addition & 1 deletion frontend/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"@splidejs/react-splide": "^0.7.12",
"@splidejs/splide": "^4.1.4",
"@splidejs/splide-extension-auto-scroll": "^0.5.3",
"antd": "^5.21.2",
"antd": "^5.21.5",
"autoprefixer": "^10.4.19",
"axios": "^1.7.7",
"clsx": "^2.1.1",
Expand Down
44 changes: 39 additions & 5 deletions frontend/src/components/Pages/Login.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import React, { useState, useEffect } from 'react';
import photo from '../../assets/login.png';
import { Link, useNavigate } from 'react-router-dom';
import { message } from 'antd';
import { message } from 'antd'; // Ant Design message component
import Cookies from 'js-cookie';
import { FaEye } from 'react-icons/fa';
import { FaEyeSlash } from 'react-icons/fa6';

const Login = () => {
Copy link
Owner

@RamakrushnaBiswal RamakrushnaBiswal Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@VinayLodhi1712 here also

const API_URL = import.meta.env.VITE_BACKEND_URL || 'http://localhost:3000';
const [data, setData] = useState({
Expand All @@ -18,10 +17,19 @@ const Login = () => {

const navigate = useNavigate();

// Configuring the message to show at a certain top distance
message.config({
top: 80, // Distance from the top of the viewport
duration: 2, // Auto close time in seconds
maxCount: 3, // Max number of messages
});

// Function to handle changes in the form inputs
const handleChange = (e) => {
setData({ ...data, [e.target.name]: e.target.value });
};

// Function to handle form submission (login)
const handleSubmit = async (e) => {
e.preventDefault();
setIsLoading(true);
Expand All @@ -35,14 +43,40 @@ const Login = () => {
body: JSON.stringify(data),
});
const result = await response.json();

if (!response.ok) {
throw new Error(result.message || 'Login failed');
}

// Successful login message with a light green line below
message.success({
content: 'Login successful',
className: 'success-message', // Add custom class for styling
style: {
fontSize: '22px',
right: '50px', // Position it on the right side
position: 'fixed', // Fix it to the viewport
paddingTop: '10px', // Add padding to move the text down
paddingBottom: '10px', // Padding for balance
},
});
// Set token in cookies
Cookies.set('authToken', result.token, { expire: '1h', secure: true });
message.success('Login successful');
navigate('/');
navigate('/'); // Navigate to homepage after successful login
} catch (err) {
setError(err.message || 'An error occurred. Please try again.');
// Show error message if login fails with a red line below
message.error({
content: 'Something went wrong while logging in.',
className: 'error-message', // Add custom class for styling
style: {
fontSize: '18px',
right: '50px', // Position it on the right side
position: 'fixed', // Fix it to the viewport
paddingTop: '10px', // Add padding to move the text down
paddingBottom: '10px', // Padding for balance
},
});
setError('An error occurred. Please try again.');
Comment on lines +67 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent Error Handling Across Components

Error handling implementations using message.error vary between components:

  • Signup.jsx and Login.jsx apply custom styles for enhanced user feedback.
  • Admin.jsx uses default styling without customizations.

Consider standardizing the error handling approach and extracting common styles into a shared constant to ensure consistency and maintainability across the codebase.

🔗 Analysis chain

LGTM with suggestions: Error message implemented correctly.

The error message implementation using Ant Design's message.error is well done. The custom styling enhances the visual feedback, aligning with the PR objectives.

  1. Consider extracting the message style into a separate constant for reusability, similar to the success message suggestion.

  2. The error state is still being set alongside the new message implementation. Consider if this dual approach is necessary:

setError('An error occurred. Please try again.');

If the Ant Design message is sufficient for user feedback, you might be able to remove the error state and its related UI elements for a more streamlined approach.

To ensure consistency across the application, let's check if other components are using a similar approach for error handling:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for similar error handling patterns in other components
rg -A 5 "message.error" src/components

Length of output: 93

Script:

#!/bin/bash
# Search for "message.error" patterns across the entire repository
rg "message.error" --context 5

Length of output: 6362

Script:

#!/bin/bash
# Search for "message.error" patterns within the frontend/src/components directory
rg "message.error" frontend/src/components --context 5

Length of output: 6386

} finally {
setIsLoading(false);
}
Expand Down
63 changes: 50 additions & 13 deletions frontend/src/components/Pages/Signup.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import { useState, useEffect } from 'react';
import React, { useState, useEffect } from 'react';
import { message } from 'antd'; // Import Ant Design message
import photo from '../../assets/login.png';
import { useNavigate, Link } from 'react-router-dom';
import { FaEye } from 'react-icons/fa';
import { FaEyeSlash } from 'react-icons/fa6';
import zxcvbn from 'zxcvbn'; // Password strength checker


// Configure message globally
message.config({
top: 80, // Align with the login page
duration: 2, // Duration of the message visibility
});

const Signup = () => {
const API_URL = import.meta.env.VITE_BACKEND_URL || 'http://localhost:3000';
const navigate = useNavigate();
Expand All @@ -25,43 +33,77 @@ const Signup = () => {
const handleSubmit = async (e) => {
e.preventDefault();
setIsLoading(true);
setError(null); // Reset error before submission

// Frontend validation
if (!data.email || !data.password || !data.name) {
setError('Please fill in all fields');
message.error({
content: 'Please fill in all fields.',
style: { fontSize: '18px' },
});
setIsLoading(false);
return;
}
if (data.password.length < 8) {
setError('Password must be at least 8 characters long');
message.error({
content: 'Password must be at least 8 characters long.',
style: { fontSize: '18px' },
});
setIsLoading(false);
return;
}
if (data.name.length < 3) {
setError('Name must be at least 3 characters long');
message.error({
content: 'Name must be at least 3 characters long.',
style: { fontSize: '18px' },
});
setIsLoading(false);
return;
}
if (!data.email.includes('@')) {
setError('Please enter a valid email address');
message.error({
content: 'Please enter a valid email address.',
style: { fontSize: '18px' },
});
setIsLoading(false);
return;
}

// Submit form data
try {
const response = await fetch(`${API_URL}/api/user/register`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(data),
});
const result = await response.json();

if (!response.ok) {
setIsLoading(false);
setError(result.error);
message.error({
content: result.error || 'An error occurred during registration.',
style: { fontSize: '18px' },
});
return;
}
alert('Registered successfully! Please log in.');
navigate('/');

// Successful registration message
message.success({
content: 'Registered successfully! Please log in.',
style: { fontSize: '18px' },
});

// Redirect to login page
navigate('/login');
} catch (error) {
setError(error.message);
console.error('Error:', error);
message.error({
content: 'An error occurred while submitting the form.',
style: { fontSize: '18px' },
});
} finally {
setIsLoading(false);
}
};

Expand Down Expand Up @@ -139,11 +181,6 @@ const Signup = () => {
Strength: {getPasswordStrengthText(passwordStrength)}
</p>
</div>
{error && (
<div className="w-full p-2 bg-red-100 text-red-700 border border-red-400 rounded-md">
{error}
</div>
)}
<h3 className="flex justify-between w-full">
Already have an account?
<Link
Expand Down
Loading