search

ucey-star / ID8

Unlock your idea's potential with ID8, the ultimate idea validation tool. Whether you're brainstorming a new product, business, or innovation, our app helps you evaluate feasibility, get real-time feedback, and refine your concept
0 stars 1 forks source link

Add Navbar component based on Figma #17

Closed avalucianelson closed 7 hours ago

avalucianelson commented 3 days ago

Description

I created the Navbar component based on our Figma designs. This component includes navigation links with icons and a gradient background. It is designed to be consistent across the website. It is not yet added to anybody's pages.

Type of Change

Testing Instructions

  1. Local Setup:
    • Ensure your development environment is set up and running.
    • Switch to the feature/navbar-update branch.
  2. Integration:
    • Add the Navbar component to a page of your choice (e.g., Home or Profile).
    • Import the component: import Navbar from "../_components/Navbar";
    • Use the component in the page's JSX: .
  3. Visual Inspection:
    • Verify that the navbar matches the Figma design, with correct icons, text, and gradient borders.
  4. Functionality:
    • Click each navigation link to ensure it routes to the correct page.
    • Confirm that the current page's link is underlined.
  5. Responsive Design:
    • Test the navbar on different screen sizes to ensure it is responsive and visually consistent.
  6. Accessibility:
    • Use a screen reader to verify that each icon has an appropriate aria-label.

Please follow these steps to verify the changes. Let me know if you encounter any issues or have suggestions for improvements.

Screenshots (if applicable)

Here's the Navbar in Figma:

image

Here's the Navbar I made:

image

As you might notice, the icons don't have the same gradient in my code as they do in Figma. Me and all of my AI tools couldn't puzzle that one out -- if you can, that'd be incredible! But I think this works for now, so that we have a Navbar more similar to what we had in Figma. I think we now need to delete the "Header" component in the codebase right now, but I'm not sure (wanted to check before doing so).

katyaivaniuk commented 3 days ago

Awesome work @avalucianelson! Very easy to understand all the changes you have done, and how they contribute to the project, great PR!

Formatting

Prettier Check: It seems there are code style issues in src/app/_components/Navbar.tsx. Maybe try running npx prettier --write . to fix formatting automatically to ensure consistency across the codebase.

Code

Gradient Issue: You mentioned the icons lack the same gradient as the Figma design. Experimenting with background-clip: text CSS property could help.

avalucianelson commented 2 days ago

@FlaIespa I revisited this and now all the checks passed -- I think the issue you flagged with prettier should be resolved :) plz lmk! :)

Mykhailo-Chudyk commented 1 day ago

Overall, LGTM! Here are a few points to consider:

  1. Each navigation item uses the same structure, so it might be helpful to create a separate NavItem component and import it here to reduce repetition.
  2. Agree with Flavia that we can handle rerouting to the correct pages later, so we can ignore it for now.
  3. Should we add the NavBar directly to the layout file, or continue importing it into individual pages? Something for us to consider.
  4. Lastly, I checked the responsiveness, and it looks a bit off on mobile devices. We might want to discuss with Saleh to confirm the expected behavior for phones. (See photo below for iPhone 14 Pro Max.)
image
lewiskyron commented 7 hours ago

Overall, LGTM! Here are a few points to consider:

  1. Each navigation item uses the same structure, so it might be helpful to create a separate NavItem component and import it here to reduce repetition.
  2. Agree with Flavia that we can handle rerouting to the correct pages later, so we can ignore it for now.
  3. Should we add the NavBar directly to the layout file, or continue importing it into individual pages? Something for us to consider.
  4. Lastly, I checked the responsiveness, and it looks a bit off on mobile devices. We might want to discuss with Saleh to confirm the expected behavior for phones. (See photo below for iPhone 14 Pro Max.)
image

I agree that we can introduce the navItems. Created issue https://github.com/ucey-star/ID8/issues/23 to keep track of this.