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.

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

Progress bar overlaps navbar fixed #1881

Merged
merged 8 commits into from
Jul 22, 2024
Merged

Conversation

Priyal208
Copy link
Contributor

@Priyal208 Priyal208 commented Jul 17, 2024

Fixes Issue

fixes #1834

Changes proposed

The progrss bar earlier overlapped navbar and hid some portion of navbar, I gave appropriate padding and fixed it so that overlapping is fixed

Screenshots

Screenshot 2024-07-16 205245

Note to reviewers

You can see that overlap is now gone

Summary by CodeRabbit

  • New Features

    • Introduced a responsive UserNavbar component that enhances navigation with user authentication, search capabilities, and dynamic dropdown menus.
    • Added a mobile-friendly toggle for navigation links, improving usability on smaller devices.
  • Style

    • Improved navigation bar padding for better spacing on small screens.

Copy link

vercel bot commented Jul 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
vigybag ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2024 3:30pm

Copy link
Contributor

coderabbitai bot commented Jul 17, 2024

Walkthrough

The recent changes enhance the UserNavbar component by introducing responsive design features. The top padding has been adjusted to improve layout on smaller devices, addressing the overlap with the progress bar. Additionally, the component now offers user authentication and navigation functionalities, enriching the overall user experience.

Changes

Files Change Summary
src/components/Navbar/UserNavbar.jsx Updated top padding class from pt-4 to pt-4 sm:pt-5; added responsive navigation features, including user authentication and mobile toggle.

Sequence Diagram(s)

Not applicable—changes are primarily related to styling and new component functionality without affecting control flow.

Assessment against linked issues

