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

Global messages implemented #81

Closed
wants to merge 40 commits into from
Closed

Global messages implemented #81

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
6494bc9
Created Feedback Submission System
samar12-rad Oct 4, 2024
05a8e22
Code rabbit changes
samar12-rad Oct 4, 2024
ed67389
removed env from backend
samar12-rad Oct 4, 2024
38b7c35
code rabbit changes 2.0
samar12-rad Oct 4, 2024
6a7ef29
Fix the all contributors bot #58
Sapna127 Oct 4, 2024
9c6f337
docs(contributor): contrib-readme-action has updated readme
github-actions[bot] Oct 4, 2024
b42bd8d
code rabbit resolves
samar12-rad Oct 4, 2024
d5be656
Merge branch 'main' into contributors
RamakrushnaBiswal Oct 4, 2024
c2ec8e2
added validator
samar12-rad Oct 4, 2024
d2a7020
Merge branch 'main' of https://github.com/samar12-rad/PlayCafe into f…
samar12-rad Oct 4, 2024
2416f88
updated
17arindam Oct 4, 2024
bfb942b
Display global messages implemented
17arindam Oct 4, 2024
3ce1cfd
replaced new logo with the old logo
Navneetdadhich Oct 4, 2024
c43d8a9
code rabbit suggestion 1
17arindam Oct 4, 2024
77174be
code rabbit suggestion 2
17arindam Oct 4, 2024
c7d058b
enhancement:hamburger menu to toggle to 'X' on mobile
MutiatBash Oct 4, 2024
e5492ae
Merge pull request #65 from samar12-rad/feedback-form
RamakrushnaBiswal Oct 5, 2024
525bdfe
Merge branch 'main' into contributors
RamakrushnaBiswal Oct 5, 2024
cab8921
Merge pull request #77 from Sapna127/contributors
RamakrushnaBiswal Oct 5, 2024
285bebd
docs(contributor): contrib-readme-action has updated readme
github-actions[bot] Oct 5, 2024
f182713
new playcafe logo
Navneetdadhich Oct 5, 2024
fcf44df
reduced the size
Navneetdadhich Oct 5, 2024
0879bed
Merge pull request #85 from Nvntdad/main
RamakrushnaBiswal Oct 5, 2024
17d3029
docs(contributor): contrib-readme-action has updated readme
github-actions[bot] Oct 5, 2024
27b7a3e
Fix: Added Lazy load to few images
tejasbenibagde Oct 5, 2024
6541f37
Error system created with Winston Logger library
samar12-rad Oct 5, 2024
f9fe8bd
Merge branch 'main' of https://github.com/samar12-rad/PlayCafe into e…
samar12-rad Oct 5, 2024
206280c
Complete Backend Error system created with Winston Logger library
samar12-rad Oct 5, 2024
9ca75c6
performed 2/2 code rabbit changes
samar12-rad Oct 5, 2024
ab717ea
Merge pull request #83 from MutiatBash/hamburger-menu-toggle
RamakrushnaBiswal Oct 5, 2024
e58c8c0
docs(contributor): contrib-readme-action has updated readme
github-actions[bot] Oct 5, 2024
bd44e24
Merge pull request #89 from samar12-rad/errorHandler/backend
RamakrushnaBiswal Oct 5, 2024
f6703a7
docs(contributor): contrib-readme-action has updated readme
github-actions[bot] Oct 5, 2024
e41beaf
Merge pull request #88 from tejasbenibagde/main
RamakrushnaBiswal Oct 5, 2024
a54dbe4
docs(contributor): contrib-readme-action has updated readme
github-actions[bot] Oct 5, 2024
cd66b3a
updated
17arindam Oct 4, 2024
01dd8f6
Display global messages implemented
17arindam Oct 4, 2024
1f2e6e0
code rabbit suggestion 1
17arindam Oct 4, 2024
7812c74
code rabbit suggestion 2
17arindam Oct 4, 2024
e761101
Merge branch 'kindeauth' of https://github.com/17arindam/PlayCafe int…
17arindam Oct 5, 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
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions frontend/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"@kinde-oss/kinde-auth-react": "^4.0.4",
"@splidejs/splide": "^4.1.4",
"@splidejs/splide-extension-auto-scroll": "^0.5.3",
"antd": "^5.21.2",
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

