feat(builder-page): Add personal builder profile page #15

All-Khwarizmi · 2025-01-07T18:52:39Z

Description

Added personal builder profile page featuring:

Introduction & social links
Journey timeline
Focus areas
Languages

Additional Information

First contribution to batch repository
Following issue requirements

Related Issues

#11

Your ENS/address: 0x7d971C39bb700AcEc20879D46f092dC0DB1dbF9E

vercel · 2025-01-07T18:52:43Z

@All-Khwarizmi is attempting to deploy a commit to the BuidlGuidl Team on Vercel.

A member of the Team first needs to authorize it.

vercel · 2025-01-08T06:45:59Z

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

Name	Status	Preview	Comments	Updated (UTC)
batch12.buidlguidl.com	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 10, 2025 8:50am

phipsae

Thanks for your nice personal page!

You paid attention to mobile responsivness and dark/light mode.
Also the PR description is very good! Good job!

Below you'll find some little comments.

packages/nextjs/app/builders/0x7d971C39bb700AcEc20879D46f092dC0DB1dbF9E/page.tsx

All-Khwarizmi · 2025-01-08T08:16:14Z

@phipsae I've implemented the requested changes:

Added NextPage type
Changed to Next.js Image component (and updated the next.config accordingly)
Moved BUILDER_INFO to module scope and used made it read only
Renamed component for clarity

Thank you for the review and the detailed suggestions. Learning a lot already 🚀

phipsae

Thanks for the changes!

Just found a little thing. Then we are ready to merge.

phipsae · 2025-01-09T14:31:01Z

packages/nextjs/app/builders/0x7d971C39bb700AcEc20879D46f092dC0DB1dbF9E/page.tsx

+              <Image
+                src={BUILDER_INFO.avatar}
+                alt="Profile"
+                width={128}
+                height={128}
+                className="rounded-full object-cover"
+              />


In the console I see the following warning, would be nice if you could fix this pls.

Solved!

Added the priority property to improve the LCP loading.

For educational purposes here's the Next.js docs on the Image component priority property and a related explanation of the LCP (and core web vitals) .

phipsae

I just discovered another small issue that causes the following error during deployment to Vercel:

Type error: Page "app/builders/0x7d971C39bb700AcEc20879D46f092dC0DB1dbF9E/page.tsx" does not match the required types of a Next.js Page. "JasonBuilderProfile" is not a valid Page export field.

phipsae · 2025-01-09T14:46:56Z

packages/nextjs/app/builders/0x7d971C39bb700AcEc20879D46f092dC0DB1dbF9E/page.tsx

+    },
+  ],
+} as const;
+export const JasonBuilderProfile: NextPage = () => {


To ensure compatibility with the Next.js deployment on vercel, we need to include a default export at the bottom of the code, such as export default JasonBuilderProfile.

I had the default export actually.

I had to remove the named export for this to work.

…. Updated profile page

phipsae · 2025-01-10T05:17:42Z

I am a bit confused, why is @NikolaiL pushing his page to this PR?
@All-Khwarizmi: If you want to have the latest version of the app, pls merge main into it.
We don't have have his profile page in this PR.

Let me know when you have any questions or need support!

- Add priority attribut to Next.js Image component to improve Largest Contentful Paint (LCP). Since the image is detected in this layer we prioritize faster loading than lazy one. - Remove `named` export in favor of the `default` to be complaint with the NextPage type.

All-Khwarizmi · 2025-01-10T05:26:10Z

I am a bit confused, why is @NikolaiL pushing his page to this PR? @All-Khwarizmi: If you want to have the latest version of the app, pls merge main into it. We don't have have his profile page in this PR.

Let me know when you have any questions or need support!

Could it be related to the fact that I rebase instead of merging to have the latest changes ?

That's what I usually do when working on a feature for some time. I feel it helps avoiding conflicts but maybe that's something that should be handled in the PR?

phipsae · 2025-01-10T06:14:58Z

I am a bit confused, why is @NikolaiL pushing his page to this PR? @All-Khwarizmi: If you want to have the latest version of the app, pls merge main into it. We don't have have his profile page in this PR.
Let me know when you have any questions or need support!

Could it be related to the fact that I rebase instead of merging to have the latest changes ?

That's what I usually do when working on a feature for some time. I feel it helps avoiding conflicts but maybe that's something that should be handled in the PR?

Ah true :)

As a reviewer, it’s confusing to see all the changes and files that have already been merged into main appearing again in this PR. This makes it harder to focus on the actual changes being introduced. Since main is already in production and your PR will be merged into it, I believe the correct approach would be to merge the latest main into your branch. This makes the review process clearer in this context clearer.

Does this makes sense?

All-Khwarizmi · 2025-01-10T07:25:35Z