Objective Addressed Explanation
Add necessary padding to prevent navbar and progress bar overlap (#1834)

Poem

In code, we weave a small design,
For UserNavbar to align,
No more overlap, smooth and clear,
The padding fixed, so never fear.
Now on small screens, it shall shine,
A tweak so simple, yet so fine! 🌟


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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, codebase verification and nitpick comments (1)
src/components/Navbar/UserNavbar .jsx (1)

Line range hint 1-65: General review of the Navbar component.

The Navbar component is well-structured with clear separation of concerns and appropriate use of React hooks and conditional rendering. The use of local storage for managing authentication state (isLoggedIn, username) is straightforward, though consider the security implications of storing such information in local storage.

The commented-out useEffect for fetching the username could be removed if it's no longer needed, to clean up the codebase.

-  // useEffect(() => {
-  //   const fetchUsername = async () => {
-  //     try {
-  //       const username = localStorage.getItem("username");
-  //       console.log(username);
-  //     } catch (error) {
-  //       console.error("Error fetching username:", error);
-  //       setUsername("User");
-  //     }
-  //   };
-  //
-  //   if (isLoggedIn) {
-  //     fetchUsername();
-  //   }
-  // }, [isLoggedIn]);

Consider adding more robust error handling and user feedback mechanisms, especially around the logout process to enhance user experience.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1beba07 and 38e29b7.

Files selected for processing (1)
  • src/components/Navbar/UserNavbar .jsx (1 hunks)
Additional comments not posted (1)
src/components/Navbar/UserNavbar .jsx (1)

67-67: Padding adjustment to prevent overlap is appropriate.

The change from pt-4 to pt-4 sm:pt-5 correctly adds extra padding on small screens, which should prevent the navbar from overlapping with the progress bar as reported. This is a targeted and minimal change that addresses the issue effectively.

Please ensure to verify the visual layout on different screen sizes to confirm that the adjustment behaves as expected across all devices.

@Priyal208
Copy link
Contributor Author

@codervivek5 kindly review this

@codervivek5
Copy link
Owner

fix merge conflict

src/components/Navbar/UserNavbar .jsx

remove extra space from the file name

  • UserNavbar .jsx to UserNavbar.jsx

share any image or video for review your changes

@codervivek5
Copy link
Owner

pull the code first

@Priyal208
Copy link
Contributor Author

@codervivek5 i have attached the image in the pr desc pl check

@Priyal208
Copy link
Contributor Author

@codervivek5 now pls check

@Priyal208
Copy link
Contributor Author

@codervivek5 review this pls now

Copy link

vercel bot commented Jul 20, 2024

@Priyal208 is attempting to deploy a commit to the Vivek Prajapati's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Priyal208
Copy link
Contributor Author

@codervivek5 what about this pr pls check

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (2)
src/components/Navbar/UserNavbar.jsx (2)

10-10: Consider renaming the component to UserNavbar.

The file name suggests UserNavbar, but the component is named Navbar. For consistency, consider renaming the component.

- const Navbar = ({ isAdmin }) => {
+ const UserNavbar = ({ isAdmin }) => {

13-34: Remove commented-out code.

The commented-out code related to fetching the username can be removed for better readability.

-  // const [username, setUsername] = useState("");
-  // useEffect(() => {
-  //   const fetchUsername = async () => {
-  //     try {
-  //       const username = localStorage.getItem("username");
-  //       console.log(username);
-  //     } catch (error) {
-  //       console.error("Error fetching username:", error);
-  //       setUsername("User");
-  //     }
-  //   };
-  //   if (isLoggedIn) {
-  //     fetchUsername();
-  //   }
-  // }, [isLoggedIn]);
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 38e29b7 and b1f775d.

Files selected for processing (1)
  • src/components/Navbar/UserNavbar.jsx (1 hunks)
Additional comments not posted (11)
src/components/Navbar/UserNavbar.jsx (11)

1-8: LGTM!

The import statements are appropriate and necessary for the functionality provided in the file.


36-36: LGTM!

The toggleNavbar function is straightforward and correct.


37-37: LGTM!

The handleSearch function is straightforward and correct.


49-51: LGTM!

The handleDropdownToggle function is straightforward and correct.


53-57: LGTM!

The handleClickOutside function is well-implemented and necessary for managing the dropdown state.


59-64: LGTM!

The useEffect hook for handling clicks outside the dropdown is correctly implemented.


71-71: LGTM!

The NavLogo component usage is correct.


108-111: LGTM!

The SearchBar component usage is correct.


112-112: LGTM!

The CartIcon component usage is correct.


181-190: LGTM!

The MobileMenu component usage is correct.


39-47: Fix localStorage.setItem usage.

The localStorage.setItem should use a boolean value for isLoggedIn.

-      localStorage.setItem("isLoggedIn", false);
+      localStorage.setItem("isLoggedIn", "false");

Likely invalid or redundant comment.

Comment on lines +66 to +191
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 text-base font-bold">
Stationary
</Link>
<Link
to="/popularCategories/bodyCare"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 text-base font-bold">
Body-Care
</Link>
</div>
</div>
</div>
</div>

<div className="flex items-center">
<div className="md:block hidden">
<div className="ml-4 flex items-center md:ml-6 gap-6">
<SearchBar
searchTerm={searchTerm}
handleSearch={handleSearch}
/>
<CartIcon />
{isLoggedIn ? (
<div className="relative flex gap-3 items-center">
{/* need styling fix then implement only */}
{/* <p className="mr-2 overflow-hidden flex text-gray-800">{`Hi, ${username}`}</p> */}
<FaUserCircle
onClick={handleDropdownToggle}
className="text-3xl cursor-pointer"
/>
<span className="text-green-700 font-bold">{username}</span>
{showDropdown && (
<div
ref={dropdownRef}
className="absolute right-0 mt-32 w-48 bg-white rounded-md shadow-lg py-1">
<Link
to="/dashboard"
className="block px-4 py-2 text-gray-800 hover:bg-gray-200">
Dashboard
</Link>
<button
onClick={handleLogout}
className="block px-4 py-2 text-gray-800 hover:bg-gray-200 w-full text-left">
Logout
</button>
</div>
)}
</div>
) : (
<AuthButton isLoggedIn={isLoggedIn} />
)}
</div>
</div>
<div className="-mr-2 flex md:hidden">
<button
onClick={toggleNavbar}
className="inline-flex items-center justify-center p-2 rounded-md text-green-800 hover:text-gray-600 focus:outline-none">
{isOpen ? (
<svg
className="h-6 w-6"
stroke="#15803D"
fill="#15803D"
viewBox="0 0 24 24">
<path
strokeLinecap="round"
strokeLinejoin="round"
strokeWidth="2"
d="M6 18L18 6M6 6l12 12"
/>
</svg>
) : (
<svg
className="h-6 w-6"
stroke="#15803D"
fill="#15803D"
viewBox="0 0 24 24">
<path
strokeLinecap="round"
strokeLinejoin="round"
strokeWidth="2"
d="M4 6h16M4 12h16M4 18h16"
/>
</svg>
)}
</button>
</div>
</div>
</div>
</div>

<MobileMenu
isOpen={isOpen}
searchTerm={searchTerm}
handleSearch={handleSearch}
handleDropdown={handleDropdownToggle}
openDropdown={showDropdown}
isLoggedIn={isLoggedIn}
handleLogout={handleLogout}
username={username}
/>
</nav>
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve class names for readability.

The class names for styling can be improved for readability and maintainability.

-    <nav className="bg-[#ecd5c5] shadow-lg md:w-full fixed md:z-50 -mt-1 w-[100vw] z-50 pt-4 sm:pt-5">
+    <nav className="bg-[#ecd5c5] shadow-lg w-full fixed z-50 pt-4 sm:pt-5 -mt-1">
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
return (
<nav className="bg-[#ecd5c5] shadow-lg md:w-full fixed md:z-50 -mt-1 w-[100vw] z-50 pt-4 sm:pt-5">
<div className="max-w-7xl mx-auto px-4 sm:px-6 lg:px-8">
<div className="flex justify-between h-20">
<div className="flex items-center w-full">
<NavLogo />
<div className="hidden md:block lg:block">
<div className="ml-10 flex items-baseline space-x-4">
<div className="py-1 flex justify-evenly items-center">
<Link
to="/popularCategories/fashionAccessories"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 font-bold text-base">
Fashion
</Link>
<Link
to="/popularCategories/customizedGifts"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 text-base font-bold">
Gifts
</Link>
<Link
to="/popularCategories/furnitureDecor"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 text-base font-bold">
Furniture
</Link>
<Link
to="/popularCategories/printingStationery"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 text-base font-bold">
Stationary
</Link>
<Link
to="/popularCategories/bodyCare"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 text-base font-bold">
Body-Care
</Link>
</div>
</div>
</div>
</div>
<div className="flex items-center">
<div className="md:block hidden">
<div className="ml-4 flex items-center md:ml-6 gap-6">
<SearchBar
searchTerm={searchTerm}
handleSearch={handleSearch}
/>
<CartIcon />
{isLoggedIn ? (
<div className="relative flex gap-3 items-center">
{/* need styling fix then implement only */}
{/* <p className="mr-2 overflow-hidden flex text-gray-800">{`Hi, ${username}`}</p> */}
<FaUserCircle
onClick={handleDropdownToggle}
className="text-3xl cursor-pointer"
/>
<span className="text-green-700 font-bold">{username}</span>
{showDropdown && (
<div
ref={dropdownRef}
className="absolute right-0 mt-32 w-48 bg-white rounded-md shadow-lg py-1">
<Link
to="/dashboard"
className="block px-4 py-2 text-gray-800 hover:bg-gray-200">
Dashboard
</Link>
<button
onClick={handleLogout}
className="block px-4 py-2 text-gray-800 hover:bg-gray-200 w-full text-left">
Logout
</button>
</div>
)}
</div>
) : (
<AuthButton isLoggedIn={isLoggedIn} />
)}
</div>
</div>
<div className="-mr-2 flex md:hidden">
<button
onClick={toggleNavbar}
className="inline-flex items-center justify-center p-2 rounded-md text-green-800 hover:text-gray-600 focus:outline-none">
{isOpen ? (
<svg
className="h-6 w-6"
stroke="#15803D"
fill="#15803D"
viewBox="0 0 24 24">
<path
strokeLinecap="round"
strokeLinejoin="round"
strokeWidth="2"
d="M6 18L18 6M6 6l12 12"
/>
</svg>
) : (
<svg
className="h-6 w-6"
stroke="#15803D"
fill="#15803D"
viewBox="0 0 24 24">
<path
strokeLinecap="round"
strokeLinejoin="round"
strokeWidth="2"
d="M4 6h16M4 12h16M4 18h16"
/>
</svg>
)}
</button>
</div>
</div>
</div>
</div>
<MobileMenu
isOpen={isOpen}
searchTerm={searchTerm}
handleSearch={handleSearch}
handleDropdown={handleDropdownToggle}
openDropdown={showDropdown}
isLoggedIn={isLoggedIn}
handleLogout={handleLogout}
username={username}
/>
</nav>
return (
<nav className="bg-[#ecd5c5] shadow-lg w-full fixed z-50 pt-4 sm:pt-5 -mt-1">
<div className="max-w-7xl mx-auto px-4 sm:px-6 lg:px-8">
<div className="flex justify-between h-20">
<div className="flex items-center w-full">
<NavLogo />
<div className="hidden md:block lg:block">
<div className="ml-10 flex items-baseline space-x-4">
<div className="py-1 flex justify-evenly items-center">
<Link
to="/popularCategories/fashionAccessories"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 font-bold text-base">
Fashion
</Link>
<Link
to="/popularCategories/customizedGifts"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 text-base font-bold">
Gifts
</Link>
<Link
to="/popularCategories/furnitureDecor"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 text-base font-bold">
Furniture
</Link>
<Link
to="/popularCategories/printingStationery"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 text-base font-bold">
Stationary
</Link>
<Link
to="/popularCategories/bodyCare"
className="text-green-800 hover:text-green-500 hover:underline block px-4 py-2 text-base font-bold">
Body-Care
</Link>
</div>
</div>
</div>
</div>
<div className="flex items-center">
<div className="md:block hidden">
<div className="ml-4 flex items-center md:ml-6 gap-6">
<SearchBar
searchTerm={searchTerm}
handleSearch={handleSearch}
/>
<CartIcon />
{isLoggedIn ? (
<div className="relative flex gap-3 items-center">
{/* need styling fix then implement only */}
{/* <p className="mr-2 overflow-hidden flex text-gray-800">{`Hi, ${username}`}</p> */}
<FaUserCircle
onClick={handleDropdownToggle}
className="text-3xl cursor-pointer"
/>
<span className="text-green-700 font-bold">{username}</span>
{showDropdown && (
<div
ref={dropdownRef}
className="absolute right-0 mt-32 w-48 bg-white rounded-md shadow-lg py-1">
<Link
to="/dashboard"
className="block px-4 py-2 text-gray-800 hover:bg-gray-200">
Dashboard
</Link>
<button
onClick={handleLogout}
className="block px-4 py-2 text-gray-800 hover:bg-gray-200 w-full text-left">
Logout
</button>
</div>
)}
</div>
) : (
<AuthButton isLoggedIn={isLoggedIn} />
)}
</div>
</div>
<div className="-mr-2 flex md:hidden">
<button
onClick={toggleNavbar}
className="inline-flex items-center justify-center p-2 rounded-md text-green-800 hover:text-gray-600 focus:outline-none">
{isOpen ? (
<svg
className="h-6 w-6"
stroke="#15803D"
fill="#15803D"
viewBox="0 0 24 24">
<path
strokeLinecap="round"
strokeLinejoin="round"
strokeWidth="2"
d="M6 18L18 6M6 6l12 12"
/>
</svg>
) : (
<svg
className="h-6 w-6"
stroke="#15803D"
fill="#15803D"
viewBox="0 0 24 24">
<path
strokeLinecap="round"
strokeLinejoin="round"
strokeWidth="2"
d="M4 6h16M4 12h16M4 18h16"
/>
</svg>
)}
</button>
</div>
</div>
</div>
</div>
<MobileMenu
isOpen={isOpen}
searchTerm={searchTerm}
handleSearch={handleSearch}
handleDropdown={handleDropdownToggle}
openDropdown={showDropdown}
isLoggedIn={isLoggedIn}
handleLogout={handleLogout}
username={username}
/>
</nav>

@Priyal208
Copy link
Contributor Author

@codervivek5 I have 3PRs pending to be reviewed since last 1 week, pls look into that

@Priyal208
Copy link
Contributor Author

@codervivek5 MERGE THIS, its been a week im trying to get a review, i have 3PRS pending to be reviewed, pls look into that

@codervivek5 codervivek5 merged commit 66d3e3d into codervivek5:main Jul 22, 2024
7 of 8 checks passed
@Priyal208
Copy link
Contributor Author

@codervivek5 add gssoc label and level to this, its not counted in my gssoc points its been 3 days

@codervivek5 codervivek5 added good first issue Good for newcomers gssoc labels Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

progress bar and navbar co-incide
2 participants