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> Organization Page #8

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

danielpeterkim
Copy link

Organization page using dummy data, and a non-functional google maps embed.

To access go to route organization/:id
The id can be anything currently as they all go to the same place.

Organization page using dummy data, and a google maps embed. Does not contain image support
@danielpeterkim
Copy link
Author

Nvm, it was just a unused package. My b.

@danielpeterkim danielpeterkim reopened this Feb 5, 2025
Updated for Prettier Formatting for Build
@miguel-merlin
Copy link
Member

@danielpeterkim For future pull requests, can you include screenshots of the changes you made

Copy link
Member

@miguel-merlin miguel-merlin left a comment

Choose a reason for hiding this comment

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

Good work! I left some small comments!

import { Card } from 'antd';

// The Left Column Top Card
const OrganizationDisplayCard: React.FC = () => {
Copy link
Member

Choose a reason for hiding this comment

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

I think React.FC is going to get deprecated at some point. Just do const OrganizationDisplayCard = () => {}

<Card
title={<span style={{ fontSize: '2rem' }}>A Wider Circle</span>}
bordered
style={{
Copy link
Member

Choose a reason for hiding this comment

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

We usually prefer using tailwind for styling. Can you change the style prop for className= ...

boxShadow: '0 4px 8px rgba(0,0,0,0.2)',
}}
>
<div style={{ fontSize: '1rem', color: '#333' }}>
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here about tailwind

@@ -0,0 +1,47 @@
import React from 'react';
import { Card } from 'antd';
Copy link
Member

Choose a reason for hiding this comment

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

Could you maybe add an interface prop to this component?

interface OrganizationDisplayCard {
   organization: Organization
}

You will need to import the type


// Example "Services" data array.
// In a real app, you'll fetch this from a backend and store it in state or props.
const servicesData = [
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this move this array to data/services.ts and import it from there. It will be cleaner

Copy link
Author

Choose a reason for hiding this comment

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

that makes sense, will do

<p>
<strong>Address:</strong> {address}
</p>
{/* Example "chips" for categories */}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need these comments

};

// Render a "Location" item
const LocationCard: React.FC<{
Copy link
Member

Choose a reason for hiding this comment

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

Can you add this card in another file? LocationCard.tsx

<>
<Navbar />
{/* PLACE HOLDER SEARCH BAR*/}
{/* TO DO: Add search bar that links back to "Organizations" page with search query */}
Copy link
Member

Choose a reason for hiding this comment

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

Is the todo going to be addressed in another PR?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I didn't really understand the functions in the Navbar of the Organization page at the time, but I would like to revisit it in a new PR.

<Content style={{ margin: '2rem' }}>
<div style={{ display: 'flex', gap: '2rem', flexWrap: 'wrap' }}>
{/* Left Column: Organization Display Card */}
<div style={{ flex: '1 1 400px', minWidth: '350px' }}>
Copy link
Member

Choose a reason for hiding this comment

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

tailwind

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, will do. Saw the use of style for the drop shadows and totally forgot about tailwind. :P

</div>
</Card>
{/* Google Map Embed */}
<iframe
Copy link
Member

Choose a reason for hiding this comment

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

We have a component? Why not use it here?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, map was empty at that time but definitely could have just put it there, whoopsy.

Addressed and changed various styling inconsistencies, switching all CSS to tailwind. Created Seperate component file for location card, and moved dummy data to Service Data,
Created life-like dummy data, allowing me to change all the pages to take in actual data objects like Services and Organizations.
@danielpeterkim
Copy link
Author

image

Switched all Organization Page styling to Tailwind and Added Map Embed

image

Created a DummyData.tsx, which has Organization, Service, and Location Objects (use responsibly as some field were removed for convenience)

image
Created a OrganizationLayoutServiceCard.tsx for the Service cards in the Organization Page (they display more information than ServiceCard.tsx)

image

OrganizationTabs.tsx functionally take in Service and Location Data to create tabs, although the LocationCard input data is preprocessed.

Various data types were inputted incorrectly, I fixed them
@danielpeterkim
Copy link
Author

I can't seem to find why it isn't building. MapProps isn't used, while Maps itself doesn't take in any arguments? Not sure, definite blocker

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.

2 participants