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

requried changes done #357 #423

Closed
wants to merge 1 commit into from
Closed

requried changes done #357 #423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
52 changes: 44 additions & 8 deletions frontend/src/App.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,52 @@ input[type='radio'] {
}

.star {
cursor: pointer;
font-size: 2rem;
margin-bottom: 0;
cursor: pointer;
font-size: 2rem;
margin-bottom: 0;
}

.card {
transition: all 0.3s ease; /* Ensure smooth transitions */
@apply transition-all duration-300 ease-in-out; /* Smooth transition */
}

.card:hover {
@apply shadow-lg; /* Adds a shadow on hover */
}

/* Responsive adjustments for different screen sizes */
@media (min-width: 640px) {
/* Small devices (sm) */
.star {
font-size: 1.5rem; /* Smaller star size for narrow screens */
}
}

@media (min-width: 768px) {
/* Medium devices (md) */
.star {
font-size: 1.75rem; /* Slightly larger star size */
}

.rating-button {
@apply p-3; /* More padding for buttons on medium screens */
}
}

@media (min-width: 1024px) {
/* Large devices (lg) */
.card {
@apply rounded-lg p-4; /* Larger padding and rounded corners */
}

/* Shadow effect on hover */
.card:hover {
box-shadow: 0 8px 16px rgba(0, 0, 0, 0.3); /* Add a shadow on hover */
}

@media (min-width: 1280px) {
/* Extra large devices (xl) */
.star {
font-size: 2rem; /* Default large star size */
}

.rating-button {
@apply p-4; /* Extra padding for larger screens */
}
}
Comment on lines +42 to +77
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

Consider using Tailwind's responsive modifiers.

The media queries could be simplified using Tailwind's built-in responsive modifiers. This would reduce code duplication and improve maintainability.

Example refactor for the star and rating-button classes:

.star {
-  cursor: pointer;
-  font-size: 2rem;
-  @apply mb-0;
+  @apply cursor-pointer text-2xl sm:text-xl md:text-[1.75rem] xl:text-2xl mb-0;
}

.rating-button {
-  @apply flex-grow bg-neutral-50 text-center p-2 rounded-md cursor-pointer select-none;
+  @apply flex-grow bg-neutral-50 text-center p-2 md:p-3 xl:p-4 rounded-md cursor-pointer select-none;
}

Committable suggestion was skipped due to low confidence.

190 changes: 55 additions & 135 deletions frontend/src/components/Shared/footer/Content.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { useEffect, useState } from 'react';
import Logo from '../../../assets/Logo/playcafe.png';
import googleImage from '../../../assets/img/google.png';
import { FaFacebook, FaInstagram, FaTiktok, FaGithub } from 'react-icons/fa';
import Google from './Google';

export default function Content() {
return (
<div className="bg-amber-100 dark:bg-black pt-16 py-8 px-12 h-full w-full flex flex-col justify-between md:pt-24`">
<div className="bg-amber-100 dark:bg-gray-900 pt-16 py-8 px-4 sm:px-6 lg:px-12 h-full w-full flex flex-col justify-between md:pt-24">
<Nav />
<Section2 />
<NewsletterForm />
Expand Down Expand Up @@ -41,179 +40,100 @@ const NewsletterForm = () => {
};

return (
<div className="absolute top-12 right-60 p-2 rounded-md shadow-lg ">
<h3 className="text-white text-lg mb-2">Subscribe to our Newsletter</h3>
<form onSubmit={handleSubmit} className="flex flex-col md:flex-row gap-2 md:gap-4">
<div className="flex items-center border rounded-md bg-white">
<span className="p-2 text-gray-600">
📧
</span>
<input
type="email"
className="p-2 rounded-r-none border-l border-gray-300 focus:outline-none"
placeholder="Enter your email"
value={email}
onChange={(e) => setEmail(e.target.value)}
required
/>
<div className="fixed bottom-4 right-4 bg-white dark:bg-gray-800 p-4 rounded-lg shadow-lg max-w-xs w-full text-center z-50">
<h3 className="text-lg font-semibold mb-3 text-black dark:text-white">Subscribe to our Newsletter</h3>
<form onSubmit={handleSubmit} className="flex flex-col gap-2 items-center">
<div className="flex items-center border rounded-md bg-gray-100 dark:bg-gray-700 w-full">
<span className="p-2 text-gray-600 dark:text-gray-300">📧</span>
<input
type="email"
className="p-2 rounded-r-none w-full border-l border-gray-300 dark:border-gray-600 focus:outline-none bg-transparent text-black dark:text-white"
placeholder="Enter your email"
value={email}
onChange={(e) => setEmail(e.target.value)}
required
Comment on lines +43 to +54
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

Consider improving the newsletter form's mobile responsiveness

The fixed positioning at bottom-right with no dismiss option could cause usability issues:

  1. May block important content on mobile devices
  2. Users cannot dismiss the form if they don't want to subscribe
  3. No specific mobile adaptations for smaller screens

Consider these improvements:

-    <div className="fixed bottom-4 right-4 bg-white dark:bg-gray-800 p-4 rounded-lg shadow-lg max-w-xs w-full text-center z-50">
+    <div className="fixed bottom-4 right-4 bg-white dark:bg-gray-800 p-4 rounded-lg shadow-lg max-w-xs w-full text-center z-50 sm:w-auto">
+      <button 
+        onClick={() => setIsVisible(false)}
+        className="absolute top-2 right-2 text-gray-500 hover:text-gray-700"
+        aria-label="Close newsletter form"
+      >
+        ×
+      </button>

Also consider adding a state to control visibility:

const [isVisible, setIsVisible] = useState(true);
// Wrap the entire component JSX with:
isVisible && (
  // ... existing JSX
)

/>
</div>
<button type="submit" className="p-2 bg-green-500 text-white rounded-md ml-2">
<button type="submit" className="p-2 bg-green-500 text-white rounded-md w-full mt-2">
Subscribe
</button>
</form>
{message && <p className="mt-2 text-sm text-green-500">{message}</p>}
</div>
);
};

const Section2 = () => {
const [isWide, setIsWide] = useState(null);

useEffect(() => {
const handleResize = () => setIsWide(window.innerWidth > 640);
handleResize(); // Set initial value
handleResize();
window.addEventListener('resize', handleResize);
return () => window.removeEventListener('resize', handleResize);
}, []);

if (isWide === null) return null;

return (
<>
{!isWide && (
<div className="flex justify-center">
<img
className="w-24 bg-transparent p-0 rounded-3xl h-24"
alt="logo"
src={Logo}
loading="lazy"
/>
</div>
)}
<div

className={`flex ${isWide ? 'justify-between items-end' : 'flex-col items-center'
} text-white`}

>
<div>
<h1
className={`${isWide ? 'text-[7.5vw]' : 'text-[12vw]'} leading-[0.8]`}
>
BoardGame Cafe{!isWide && <br />}
</h1>
<p className={`flex ${isWide ? `text-[center] mt-3 ml-[620px]` : `text-base mt-1`}`}>
©2024 by Sip & Play
</p>
</div>
</div>
</>
<div className="flex flex-col items-center text-center my-6">
<img src={Logo} alt="Logo" className="w-20 h-20 sm:w-24 sm:h-24 bg-transparent p-0 rounded-3xl" loading="lazy" />
<h1 className={`${isWide ? 'text-5xl' : 'text-3xl'} font-bold leading-tight text-black dark:text-white mt-4`}>
BoardGame Cafe
</h1>
<p className="text-gray-600 dark:text-gray-400 mt-2">©2024 by Sip & Play</p>
</div>
Comment on lines +79 to +85
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

Simplify responsive design using Tailwind's built-in breakpoints

The current implementation using window.innerWidth can cause layout shifts and unnecessary re-renders. Consider using Tailwind's responsive classes instead.

-      <h1 className={`${isWide ? 'text-5xl' : 'text-3xl'} font-bold leading-tight text-black dark:text-white mt-4`}>
+      <h1 className="text-3xl sm:text-5xl font-bold leading-tight text-black dark:text-white mt-4">

You can also remove the entire useEffect and isWide state as they would no longer be needed.

📝 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
<div className="flex flex-col items-center text-center my-6">
<img src={Logo} alt="Logo" className="w-20 h-20 sm:w-24 sm:h-24 bg-transparent p-0 rounded-3xl" loading="lazy" />
<h1 className={`${isWide ? 'text-5xl' : 'text-3xl'} font-bold leading-tight text-black dark:text-white mt-4`}>
BoardGame Cafe
</h1>
<p className="text-gray-600 dark:text-gray-400 mt-2">©2024 by Sip & Play</p>
</div>
<div className="flex flex-col items-center text-center my-6">
<img src={Logo} alt="Logo" className="w-20 h-20 sm:w-24 sm:h-24 bg-transparent p-0 rounded-3xl" loading="lazy" />
<h1 className="text-3xl sm:text-5xl font-bold leading-tight text-black dark:text-white mt-4">
BoardGame Cafe
</h1>
<p className="text-gray-600 dark:text-gray-400 mt-2">©2024 by Sip & Play</p>
</div>

);
};

const Nav = () => {
const navLinks = [
{
name: 'Home',
link: '/',
},
{
name: 'Events',
link: '/events',
},
{
name: 'Reservation',
link: '/reservation',
},
{
name: 'Boardgame',
link: '/boardgame',
},
{
name: 'About',
link: '/about',
},
{ name: 'Home', link: '/' },
{ name: 'Events', link: '/events' },
{ name: 'Reservation', link: '/reservation' },
{ name: 'Boardgame', link: '/boardgame' },
{ name: 'About', link: '/about' },
];

const socialLink = [
{
name: 'Facebook',
link: 'https://www.facebook.com/sipnplaynyc/',
icon: <FaFacebook />,
},
{
name: 'Instagram',
link: 'https://www.instagram.com/sipnplaynyc/?hl=en',
icon: <FaInstagram />,
},
{
name: 'Tiktok',
link: 'https://www.tiktok.com/@sipnplaynycofficial?lang=en',
icon: <FaTiktok />,
},
{
name: 'GitHub',
link: 'https://github.com/RamakrushnaBiswal/PlayCafe',
icon: <FaGithub />,
},
{ name: 'Facebook', link: 'https://www.facebook.com/sipnplaynyc/', icon: <FaFacebook /> },
{ name: 'Instagram', link: 'https://www.instagram.com/sipnplaynyc/?hl=en', icon: <FaInstagram /> },
{ name: 'Tiktok', link: 'https://www.tiktok.com/@sipnplaynycofficial?lang=en', icon: <FaTiktok /> },
{ name: 'GitHub', link: 'https://github.com/RamakrushnaBiswal/PlayCafe', icon: <FaGithub /> },
];

const emailAddress = '[email protected]';

return (
<div className="flex md:flex-row flex-col shrink-0 gap-4 sm:gap-20 ml-[30px]">
<div className="flex justify-between md:gap-20">
<div className="flex flex-col gap-2 text-black dark:text-white">
<h3 className="mb-2 uppercase text-black dark:text-white cursor-pointer">About</h3>
{navLinks.map((item, index) => (
<a
className="hover:text-white duration-300"
key={index}
href={item.link}
>
{item.name}
</a>
))}
</div>
<div className="flex flex-col md:flex-row justify-between items-start md:items-center gap-4 sm:gap-8 md:gap-12 p-4 sm:p-6">
<div className="flex flex-col gap-3">
<h3 className="uppercase font-semibold text-black dark:text-white">About</h3>
{navLinks.map((item, index) => (
<a key={index} href={item.link} className="hover:text-gray-700 dark:hover:text-gray-300 transition-colors text-sm sm:text-base">
{item.name}
</a>
))}
</div>

<div className="flex flex-col gap-2 text-black dark:text-white">
<h3 className="mb-2 uppercase text-black dark:text-white ml-[30px] cursor-pointer">Socials</h3>
{socialLink.map((item, index) => (
<a
target="_blank"
className="hover:text-white duration-300 flex items-center gap-2"
key={index}
href={item.link}
aria-label={`${item.name} - opens in a new tab`}
>
{item.icon}
{item.name}
</a>
))}
</div>
<div className="flex flex-col gap-3">
<h3 className="uppercase font-semibold text-black dark:text-white">Socials</h3>
{socialLink.map((item, index) => (
<a key={index} href={item.link} target="_blank" rel="noopener noreferrer" className="flex items-center gap-2 hover:text-gray-700 dark:hover:text-gray-300 transition-colors text-sm sm:text-base">
{item.icon} {item.name}
</a>
))}
</div>

<div className="flex flex-col text-black dark:text-white">
<h3 className="mb-2 uppercase text-black dark:text-white ml-[30px] cursor-pointer">Contact Us</h3>
<a
href={`mailto:${emailAddress}`}
className="block mb-2 hover:underline"
>
<div className="flex flex-col gap-3">
<h3 className="uppercase font-semibold text-black dark:text-white">Contact Us</h3>
<a href={`mailto:${emailAddress}`} className="hover:underline text-gray-800 dark:text-gray-300 text-sm sm:text-base">
{emailAddress}
</a>
<a href="tel:+17189711684" className="mb-2 hover:underline">
<a href="tel:+17189711684" className="hover:underline text-gray-800 dark:text-gray-300 text-sm sm:text-base">
718-971-1684
</a>

{/* <div className="flex items-center justify-center mt-4">
<img
src={googleImage}
alt="Google Translate"
className="w-[2rem] h-[2rem] mr-[65px]"
/>
</div> */}
<Google />
</div>
<div className="flex flex-col md:ml-4 mt-4 md:mt-0"> {/* NewsletterForm flex container */}
<NewsletterForm />
</div>
</div>
);
};
Loading