I am a bit confused, why is @NikolaiL pushing his page to this PR? @All-Khwarizmi: If you want to have the latest version of the app, pls merge main into it. We don't have have his profile page in this PR.
Let me know when you have any questions or need support!

Could it be related to the fact that I rebase instead of merging to have the latest changes ?
That's what I usually do when working on a feature for some time. I feel it helps avoiding conflicts but maybe that's something that should be handled in the PR?

Ah true :)

As a reviewer, it’s confusing to see all the changes and files that have already been merged into main appearing again in this PR. This makes it harder to focus on the actual changes being introduced. Since main is already in production and your PR will be merged into it, I believe the correct approach would be to merge the latest main into your branch. This makes the review process clearer in this context clearer.

Does this makes sense?

Yes makes perfect sense!

This is the perfect example for someone wanting to argue in favor of merging over rebasing.

phipsae · 2025-01-10T08:51:31Z

Thanks a lot for the nice personal page and its merged.

phipsae requested review from derrekcoleman and phipsae and removed request for derrekcoleman January 8, 2025 04:35

phipsae assigned All-Khwarizmi Jan 8, 2025

vercel bot deployed to Preview January 8, 2025 06:48 View deployment

phipsae requested changes Jan 8, 2025

View reviewed changes

feat(builder-page): Add personal builder profile page

255faab

All-Khwarizmi force-pushed the feat/add-jason-profile branch from da536f5 to 255faab Compare January 8, 2025 07:52

refactor: implement PR review suggestions and image config

3a74b61

FrankiePower mentioned this pull request Jan 8, 2025

Frankie Power's Profile Page #19

Open

vercel bot had a problem deploying to Preview January 9, 2025 14:30 Failure

phipsae requested changes Jan 9, 2025

View reviewed changes

phipsae mentioned this pull request Jan 9, 2025

Add DimaKush personal page #20

Open

NikolaiL added 2 commits January 10, 2025 05:48

added profile page for nikolaiLeb

c04322b

added avatars.githubusercontent.com as image source to next.config.js…

353f245

…. Updated profile page

vercel bot had a problem deploying to Preview January 10, 2025 05:05 Failure

All-Khwarizmi added 2 commits January 10, 2025 06:21

fix: merge duplicate image objects

ae656e7

vercel bot deployed to Preview January 10, 2025 06:30 View deployment

Merge branch 'main' into feat/add-jason-profile

9326858

vercel bot deployed to Preview January 10, 2025 08:50 View deployment

phipsae merged commit 00f18a1 into BuidlGuidl:main Jan 10, 2025
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(builder-page): Add personal builder profile page #15

feat(builder-page): Add personal builder profile page #15

All-Khwarizmi commented Jan 7, 2025 •

edited

Loading

vercel bot commented Jan 7, 2025

vercel bot commented Jan 8, 2025 •

edited

Loading

phipsae left a comment

All-Khwarizmi commented Jan 8, 2025

phipsae left a comment

phipsae Jan 9, 2025

All-Khwarizmi Jan 10, 2025

phipsae left a comment

phipsae Jan 9, 2025

All-Khwarizmi Jan 10, 2025

phipsae commented Jan 10, 2025

All-Khwarizmi commented Jan 10, 2025

phipsae commented Jan 10, 2025

All-Khwarizmi commented Jan 10, 2025 •

edited

Loading

phipsae commented Jan 10, 2025

feat(builder-page): Add personal builder profile page #15

feat(builder-page): Add personal builder profile page #15

Conversation

All-Khwarizmi commented Jan 7, 2025 • edited Loading

Description

Additional Information

Related Issues

vercel bot commented Jan 7, 2025

vercel bot commented Jan 8, 2025 • edited Loading

phipsae left a comment

Choose a reason for hiding this comment

All-Khwarizmi commented Jan 8, 2025

phipsae left a comment

Choose a reason for hiding this comment

phipsae Jan 9, 2025

Choose a reason for hiding this comment

All-Khwarizmi Jan 10, 2025

Choose a reason for hiding this comment

phipsae left a comment

Choose a reason for hiding this comment

phipsae Jan 9, 2025

Choose a reason for hiding this comment

All-Khwarizmi Jan 10, 2025

Choose a reason for hiding this comment

phipsae commented Jan 10, 2025

All-Khwarizmi commented Jan 10, 2025

phipsae commented Jan 10, 2025

All-Khwarizmi commented Jan 10, 2025 • edited Loading

phipsae commented Jan 10, 2025

All-Khwarizmi commented Jan 7, 2025 •

edited

Loading

vercel bot commented Jan 8, 2025 •

edited

Loading

All-Khwarizmi commented Jan 10, 2025 •

edited

Loading