-
Notifications
You must be signed in to change notification settings - Fork 12
Add personal builder page for Khano #21
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
melanke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your personal page ^^
I'd like to ask you to improve your PR's description:
- Try sticking to the PR description template so everything stays consistent.
- Please include a screenshot of your page. This is a common practice in many open-source projects and helps reviewers quickly spot issues and navigate through the many PRs we handle. It would be really helpful if you could attach versions in both light and dark themes, and a mobile view as well.
- Please link the issue in the PR description. You can do this using
#followed by the issue number.
And here's my review:
packages/nextjs/app/builders/0x5258c93FbF34600a89bb1B7A57Fe9B981A693Fc7/page.tsx
Outdated
Show resolved
Hide resolved
packages/nextjs/app/builders/0x5258c93FbF34600a89bb1B7A57Fe9B981A693Fc7/page.tsx
Outdated
Show resolved
Hide resolved
|
Hey! I removed the unrelated Hardhat files as requested. |
melanke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠ Please, check my previous comment about the PR description: #21 (review)
packages/nextjs/app/builders/0x5258c93FbF34600a89bb1B7A57Fe9B981A693Fc7/page.tsx
Show resolved
Hide resolved
packages/nextjs/app/builders/0x5258c93FbF34600a89bb1B7A57Fe9B981A693Fc7/page.tsx
Outdated
Show resolved
Hide resolved
|
@MrKhano please, edit your PR description: |
packages/nextjs/app/builders/0x5258c93FbF34600a89bb1B7A57Fe9B981A693Fc7/AddressBlock.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| <div className="flex flex-col items-center gap-2"> | ||
| <p className="text-sm font-semibold">Address:</p> | ||
| <Address address="0x5258c93FbF34600a89bb1B7A57Fe9B981A693Fc7" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! I noticed your address in not using the checksummed version. Using EIP-55 casing is the standard and makes it easier to spot typos, which helps avoid mistakes when someone copies or validates the address.
The Address component spects it to be correctly formatted, so, could you replace it by this?
0x5258c93fBf34600a89bb1B7A57Fe9B981A693Fc7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is out of the scope of your issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is out of the scope of your issue. It's something that should be aligned with the team.

Personal Builder Page – Khano
Closes #21
This PR adds my personal builder page to the Batch 22 website.
It includes:
my avatar
my Web3 bio
my wallet address (via Scaffold UI Address component)
my relevant social links
I followed the builder page structure and styling conventions used by other contributors.
Screenshots