Add Build Stats Generation and Bundle Analysis Tools

To effectively monitor and manage the bundle size impact of new dependencies like "antd", please implement the following:

  • Add a build script that generates stats:

    "build:stats": "vite build --mode production --outDir dist-stats"

  • Add a bundle analysis tool:

    "analyze": "vite-bundle-analyzer"

These additions will help in tracking and optimizing the bundle size, ensuring that new dependencies do not negatively affect the application's load times.

🔗 Analysis chain

Verify impact on bundle size

The addition of new dependencies may impact the overall bundle size of the application. It's important to ensure that these additions don't significantly increase the bundle size, which could affect load times.

To assess the impact on bundle size, please run the following script:

This script checks for existing build and analysis scripts, and provides suggestions if they're not present. Implementing these suggestions will help in monitoring and managing the bundle size as new dependencies are added.

Also applies to: 27-27

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Analyze the impact of new dependencies on bundle size

# Test: Check if there's a build script that generates stats
echo "Checking for a build script that generates stats:"
grep '"build"' package.json | grep 'stats'

# Test: If no stats script exists, suggest adding one
if [ $? -ne 0 ]; then
  echo "Consider adding a build script that generates stats. For example:"
  echo '"build:stats": "vite build --mode production --outDir dist-stats"'
fi

# Test: Check if there's an existing bundle analysis tool
echo "Checking for existing bundle analysis tools:"
grep -E '"analyze"|"bundle-analyzer"' package.json

# Test: If no analysis tool exists, suggest adding one
if [ $? -ne 0 ]; then
  echo "Consider adding a bundle analysis tool. For example:"
  echo '"analyze": "vite-bundle-analyzer"'
fi

# Note: Actual bundle size analysis would require building the project,
# which is beyond the scope of this script.

Length of output: 905

Script:

#!/bin/bash
# Description: Analyze the impact of new dependencies on bundle size

# Test: Check if there's a build script that generates stats in frontend/package.json
echo "Checking for a build script that generates stats in frontend/package.json:"
grep '"build"' frontend/package.json | grep 'stats'

# Test: If no stats script exists, suggest adding one
if [ $? -ne 0 ]; then
  echo "Consider adding a build script that generates stats. For example:"
  echo '"build:stats": "vite build --mode production --outDir dist-stats"'
fi

# Test: Check if there's an existing bundle analysis tool in frontend/package.json
echo "Checking for existing bundle analysis tools in frontend/package.json:"
grep -E '"analyze"|"bundle-analyzer"' frontend/package.json

# Test: If no analysis tool exists, suggest adding one
if [ $? -ne 0 ]; then
  echo "Consider adding a bundle analysis tool. For example:"
  echo '"analyze": "vite-bundle-analyzer"'
fi

# Note: Actual bundle size analysis would require building the project,
# which is beyond the scope of this script.

Length of output: 931

"autoprefixer": "^10.4.19",
"axios": "^1.7.7",
"clsx": "^2.1.1",
Expand Down
40 changes: 35 additions & 5 deletions frontend/src/components/Shared/Navbar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import { useState, useEffect } from "react";
import Logo from "../../assets/Logo/logo.png";
import { Link, useLocation } from "react-router-dom";
import { useKindeAuth } from "@kinde-oss/kinde-auth-react";
import { message } from "antd";

const Navbar = () => {
const { login, logout, isAuthenticated } = useKindeAuth();
const [isScrolled, setIsScrolled] = useState(false);
const [isMenuOpen, setIsMenuOpen] = useState(false);
const location = useLocation();

const [wasAuthenticated, setWasAuthenticated] = useState(false);
const menuItems = [
{ name: "Home", path: "/" },
{ name: "Events", path: "/events" },
Expand All @@ -30,6 +31,14 @@ const Navbar = () => {
};
}, []);

useEffect(() => {
// Check when user goes from not authenticated to authenticated
if (!wasAuthenticated && isAuthenticated) {
message.success("Login successful!");
}
setWasAuthenticated(isAuthenticated);
}, [isAuthenticated, wasAuthenticated]);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid unnecessary re-renders in useEffect due to dependency on 'wasAuthenticated'

Including wasAuthenticated in the dependency array of the useEffect hook may cause unnecessary re-renders because wasAuthenticated is updated within the effect. This can lead to the effect running more times than necessary. To prevent this, consider using a useRef to keep track of the previous authentication state without causing re-renders.

