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

Add builders list #19

Merged
merged 6 commits into from
Jun 10, 2024
Merged

Conversation

wildanvin
Copy link
Contributor

Description

I added a list with the builders that have checked in. It displays the builder's address and an icon that takes you to the builder page.

image

Additional Information

Related Issues

Hopefully it will close issue #3

Your ENS/address: wildanvin.eth

Copy link

vercel bot commented Jun 6, 2024

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

Name Status Preview Comments Updated (UTC)
batch6-buidlguidl-com-nextjs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 8, 2024 6:54pm

Copy link
Member

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

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

Hey thanks @wildanvin !! What if we move this logic, tidy a bit and create a new page for /builders route ?

Maybe handle home page styling separately in #6 also discussing with peer's what to show on home page ?

@wildanvin
Copy link
Contributor Author

Absolutely, I will create the builders page then.

I will suggest updating the nav bar on the home page in order to access the new builders page

@technophile-04
Copy link
Member

I will suggest updating the nav bar on the home page in order to access the new builders page

Ohh yes we should def add it !

Also btw, no need to create a new PR you can revert the changes to home page by doing :

git revert ${commithash} in this this case git revert f689658 this will revert your last commit (that is changes to home page) and then you can create new page.tsx inside app/builders

This reverts commit f689658.
@wildanvin
Copy link
Contributor Author

Hello! I was about to ask you about making a new PR hehe...
I realized that there are changes merged in the home page regarding the issue #5 "Get the check-in count from the Batch Registry contract". If I do git revert f689658 I won't have those changes and there will be a conflict when trying to merge right?

@wildanvin
Copy link
Contributor Author

Hmmm... hopefully all is good now...
image

@jriyyya
Copy link
Contributor

jriyyya commented Jun 7, 2024

@wildanvin Looks good 🚀, But you forgot the Navbar update

@wildanvin
Copy link
Contributor Author

@wildanvin Looks good 🚀, But you forgot the Navbar update

you are right! I think i will make that in a different PR

@Okhayeeli
Copy link
Contributor

Great job @wildanvin !!!

@technophile-04
Copy link
Member

Thanks @wildanvin for all the changes !!


you are right! I think i will make that in a different PR

I think it makes sense to add it in this PR itself ? It should be just couple of lines addition in Header.tsx and once we merge this PR, could you easily navigate to builders page instead of manually typing the URL . What do you say?

@wildanvin
Copy link
Contributor Author

I added the builders page in the navbar. I guess you were right @jriyyya 😅

addBuildersInNavbar.mp4

@carletex carletex requested a review from technophile-04 June 10, 2024 10:30
Copy link
Member

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

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

Works and looks great !! Thanks @wildanvin and also other for review !!

@technophile-04 technophile-04 merged commit c967d62 into BuidlGuidl:main Jun 10, 2024
3 checks passed
@wildanvin
Copy link
Contributor Author

Awesome! thanks a lot for the feedback and explanations @technophile-04 !!

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

Successfully merging this pull request may close these issues.

4 participants