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

Issue 7 - Create builders list page #29

Merged
merged 14 commits into from
Sep 23, 2024

Conversation

danitome24
Copy link
Contributor

@danitome24 danitome24 commented Sep 14, 2024

Description

Created a page to display a list of builders and mentors. The page features:

  • A section for mentors with their photos, names, and links to their personal pages.

  • A section for builders with similar details.

  • A dynamic counter that shows how many builders have checked in out of the total.

You can see a first template:

image

Additional Information

Related Issues

Closes #7

Your ENS/address: 0xaa4C60b784E2b3E485035399bF1b1aBDeD66A60f

Copy link

vercel bot commented Sep 14, 2024

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

Name Status Preview Comments Updated (UTC)
batch9-buidlguidl-com-nextjs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 20, 2024 9:20pm

@danitome24
Copy link
Contributor Author

danitome24 commented Sep 14, 2024

TODO:

  • Split into smaller components.
  • Handle loading events message (display some message that is loading data).
  • What we do with randomImages?

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.

Great job! Very clean architecture. I left some suggestions about the pictures - lmk what you think!

Comment on lines +9 to +21
const getRandomGender = () => {
return Math.random() < 0.5 ? "men" : "women";
};

const getRandomUserImage = () => {
const randomInt = Math.floor(Math.random() * 50);
const randomGender = getRandomGender();

return `https://randomuser.me/api/portraits/${randomGender}/${randomInt}.jpg`;
};

const getBuilderFromEvent = (event: any): Builder => ({
image: getRandomUserImage(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder what else we can do for profile pictures 🤔

Your design does a great job emphasizing the rounded portraits, so it seems silly to have random placeholders in the final version.

  • Is it possible to map builders to their GitHub profiles and fetch those avatars, just like you do for mentors?
  • If not, there are also BlockieAvatars (SE-2 docs) you can modify.
  • Here's where they are used in the BuidlGuidl app!
Screenshot 2024-09-19 at 2 52 20 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried to obtain the GitHubs of the builders automatically, but I haven't found a way. For this, I suppose we would need to manually keep a record of all users and their GitHubs. I have therefore decided to use BlockieAvatars for the image.

Maybe somebody can help with this and find a way to get github profiles automatically.

Copy link
Contributor

Choose a reason for hiding this comment

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

we could use ENS avatars. let me try something out...

Comment on lines 26 to 45
const mentors: Mentor[] = [
{
name: "Eda",
image: "https://avatars.githubusercontent.com/u/22100698?v=4",
link: "https://github.com/edakturk14",
checkedIn: false,
},
{
name: "Derrek",
image: "https://avatars.githubusercontent.com/u/80121818?v=4",
link: "https://github.com/derrekcoleman",
checkedIn: false,
},
{
name: "Philip",
image: "https://avatars.githubusercontent.com/u/90064316?v=4",
link: "https://github.com/phipsae",
checkedIn: false,
},
];
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really appreciate your inclusion of the mentors :)))

Comment on lines 1 to 9
export type Builder = {
image: string;
link: string;
checkedIn: boolean;
};

export type Mentor = Builder & {
name: string;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very clean

@ishtails
Copy link
Contributor

ishtails commented Sep 20, 2024

@danitome24 really nice work! i have some small changes that i'd like to contribute to this PR. not sure how do i push my commit in here tho.. do i need to fork your fork and push to that? or something else...

here's the screenshot of what i'm thinking:

  • ens avatar is displayed whenever it exists
  • user address is displayed below the avatar for identification
  • filtered duplicate builder entries before rendering

screenshot:
Screenshot (433)

@derrekcoleman
Copy link
Collaborator

@ishtails you've got the right idea. Check out this discussion about how to manage multiple “remotes” for your local version of the repo. The goal is to make a PR to Dani’s fork of the repo (rather than to the main repo we’re having this discussion in).

@ishtails
Copy link
Contributor

@ishtails you've got the right idea. Check out this discussion about how to manage multiple “remotes” for your local version of the repo. The goal is to make a PR to Dani’s fork of the repo (rather than to the main repo we’re having this discussion in).

Tried forking his fork, for some reason i am not able to do that.. so i am thinking to just open a new PR for making the changes i proposed once this is merged (anyway i wanted to redesign the UI like the new home page layout). please proceed with the merge as usual. i'll pick it up once this PR is closed.

@derrekcoleman
Copy link
Collaborator

Tried forking his fork, for some reason i am not able to do that.. so i am thinking to just open a new PR for making the changes i proposed once this is merged (anyway i wanted to redesign the UI like the new home page layout). please proceed with the merge as usual. i'll pick it up once this PR is closed.

Not so fast @ishtails - you can do this. Your goal is to make a PR to his repo, specifically to his 7-list-members-of-batch branch. You don't need to fork his repo if you already have this repo locally. Instead, you need to change your remote to his repo's URL, then pull/push there.

Resources to learn about changing your remote, which controls where your git push goes.

Definitely worth learning because (a) you will encounter this again in the future and (b) even though the learning make take a moment, the actual fix is only a couple git commands.

ishtails and others added 2 commits September 21, 2024 02:42
- ens avatar is displayed whenever it exists
- user address is displayed below the avatar for identification
- filtered duplicate builder entries before rendering
@ishtails
Copy link
Contributor

Tried forking his fork, for some reason i am not able to do that.. so i am thinking to just open a new PR for making the changes i proposed once this is merged (anyway i wanted to redesign the UI like the new home page layout). please proceed with the merge as usual. i'll pick it up once this PR is closed.

Not so fast @ishtails - you can do this. Your goal is to make a PR to his repo, specifically to his 7-list-members-of-batch branch. You don't need to fork his repo if you already have this repo locally. Instead, you need to change your remote to his repo's URL, then pull/push there.

Resources to learn about changing your remote, which controls where your git push goes.

Definitely worth learning because (a) you will encounter this again in the future and (b) even though the learning make take a moment, the actual fix is only a couple git commands.

haha you were right, i was just running away xD.. got it working with your and @danitome24's help. thankyou for teaching me something new :))

@derrekcoleman
Copy link
Collaborator

I’m proud of your persistence! Thanks for the help, @danitome24 🤝

@derrekcoleman derrekcoleman merged commit bd180d7 into BuidlGuidl:main Sep 23, 2024
3 checks passed
@derrekcoleman derrekcoleman linked an issue Sep 25, 2024 that may be closed by this pull request
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. Tweak the main page 7. List the members of the batch (read BatchRegistry contract)
3 participants