-
Notifications
You must be signed in to change notification settings - Fork 16
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
Frankie Power's Profile Page #19
base: main
Are you sure you want to change the base?
Conversation
@FrankiePower is attempting to deploy a commit to the BuidlGuidl Team on Vercel. A member of the Team first needs to authorize it. |
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.
Thanks for your PR, FrankiePower! I appreciate how descriptive your PR message is 🔥
Please take a look at my comments and let me know if you have any questions!
packages/nextjs/public/0x24e765Fcd00106D7175837848ec9073f9fEb9d8e.jpeg
Outdated
Show resolved
Hide resolved
packages/nextjs/app/builders/0x24e765Fcd00106D7175837848ec9073f9fEb9d8e/page.tsx
Outdated
Show resolved
Hide resolved
packages/nextjs/app/builders/0x24e765Fcd00106D7175837848ec9073f9fEb9d8e/page.tsx
Outdated
Show resolved
Hide resolved
packages/nextjs/app/builders/0x24e765Fcd00106D7175837848ec9073f9fEb9d8e/page.tsx
Outdated
Show resolved
Hide resolved
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 again! Thanks for deleting package-lock.json
and making the public
folder.
I have two more requests related to the image filenames and yarn.lock
, when you have a moment.
packages/nextjs/app/builders/0x24e765Fcd00106D7175837848ec9073f9fEb9d8e/page.tsx
Outdated
Show resolved
Hide resolved
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 the updates!
A friendly tip: ensure your PR descriptions are concise and feel authentic. To me as a reviewer, it seems it was just copied from an LLM :)
Also, make sure to run your code before pushing it and requesting feedback. If you had, you'd have noticed that the pictures aren’t displaying as expected.
See below my comments.
Please don't hesitate to reach out if anything is unclear.
href: string; | ||
} | ||
|
||
const profileImages = ["/image1.jpeg", "/image2.jpg"]; |
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.
Pls make sure you have the right paths to the picture.
Have you tried to run your page before pushing? Because like this it cannot work :)
<button | ||
onClick={handleNextImage} | ||
className="bg-white rounded-full dark:bg-base-300 btn btn-circle" | ||
aria-label="Switch profile picture" | ||
> |
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.
Make sure that the format of the pictures are the same, so that when I click the button the code doesn't jump around.
<button | ||
onClick={toggleLocation} | ||
className="bg-white rounded-full dark:bg-base-300 btn btn-circle relative" | ||
aria-label={showLocation ? "Show globe" : "Show location"} | ||
> | ||
<div className="transition-all duration-300 transform group-hover:scale-110"> | ||
{showLocation ? <NigerianFlag /> : <Globe className="h-6 w-6" />} | ||
</div> | ||
</button> |
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.
What is the button for? Just to switch between the Nigerian flag and the globe symbol?
To me it seems the code is not complete, since its called toggleLocation?
const [showLocation, setShowLocation] = useState(false); | ||
|
||
const handleNextImage = () => { | ||
setCurrentImage(prev => (prev + 1) % profileImages.length); |
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.
Since we have a constant array profileImages
, why do we need this logic?
@FrankiePower don't forget your personal page :) |
834d34e
to
da6fa7c
Compare
@FrankiePower in here should be only files that relates to your personal page review, please. |
@FrankiePower if you're not sure how to proceed, look up |
Add Personal Profile Page
Overview
Added a new profile page that serves as a portfolio/introduction. The page showcases my experience as a Web3 developer, includes social links, and provides easy access to my contact information.
Features
Technical Implementation
next/image
for optimized image loadingComponents Added
pages/profile.tsx
)Screenshots
Additional Information
Related Issues #11
Your ENS/address: 0x24e765Fcd00106D7175837848ec9073f9fEb9d8e