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

Created Navbar Components #24

Merged
merged 36 commits into from Oct 16, 2022
Merged

Created Navbar Components #24

merged 36 commits into from Oct 16, 2022

Conversation

Onyelaudochukwuka
Copy link
Contributor

@Onyelaudochukwuka Onyelaudochukwuka commented Oct 4, 2022

πŸ› οΈ Fixes Issue

closes #17

πŸ‘¨β€πŸ’» Changes proposed

  • Added 12 new Navbars
  • Removed the coming soon label

βœ”οΈ Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

πŸ“„ Note to reviewers

πŸ“· Screenshots

@vercel
Copy link

vercel bot commented Oct 4, 2022

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated
drip-ui βœ… Ready (Inspect) Visit Preview Oct 16, 2022 at 1:59PM (UTC)

@khazifire
Copy link
Owner

@Onyelaudochukwuka since most of the headers are the same with the difference being only in the hamburger menu (you can consider removing some of them, since its just duplicates)

for instance (2 lines and 3 line hamburger , its the same thing), so i think you can just include the 3 lines since its the standard
and also consider adding hover effect on the nav items

Thank you

@Onyelaudochukwuka
Copy link
Contributor Author

Onyelaudochukwuka commented Oct 4, 2022

@Onyelaudochukwuka since most of the headers are the same with the difference being only in the hamburger menu (you can consider removing some of them, since its just duplicates)

for instance (2 lines and 3 line hamburger , its the same thing), so i think you can just include the 3 lines since its the standard and also consider adding hover effect on the nav items

Thank you

I'll fix these

@akebu6
Copy link

akebu6 commented Oct 4, 2022

@Onyelaudochukwuka kindly squash all commits into one as this is best for any production code in order to make the VC graph cleaner

@Onyelaudochukwuka
Copy link
Contributor Author

Resolved all issues

@Onyelaudochukwuka
Copy link
Contributor Author

Onyelaudochukwuka commented Oct 4, 2022

closes #17 and #20

@khazifire
Copy link
Owner

@Onyelaudochukwuka im glad you were able to fix the error that were in your previous pull request, something just caught my eyes as I was reviewing your work.

its impressible but i think for components with nested elements, you can just create them normally instead of using json

e.g. you can create nav1, nav2 component and then import them in the navbar file, that way it would be so much cleaner to update and modify later on

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Header Component
3 participants