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

Implementation of issue number 6 #27

Merged
merged 4 commits into from
Aug 26, 2024
Merged

Conversation

ruhneb2004
Copy link
Contributor

Description

Added a batch member logo on the left hand side of the footer, this will only appear if they are in the allowList. There is also a button on the right side of showing whether they have done the check In or not. If they have then on clicking the button they can see a alert showing the contract address they have deployed. If not then on click they will be redirected to the issue #10.

Screenshot 2024-08-21 at 8 39 34 PM Screenshot 2024-08-21 at 8 46 47 PM Screenshot 2024-08-21 at 8 41 30 PM

Additional Information

Related Issues

Closes #{10}

Your ENS/address: 0xBC428Bb80B1cc3C29164820528819Abf6b20cB88

Copy link

vercel bot commented Aug 21, 2024

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

Name Status Preview Comments Updated (UTC)
batch8-buidlguidl-com-nextjs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 26, 2024 8:18pm

@derrekcoleman
Copy link
Collaborator

This is a visually lovely PR, @ruhneb2004! I love the footer idea so they can see the custom state on every page!

I suggest you change your message that currently uses the browser's native alert window to something with prettier UI and smoother UX, such as Daisy UI's modal component. Then you can provide the link to the check-in issue as a link or a button, whatever you think makes more sense.

Make sure to rebase on recent commits to main to avoid the Lint errors that have been fixed, too.

@ruhneb2004
Copy link
Contributor Author

ruhneb2004 commented Aug 22, 2024

The above commit was not intended. I am really new to github and it's workings. Sorry!

@ruhneb2004
Copy link
Contributor Author

Changed the alert component to the daisy ui modal component. The ones who have checkedIn can now see their contract in etherscan.io and the ones who have not done the challenge can no visit the issue#10 by clicking the link.

the ones who have checkedIn

Screenshot 2024-08-22 at 9 11 52 PM

the ones who have not checkedIn

Screenshot 2024-08-22 at 9 12 47 PM

Everything else is same!

@derrekcoleman
Copy link
Collaborator

The above commit was not intended. I am really new to github and it's workings. Sorry!

Totally okay! Learning how to use git rebase to "cherry pick" which commits you want to keep is exactly what we're here to practice 🥷

Your updates are looking really nice, @ruhneb2004! Do you want to push it one step further by playing with the styling of the modal so the buttons are more centered and spaced within the width of the container?

@ruhneb2004
Copy link
Contributor Author

The github is somewhat making sense now, I have done some little projects and stuff but I haven't done this sort of stuff before, so this is really a cool learning experience.

And for the modal, I would love to give it an another try

@ruhneb2004
Copy link
Contributor Author

I have adjusted the spacing of the buttons and some padding for the text as shown below for the modal

Screenshot 2024-08-24 at 1 25 00 PM Screenshot 2024-08-24 at 1 28 33 PM

Earlier the styling was not adjusted to the light mode and the text inside the logo was not visible for the batch member logo in the light mode, I have also fixed those issues.

Screenshot 2024-08-24 at 1 26 36 PM Screenshot 2024-08-24 at 1 29 19 PM

Todo

After the list of the batch members are out to display I am thinking about making the batch member logo point to our list of the builders

Copy link
Collaborator

@derrekcoleman derrekcoleman left a comment

Choose a reason for hiding this comment

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

Loving the updated styling on the modal! 🤩

The last step is to learn how to remove the accidental commits from this pull request before I merge. I had to do the same thing recently and found this Stack Overflow answer very useful. git can be scary, but visual tools like VS Code's interactive rebase make it easier to choose which commits to keep and which to drop!

Try it out and let us know how it goes 💪

@ruhneb2004
Copy link
Contributor Author

ruhneb2004 commented Aug 26, 2024

I learned something new today! I have removed all other commits apart from the one related to this issue. I hope this is what you were looking for @derrekcoleman 🤞

@derrekcoleman derrekcoleman merged commit 0ebdcc2 into BuidlGuidl:main Aug 26, 2024
3 checks passed
@derrekcoleman
Copy link
Collaborator

Great job, @ruhneb2004! Your git-fu has become stronger today 🥷

@ruhneb2004
Copy link
Contributor Author

Haha! Thanks 😊

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.

6. Show connected wallet info
2 participants