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

Add officer card data and semester data #35

Merged
merged 10 commits into from
Dec 13, 2023
Prev Previous commit
Next Next commit
Fix undefined errors on officer cards
  • Loading branch information
Brian-Magnuson committed Nov 27, 2023
commit 97ef7dcaaa71f4f076b14323e8adc5d8c680013a
2 changes: 1 addition & 1 deletion src/app/about/officer-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export const officerData: { [key: string]: OfficerCardProps } = {
};

export const positionData: {
[key: string]: { [key: string]: OfficerCardProps };
[key: string]: { [key: string]: OfficerCardProps | undefined };
} = {
"Fall 2023": {
President: officerData.yonas_bahre,
Expand Down
52 changes: 30 additions & 22 deletions src/app/about/page.tsx
Original file line number Diff line number Diff line change
@@ -1,39 +1,46 @@
import React from "react";
import Image from "next/image";
import styles from "./About.module.css";

import OfficerCard from "./officer-card";
import officerCardData, { officerData, positionData } from "./officer-data";

export default function About() {
const officerCards = officerCardData.map((cardData) => (
Copy link
Member

Choose a reason for hiding this comment

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

[Nit]: Development comments like this should be cleaned up before merging under most situations.

<OfficerCard
key={cardData.position}
position={cardData.position}
image={cardData.image}
name={cardData.name}
socialLinksData={cardData.socialLinksData}
/>
));
// const officerCards = officerCardData.map((cardData) => (
// <OfficerCard
// key={cardData.position}
// position={cardData.position}
// image={cardData.image}
// name={cardData.name}
// socialLinksData={cardData.socialLinksData}
// />
// ));

const officerCardSection = Object.entries(positionData).map(
([semester, officers]) => {
const officerCards = Object.entries(officers).map(
([position, officer]) => (
<OfficerCard
key={`${semester} ${position}`}
position={position}
image={officer.image}
name={officer.name}
socialLinksData={officer.socialLinksData}
/>
),
([position, officer]) => {
if (officer === undefined) {
Copy link
Member

Choose a reason for hiding this comment

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

[Please Change]: From our discussion the handling here has been changed and this check is no longer needed.

return <div key={`${semester} ${position}`} />;
}

return (
<OfficerCard
key={`${semester} ${position}`}
Copy link
Member

Choose a reason for hiding this comment

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

[Please Change]: We can use just position as the key here because it's unique within each semester. In other words; we don't need to ensure the keys are unique across both loops; just the current one.

position={position}
image={officer.image}
name={officer.name}
socialLinksData={officer.socialLinksData}
/>
);
},
);

return (
<>
<React.Fragment key={semester}>
Copy link
Member

Choose a reason for hiding this comment

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

[Comment]: Great use of React.Fragment here - definitely how I would have gone about this to avoid some unnecessary div wrapping.

<h2>{semester}</h2>
{officerCards}
</>
<div className={styles.cardContainer}>{officerCards}</div>
</React.Fragment>
);
},
);
Expand Down Expand Up @@ -64,7 +71,8 @@ export default function About() {
<section className={styles.officers}>
<div className={styles.container}>
<h2>Officers</h2>
<div className={styles.cardContainer}>{officerCards}</div>
{/* <div className={styles.cardContainer}>{officerCards}</div> */}
Copy link
Member

Choose a reason for hiding this comment

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

[Nit]: Can be cleaned up as above.

{officerCardSection}
</div>
</section>
</main>
Expand Down
Loading