Added Perspective Hover Effect for Popular Board Games Card and Code cleanup : Issue no #106 #117
Conversation
|
@AbhijitMotekar99 is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request involve significant updates to the CSS and JavaScript components of the frontend application. New CSS classes are added to enhance styling and hover effects, while the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
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 (13)
frontend/src/App.css (3)
26-32: LGTM: Glow effect enhances UI and provides user feedback.The .glow-effect class and its hover state create an engaging visual effect that aligns well with the PR's objective of adding hover effects to game cards.
Consider using CSS custom properties (variables) for the shadow values to make future adjustments easier. Here's a suggested refactor:
+:root { + --glow-shadow: 0 0 10px rgba(63, 63, 63, 0.5), 0 0 20px rgba(72, 72, 72, 0.3); + --glow-shadow-hover: 0 0 20px rgba(55, 56, 26, 0.7), 0 0 30px rgba(255, 239, 69, 0.5); +} .glow-effect { - box-shadow: 0 0 10px rgba(63, 63, 63, 0.5), 0 0 20px rgba(72, 72, 72, 0.3); + box-shadow: var(--glow-shadow); } .glow-effect:hover { - box-shadow: 0 0 20px rgba(55, 56, 26, 0.7), 0 0 30px rgba(255, 239, 69, 0.5); + box-shadow: var(--glow-shadow-hover); }
38-40: LGTM: Group hover effect enhances UI.The .group:hover class adds a subtle border color change on hover, which provides additional visual feedback and aligns with the PR's objective of enhancing hover effects.
Consider using a CSS custom property for the border color to maintain consistency with other hover effects:
+:root { + --hover-border-color: rgba(143, 143, 143, 0.5); +} .group:hover { - border-color: rgba(143, 143, 143, 0.5); + border-color: var(--hover-border-color); }
Line range hint
1-41: Improve code formatting for consistency.The overall structure of the file is good, with new classes appropriately added at the end. However, there are some inconsistencies in indentation and spacing that could be improved for better readability.
Please consider the following formatting improvements:
- Ensure consistent indentation (2 or 4 spaces) throughout the file.
- Remove extra blank lines (e.g., lines 20-21).
- Ensure consistent spacing around brackets and braces.
You can use a CSS formatter like Prettier to automatically apply consistent formatting to your CSS files.
frontend/src/index.css (6)
Line range hint
9-12: Consider improving cross-browser compatibility and naming convention.The
.font-outline-2class uses the-webkit-text-strokeproperty, which is not supported in all browsers. Consider using a cross-browser solution or providing fallbacks.Additionally, consider adopting a more descriptive class name following BEM naming convention, such as
.text-outline--white-2px.Here's an improved version:
.text-outline--white-2px { -webkit-text-stroke: 2px white; text-stroke: 2px white; text-shadow: -2px -2px 0 #fff, 2px -2px 0 #fff, -2px 2px 0 #fff, 2px 2px 0 #fff; }
Line range hint
14-16: Improve class naming and Tailwind usage.The
.Poppinsclass doesn't follow conventional CSS naming (lowercase). Consider renaming it to.poppins-fontfor consistency. Also, consider using Tailwind's@applydirective and placing it within an appropriate@layerfor better organization.Here's an improved version:
@layer utilities { .poppins-font { @apply font-['Poppins',_sans-serif]; } }
Line range hint
20-26: Reconsider.primary-btnimplementation and.page-shadowplacement.The
.primary-btnclass only appliesblock, which seems incomplete for a primary button style. Consider expanding this class with more styles or removing it if unnecessary.The
.page-shadowclass applies a complex shadow effect. Consider moving this to the@layer utilitiessection as it's more of a utility class than a component.Here's a suggested improvement:
@layer components { .primary-btn { @apply block px-4 py-2 bg-blue-500 text-white rounded hover:bg-blue-600 transition-colors; } } @layer utilities { .page-shadow { @apply shadow-[inset_0_0_8px_rgba(0,0,0,0.6),12px_12px_14px_rgba(0,0,0,0.9)]; } }
Line range hint
31-44: Improve scrollbar styles for cross-browser compatibility and consistency.The current scrollbar styles are only applicable to webkit browsers. Consider using a cross-browser solution or providing fallbacks for better compatibility.
Also, the use of
beigecolor for the scrollbar might not be consistent with the overall color scheme of the application. Consider using a color variable or a Tailwind color class for better maintainability and consistency.Here's a suggested improvement using CSS variables for colors:
:root { --scrollbar-bg: theme('colors.gray.200'); --scrollbar-thumb: theme('colors.gray.400'); --scrollbar-thumb-hover: theme('colors.gray.600'); } * { scrollbar-width: thin; scrollbar-color: var(--scrollbar-thumb) var(--scrollbar-bg); } ::-webkit-scrollbar { width: 7px; height: 7px; } ::-webkit-scrollbar-track { background: var(--scrollbar-bg); } ::-webkit-scrollbar-thumb { background-color: var(--scrollbar-thumb); border-radius: 10px; } ::-webkit-scrollbar-thumb:hover { background-color: var(--scrollbar-thumb-hover); }
45-46: Remove extra empty line at the end of the file.It's good practice to end a file with a single newline character. Consider removing one of the two empty lines at the end of the file.
Line range hint
1-46: Overall code review summaryThe changes introduce new styles and components that enhance the UI. However, there are several areas for improvement:
- Cross-browser compatibility for text outline and scrollbar styles.
- Consistency in naming conventions (e.g.,
.Poppinsclass).- Better utilization of Tailwind's
@layerand@applydirectives.- Reconsideration of component vs utility classes.
- Color consistency and use of CSS variables or Tailwind color classes.
Addressing these points will improve the maintainability and consistency of the stylesheet.
frontend/src/components/Pages/Boardgame.jsx (4)
91-91: Add apostrophe in "team's" for possessive form.In the description for "Codenames" at line 91, "your teams secret code words" should use an apostrophe to indicate possession:
- Change "your teams secret code words" to "your team's secret code words".
135-137: Avoid usinghref="#"to prevent unintended page jumps.At lines 135-137, the anchor tag uses
href="#":<a className="absolute inset-0 z-10" href="#"> <span className="sr-only">View</span> </a>Using
href="#"can cause the page to scroll to the top when clicked. Consider the following alternatives:
- Use
href="/"to link to the homepage.- Use
href=""orhref="javascript:void(0);"to prevent default behavior.- Use a
buttonelement styled as a link if the anchor does not navigate.
150-151: Review inline styles for cross-browser compatibility and redundancy.At lines 150-151, the
imgtag includes inline styles:style={{ aspectRatio: "500 / 400", objectFit: "cover" }}
- The
aspect-ratioCSS property is not fully supported in all browsers. Consider using a wrapper with a fixed aspect ratio or using padding techniques for better compatibility.- The
objectFit: "cover"style is redundant since theobject-coverclass already appliesobject-fit: cover;.To improve compatibility and reduce redundancy, you might:
- Remove the inline
objectFitstyle.- Implement a fallback for the
aspect-ratioproperty.
130-130: Consider using a unique identifier for thekeyprop instead of the index.At line 130, the
keyprop is set to the index:key={index}Using the index as a key can lead to issues if the list items are reordered or data changes dynamically. If each board game has a unique identifier (e.g.,
id), consider using it:key={board.id}This enhances performance and ensures the correct handling of components in React's reconciliation process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- frontend/src/App.css (1 hunks)
- frontend/src/components/Pages/Boardgame.jsx (2 hunks)
- frontend/src/components/Pages/Register.jsx (1 hunks)
- frontend/src/index.css (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
frontend/src/App.css (1)
19-19: LGTM: Background color change enhances UI.The new background color (#F1EADC) provides a warm, neutral backdrop for the application, which aligns well with the PR's objective of enhancing the user interface.
frontend/src/components/Pages/Register.jsx (1)
145-172: Excellent refactoring of board game cards!The refactoring of the board game cards into a dynamic rendering using an array and map function is a great improvement. It enhances code maintainability, reduces redundancy, and makes it easier to update the game list in the future.
The addition of the hover effect with the glow is a nice touch that improves user interaction and visual appeal.
Great job on this refactoring! The code is now more maintainable and scalable.
frontend/src/App.css
Outdated
| perspective: 1000px; /* Adjust the value as needed */ | ||
| } |
There was a problem hiding this comment.
Remove duplicate .perspective class definition.
The .perspective class is a good addition for creating 3D-positioned elements, which aligns with the PR's goal of adding hover effects. However, the class is defined twice with the same value.
Please remove the duplicate definition. Apply this diff:
.perspective {
perspective: 1000px; /* Adjust the value as needed */
}
- .perspective {
- perspective: 1000px;
- }Also applies to: 34-36
|
@AbhijitMotekar99 1st video hover effect doesn't look good |
|
Hey @RamakrushnaBiswal |
|
Hey @RamakrushnaBiswal |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
frontend/src/components/Pages/Boardgame.jsx (1)
Line range hint
74-101: Enhance form handling and accessibility for newsletter subscription.While the newsletter subscription section is well-structured, consider the following improvements to enhance user experience and accessibility:
- Add form submission handling to prevent page reload and provide user feedback.
- Implement form validation for the email input.
- Add appropriate ARIA attributes and labels for better screen reader support.
- Consider adding a loading state to the submit button during form submission.
Here's a suggested implementation:
import React, { useState } from 'react'; // ... (rest of the imports) export default function Boardgame() { const [email, setEmail] = useState(''); const [isSubmitting, setIsSubmitting] = useState(false); const [submitMessage, setSubmitMessage] = useState(''); const handleSubmit = async (e) => { e.preventDefault(); setIsSubmitting(true); // Implement your form submission logic here // For example: // await submitNewsletter(email); setSubmitMessage('Thank you for subscribing!'); setIsSubmitting(false); setEmail(''); }; return ( // ... (rest of the component) <section className="w-full py-12 md:py-24 lg:py-32 bg-gray-100"> <div className="container mx-auto px-4 md:px-6 text-center"> <h2 className="text-3xl font-bold tracking-tight sm:text-4xl md:text-5xl lg:text-6xl mb-4"> Subscribe to our Newsletter </h2> <p className="mx-auto max-w-[700px] text-muted-foreground md:text-xl mb-6"> Stay updated with our latest boardgame collections, special offers, and events. Subscribe now and never miss out! </p> <form onSubmit={handleSubmit} className="flex flex-col items-center space-y-4 sm:space-y-0 sm:flex-row sm:space-x-4 justify-center"> <label htmlFor="email" className="sr-only">Email address</label> <input id="email" type="email" value={email} onChange={(e) => setEmail(e.target.value)} className="px-4 py-2 rounded-lg border border-gray-300 focus:outline-none focus:ring-2 focus:ring-blue-500" placeholder="Enter your email" required aria-required="true" /> <button type="submit" disabled={isSubmitting} className="px-6 py-2 text-white bg-blue-600 rounded-lg hover:bg-blue-700 focus:outline-none focus:ring-2 focus:ring-blue-500 disabled:opacity-50" aria-live="polite" > {isSubmitting ? 'Subscribing...' : 'Subscribe'} </button> </form> {submitMessage && ( <p className="mt-4 text-green-600" aria-live="polite">{submitMessage}</p> )} </div> </section> // ... (rest of the component) ); }This implementation adds form state management, submission handling, and improved accessibility features.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- frontend/src/App.css (1 hunks)
- frontend/src/components/Pages/Boardgame.jsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/App.css
🧰 Additional context used
🔇 Additional comments (2)
frontend/src/components/Pages/Boardgame.jsx (2)
Line range hint
1-101: Overall assessment: Improved code structure with room for enhancements.The changes made to the
Boardgamecomponent significantly improve code maintainability and scalability. The use of an array andmapfunction for rendering board game cards is a positive step. However, there are areas that require attention:
- Address the duplicate entries for the "Azul" game.
- Enhance accessibility throughout the component, especially in the board game cards and newsletter form.
- Implement proper form handling for the newsletter subscription.
- Consider moving inline styles to CSS classes for better maintainability.
These improvements will further enhance the quality and user experience of the component.
43-73:⚠️ Potential issueImproved code structure using array and map function.
The use of an array to store board game data and the
mapfunction to render them dynamically is a significant improvement. This approach enhances code maintainability and scalability, making it easier to add or remove board games in the future.Address duplicate entries for the game "Azul".
There are multiple entries for the game "Azul" with different image sources but identical titles and descriptions. This appears to be unintentional and needs to be corrected.
Please review and update the entries for
board6,board7, andboard8to reflect the correct board games or remove the duplicates if they are not needed.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@AbhijitMotekar99 lgtm :) |
RamakrushnaBiswal
merged commit db17962
into
RamakrushnaBiswal:main
I added hover effects, and I also cleaned up the register.jsx and boardgame.jsx code for the cards using the map method to make it shorter.
Perspective.hover.enhanced.mp4
perspective.2.mp4
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation