-
-
Notifications
You must be signed in to change notification settings - Fork 103
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.
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
changes the font of navbar and added a hover effect. #103
Conversation
@Ayush215mb 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 pull request introduces styling and structural modifications to the Changes
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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! 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
frontend/src/index.css (4)
1-1
: LGTM! Consider moving the .Poppins class into a layer.The addition of the Poppins font and the corresponding class is a good improvement for typography consistency. The import provides a wide range of weights and styles, offering flexibility in design.
Consider moving the .Poppins class into the @layer base directive for better organization and to follow Tailwind's recommended structure:
@layer base { .Poppins { font-family: "Poppins", sans-serif; } }Also applies to: 15-17
Line range hint
9-13
: Consider adding cross-browser support for text outline.The addition of the .font-outline-2 class is a nice touch for creating outlined text. However, the current implementation only works in WebKit-based browsers.
To improve cross-browser compatibility, consider using a combination of text-shadow properties. Here's an example:
@layer base { .font-outline-2 { -webkit-text-stroke: 2px white; text-shadow: -2px -2px 0 #fff, 2px -2px 0 #fff, -2px 2px 0 #fff, 2px 2px 0 #fff; } }This approach will provide a similar effect in browsers that don't support -webkit-text-stroke.
Line range hint
21-26
: LGTM! Consider improving readability of shadow values.The updates to .primary-btn and .page-shadow classes are good improvements. The complex shadow effect in .page-shadow can create an interesting depth effect.
To improve readability of the shadow values in .page-shadow, consider breaking them into multiple lines:
.page-shadow { @apply shadow-[ inset_0_0_8px_rgba(0,0,0,0.6), 12px_12px_14px_rgba(0,0,0,0.9) ]; }This format makes it easier to read and modify the shadow values if needed in the future.
Line range hint
30-44
: Consider adding cross-browser support for scrollbar styling.The addition of custom scrollbar styles is a nice touch for improving the user interface. However, the current implementation only works in WebKit-based browsers.
To improve cross-browser compatibility and maintain consistency with your project's color scheme, consider the following:
- Use CSS variables for colors to ensure consistency and easy updates:
:root { --scrollbar-bg: beige; --scrollbar-hover: #004D43; }
- Add Firefox-specific scrollbar styling:
* { scrollbar-width: thin; scrollbar-color: var(--scrollbar-bg) transparent; } *:hover { scrollbar-color: var(--scrollbar-hover) transparent; }
- Update the WebKit scrollbar styles to use the CSS variables:
::-webkit-scrollbar { width: 7px; height: 7px; } ::-webkit-scrollbar-thumb { background-color: var(--scrollbar-bg); border-radius: 10px; } ::-webkit-scrollbar-thumb:hover { background-color: var(--scrollbar-hover); }This approach will provide a more consistent scrollbar appearance across different browsers while making it easier to update the color scheme in the future.
frontend/src/components/Shared/Navbar.jsx (2)
101-103
: Improved desktop menu styling and interaction.The changes enhance the visual appeal and user experience of the desktop menu. The addition of the Poppins font, increased spacing, and larger font size improve readability. The hover effect with scale and translation provides better visual feedback to users.
Consider adding a smooth transition to the hover effect for a more polished look:
- <li key={item.name} className="transform hover:scale-110 hover:-translate-y-1 transition "> + <li key={item.name} className="transform hover:scale-110 hover:-translate-y-1 transition duration-300 ease-in-out">
124-125
: Consistent styling for login button.The login button now has the same hover effect as the menu items and logout button, which further improves the overall consistency of the user interface. This change enhances the user experience by providing uniform interaction across all clickable elements in the navbar.
Consider adding an
aria-label
to the login and logout buttons to improve accessibility:- <button + <button + aria-label="Log In" onClick={handleLogin} className={`${baseTextColorClass} ${hoverTextColorClass} transform hover:scale-110 hover:-translate-y-1 transition `} type="button" > Log In </button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- frontend/src/components/Shared/Navbar.jsx (2 hunks)
- frontend/src/index.css (2 hunks)
🧰 Additional context used
🔇 Additional comments (2)
frontend/src/components/Shared/Navbar.jsx (2)
114-116
: Consistent styling for logout button.The logout button now has the same hover effect as the menu items, which improves the overall consistency of the user interface. This change enhances the user experience by providing uniform interaction across all clickable elements in the navbar.
Line range hint
1-190
: Overall improvement in navbar styling and consistency.The changes made to the Navbar component enhance its visual appeal and user experience without affecting its core functionality. The consistent hover effects across menu items and buttons create a more cohesive interface.
Regarding the addition of the Poppins font:
- Ensure that the Poppins font is properly imported in your project, either through a CSS import or a link in the HTML file.
- Consider adding a fallback font for cases where Poppins might not be available:
- <ul className="ml-4 flex space-x-5 Poppins font-semibold text-lg"> + <ul className="ml-4 flex space-x-5 font-poppins font-semibold text-lg">Then, in your CSS (probably in
index.css
):@import url('https://fonts.googleapis.com/css2?family=Poppins:wght@400;600&display=swap'); .font-poppins { font-family: 'Poppins', sans-serif; }This approach ensures that the Poppins font is properly loaded and provides a fallback if it's not available.
To verify the font import, please run the following script:
✅ Verification successful
Let's proceed to search the entire codebase for any references to the "Poppins" font to determine where it is being imported or used.
Poppins font is properly imported and used.
The Poppins font is correctly imported in
frontend/src/index.css
and utilized within theNavbar
component and Tailwind configuration. No further action is required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if Poppins font is imported in index.css or index.html echo "Checking for Poppins font import in index.css:" rg --type css "font-family.*Poppins" src/index.css echo "Checking for Poppins font import in index.html:" rg "https://fonts.googleapis.com/css.*family=Poppins" public/index.html echo "If no results are shown, the Poppins font might not be properly imported."Length of output: 603
Script:
#!/bin/bash # Search for 'Poppins' in all CSS and JavaScript/JSX files echo "Searching for Poppins font imports in CSS files:" rg --type css "Poppins" echo "Searching for Poppins font imports in JavaScript/JSX files:" rg --type js "Poppins" echo "If no results are found, the Poppins font might not be imported or used in the codebase."Length of output: 980
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
LGTM 🚀 |
1e53ac4
into
RamakrushnaBiswal:main
@RamakrushnaBiswal You also have to assign me here and add labels in this PR for hacktober and gssoc to count it. |
I am again creating a pull request as my previous one was having many conflicts.
Please accept it as soon as possible because as you will accept other PR, this codebase will becomes outdated creating conflicts.
Summary by CodeRabbit
New Features
Bug Fixes
Style