Here's a suggested fix using useRef:

+import { useRef } from "react";

...

- const [wasAuthenticated, setWasAuthenticated] = useState(false);
+ const wasAuthenticated = useRef(false);

useEffect(() => {
  // Check when user goes from not authenticated to authenticated
- if (!wasAuthenticated && isAuthenticated) {
+ if (!wasAuthenticated.current && isAuthenticated) {
    message.success("Login successful!");
  }
- setWasAuthenticated(isAuthenticated);
+ wasAuthenticated.current = isAuthenticated;
- }, [isAuthenticated, wasAuthenticated]);
+ }, [isAuthenticated]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
// Check when user goes from not authenticated to authenticated
if (!wasAuthenticated && isAuthenticated) {
message.success("Login successful!");
}
setWasAuthenticated(isAuthenticated);
}, [isAuthenticated, wasAuthenticated]);
import { useRef } from "react";
const wasAuthenticated = useRef(false);
useEffect(() => {
// Check when user goes from not authenticated to authenticated
if (!wasAuthenticated.current && isAuthenticated) {
message.success("Login successful!");
}
wasAuthenticated.current = isAuthenticated;
}, [isAuthenticated]);


const toggleMenu = () => {
setIsMenuOpen(!isMenuOpen);
};
Expand All @@ -47,6 +56,27 @@ const Navbar = () => {
const hoverTextColorClass = isScrolled ? "hover:text-gray-900" : "hover:text-gray-800";
const baseTextColorClass = isScrolled ? "text-gray-800" : "text-gray-900";
const mobileMenuBaseTextColorClass = isScrolled ? "text-gray-900" : "text-gray-800";

// Handle login
const handleLogin = async () => {
try {
await login();

} catch (error) {
message.error("Login failed. Please try again.");
}
};


const handleLogout = async () => {
try {
await logout();
message.success("Logout successful!");
} catch (error) {
message.error("Logout failed. Please try again.");
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure consistent handling of success messages for login and logout

In handleLogout, you display a success message immediately after await logout();, but for the login action, you rely on the useEffect to display the success message. For consistency and to ensure messages are displayed correctly, consider handling both success messages using the same approach.

Option 1: Move the logout success message to the useEffect

useEffect(() => {
  // Check when user goes from authenticated to not authenticated
+ if (wasAuthenticated.current && !isAuthenticated) {
+   message.success("Logout successful!");
+ }
  if (!wasAuthenticated.current && isAuthenticated) {
    message.success("Login successful!");
  }
  wasAuthenticated.current = isAuthenticated;
}, [isAuthenticated]);

Option 2: Display the login success message directly in handleLogin

const handleLogin = async () => {
  try {
    await login();
+   message.success("Login successful!");
  } catch (error) {
    message.error("Login failed. Please try again.");
  }
};

Consider which approach aligns better with your authentication flow and whether login() or logout() cause page redirects that may prevent the success messages from displaying.

Committable suggestion was skipped due to low confidence.


return (
<nav
className={`w-full fixed top-0 z-50 transition duration-300 ${isScrolled ? "bg-[#E0F0B1]" : "bg-transparent"}
Expand Down Expand Up @@ -75,15 +105,15 @@ const Navbar = () => {
))}
{isAuthenticated ? (
<button
onClick={logout}
onClick={handleLogout}
className={`${baseTextColorClass} ${hoverTextColorClass}`}
type="button"
>
Log Out
</button>
) : (
<button
onClick={login}
onClick={handleLogin}
className={`${baseTextColorClass} ${hoverTextColorClass}`}
type="button"
>
Expand Down Expand Up @@ -120,15 +150,15 @@ const Navbar = () => {
))}
{isAuthenticated ? (
<button
onClick={logout}
onClick={handleLogout}
className={`block w-full text-left px-4 py-3 rounded-md text-base font-semibold transition duration-300
${mobileMenuBaseTextColorClass} hover:bg-amber-300 hover:text-black`}
>
Log Out
</button>
) : (
<button
onClick={login}
onClick={handleLogin}
className={`block w-full text-left px-4 py-3 rounded-md text-base font-semibold transition duration-300
${mobileMenuBaseTextColorClass} hover:bg-amber-300 hover:text-black`}
>
Expand Down
Loading