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

Feat/page profile #15

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from
Draft

Feat/page profile #15

wants to merge 4 commits into from

Conversation

manvinderjit
Copy link
Contributor

Summary

Add a profile page to show the profile for an individual volunteer.

Additional information

The profile page right now uses a profile Id 'pid' URL parameter to fetch the profile id. Let me know if this has to be updated.

Rename profile/* to profiles/* as it shows multiple files.
Update <a> in index menu.
Add Profile page for showing volunteer's profile details.
Add CSS styles for Profile page.
Add SVG icons as React components.
Add links for profiles on index page.
@manvinderjit manvinderjit linked an issue Jan 25, 2025 that may be closed by this pull request
@manvinderjit manvinderjit self-assigned this Jan 25, 2025
@manvinderjit
Copy link
Contributor Author

manvinderjit commented Jan 25, 2025

@madcampos This page displays profile for a single individual. @kbventures you have already written a ProfileList component for rendering all profiles as a list. Is it sufficient for the use case and are you planning to work on it further?

setIsErrorProfile(true);
}
const jsonDataProfile = await responseProfile.json();
// REMOVE: Filtering won't be required with API, remove filtering when integrating with backend
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: Use common pattern words like "TODO" or "FIXME".
Some extensions also help to highlight different keywords with different colors. Here is the one I use: https://marketplace.visualstudio.com/items?itemName=aaron-bond.better-comments

Copy link
Contributor

Choose a reason for hiding this comment

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

@manvinderjit we should definitely reuse that if possible. I'm not currently working on it but we shall surely expand on this in terms of functionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

@manvinderjit for front end work we should try to include a picture so we can get a quick peak without running the code(even though we will run it). Eventually when we have mobile version both images will be expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PreviewInBrowser
@kbventures As each commit creates and updates a preview branch in CloudFlare, we can preview it directly in the browser instead of running locally. Refer to the image for instructions. Apologies for it being a crude design.
If we still need images, I can upload them.

@madcampos
Copy link
Collaborator

madcampos commented Jan 29, 2025

@manvinderjit

This page displays profile for a single individual.

Not sure if it was a question, but yes, it is correct. And the layout look nice 😉

you have already written a ProfileList component for rendering all profiles as a list. Is it sufficient for the use case and are you planning to work on it further?

I think for now this is sufficient, we can reuse the TeamMemberCard component and just create a list of that, like the /team page. Then we can get back to it later once we have the API for actually querying the data.

@madcampos
Copy link
Collaborator

Please test the page for mobile using the dev tools to check for potential issues with content breaking.

localhost_3000_pages_profile__pid=2

Copy link
Contributor

@kbventures kbventures left a comment

Choose a reason for hiding this comment

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

Great work @manvinderjit

Small ask. For future branches lets create a draft sooner during the development project so we are able to coordinate more efficiently have have a pulse on progression.

@kbventures
Copy link
Contributor

@manvinderjit @madcampos

Should we introduce PascalCase for the components?

https://www.sufle.io/blog/naming-conventions-in-react

@kbventures
Copy link
Contributor

@manvinderjit nit picking but eventually we probably want Inner Components (Reusable UI Elements) for Team & Profile

@manvinderjit
Copy link
Contributor Author

manvinderjit commented Jan 29, 2025

@manvinderjit @madcampos

Should we introduce PascalCase for the components?

https://www.sufle.io/blog/naming-conventions-in-react

I am already following PascalCase for components, unless I might have missed it somewhere. Functions and variables are camel case. I can update the contribution guides if required for the same.

For the pages, I am using small case because that is how @madcampos initialized them. Not sure on the nomenclature for them.

@manvinderjit
Copy link
Contributor Author

manvinderjit commented Jan 29, 2025

@manvinderjit nit picking but eventually we probably want Inner Components (Reusable UI Elements) for Team & Profile

@kbventures Kindly elaborate on the elements parts. Do you mean the card for each team member and profile, if yes then these are already reusable components -> TeamMemberCard & ProfileCard.

Otherwise, lemme know.

@manvinderjit
Copy link
Contributor Author

Please test the page for mobile using the dev tools to check for potential issues with content breaking.

localhost_3000_pages_profile__pid=2

OOPS! I might have not checked below 600px width. Will rectify it immediately!

@manvinderjit
Copy link
Contributor Author

manvinderjit commented Jan 29, 2025

@manvinderjit

This page displays profile for a single individual.

Not sure if it was a question, but yes, it is correct. And the layout look nice 😉

you have already written a ProfileList component for rendering all profiles as a list. Is it sufficient for the use case and are you planning to work on it further?

I think for now this is sufficient, we can reuse the TeamMemberCard component and just create a list of that, like the /team page. Then we can get back to it later once we have the API for actually querying the data.

@madcampos @kbventures Apologies for the lack of clarity. What I meant to ask was that @kbventures originally created the as a reference point. I was confirming if I was expected to build upon that component or will Ken continue to work on it. I guess I will leave it for Ken.

Fix mobile content breaking under 600px.
Update message pattern in ProfileCard component.
@kbventures
Copy link
Contributor

It was simply an mvp to compare deployment solutions.

Copy link

cloudflare-workers-and-pages bot commented Jan 29, 2025

Deploying volunteer with  Cloudflare Pages  Cloudflare Pages

Latest commit: 38cc5c2
Status: ✅  Deploy successful!
Preview URL: https://2771267c.volunteer-ekr.pages.dev
Branch Preview URL: https://feat-page-profile.volunteer-ekr.pages.dev

View logs

@manvinderjit
Copy link
Contributor Author

Great work @manvinderjit

Small ask. For future branches lets create a draft sooner during the development project so we are able to coordinate more efficiently have have a pulse on progression.

@kbventures Will do.

@kbventures
Copy link
Contributor

kbventures commented Jan 29, 2025

@manvinderjit inner components and camelcase are fine. Ignore that.

@madcampos
Copy link
Collaborator

@kbventures

Should we introduce PascalCase for the components?

Yes, react component names should be in PascalCase, always, that helps to differentiate from native html elements and web components when referencing them in jsx.

Fun fact: this is also a recommendation in vue for the same reason, it helps differentiate framework components from other things.

@manvinderjit @madcampos
Should we introduce PascalCase for the components?
https://www.sufle.io/blog/naming-conventions-in-react

I am already following PascalCase for components, unless I might have missed it somewhere. Functions and variables are camel case. I can update the contribution guides if required for the same.

For the pages, I am using small case because that is how @madcampos initialized them. Not sure on the nomenclature for them.

Ops... That was an oversight on my side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Create "profile" page
3 participants