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.

Sign up for GitHub

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

Jump to bottom

Feature: Officer Section #21

Merged
merged 19 commits into from
Nov 15, 2023
Merged

Feature: Officer Section #21

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c9ace03
Add basic officer card component
Brian-Magnuson Oct 27, 2023
b63b989
Fix lint issues
Brian-Magnuson Oct 27, 2023
a871c1a
Add multiple cards to officer section
Brian-Magnuson Oct 28, 2023
9995f16
Decreased social link sizes. Added officer names and position. Need t…
Phantom0110 Nov 2, 2023
7b0529b
Added images of officers. Need to add social links. Some pictures als…
Phantom0110 Nov 4, 2023
82488d2
Merge branch 'master' of https://github.com/ufssd/ufssd-website into …
Phantom0110 Nov 4, 2023
f24d6fe
Add styles to make officer portraits the same size
Brian-Magnuson Nov 5, 2023
a23f6cf
Change officer social links to use their real links
Brian-Magnuson Nov 5, 2023
60bafa6
Update size of cards to fit 3 columns in 800px content width
Brian-Magnuson Nov 5, 2023
04288a4
Change social links to open in new tab
Brian-Magnuson Nov 5, 2023
49c7921
Add web logo for officer cards
Brian-Magnuson Nov 5, 2023
1643417
Change officer names to be center aligned
Brian-Magnuson Nov 5, 2023
244647c
Delete eric-navar.webp
Brian-Magnuson Nov 11, 2023
850045f
Remove unnecessary comments from CSS files
Brian-Magnuson Nov 11, 2023
cc1800a
Remove alt prop from OfficerCard and data and export SocialLink
Brian-Magnuson Nov 11, 2023
9cdf600
Rename OfficerCardProperties to -Props and add eslint exception
Brian-Magnuson Nov 11, 2023
91ae909
Change officer heading to be center aligned wrt container
Brian-Magnuson Nov 11, 2023
e9372c3
Change .eslintrc.js to use LF line endings
Brian-Magnuson Nov 12, 2023
9b28c0b
Move officerCardData to separate officer-data.ts file
Brian-Magnuson Nov 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added public/eric-navar.webp
View file
Open in desktop
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
Binary file not shown.
Binary file added public/public/officers/Angel_Lopez.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added public/public/officers/CJ_Weir.jpg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added public/public/officers/Michael_Hayworth.jpg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added public/public/officers/Param_Gupta.jpg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added public/public/officers/Stephen_Coomes.jpg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added public/public/officers/Yonas_Bahre.jpg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions public/web_logo.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
48 changes: 48 additions & 0 deletions src/app/about/About.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,54 @@
text-align: center;
}

/* CSS Modules recommends camelCase
See: https://github.com/css-modules/css-modules#naming */
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
.imageContainer {
display: flex;
justify-content: center;
margin-top: 2rem;
}

.title {
margin-bottom: 5rem;
font-size: 3rem;
}

.description {
margin: 0 auto;
max-width: 1000px;
}

.image {
margin: auto;
}

.officers {
/* align */
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
margin-top: 50px;
height: 400px;
/* style */
text-align: left;
/* background: purple; */
/* content */
display: flex;
justify-content: center;
}

.container {
/* align */
height: 100%;
width: 100%;
/* max-width: 1000px; */
/* style */
/* background: blue; */
}

.cardContainer {
/* content */
display: flex;
flex-wrap: wrap;
justify-content: center;
gap: 1rem;

}
60 changes: 60 additions & 0 deletions src/app/about/OfficerCard.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
.officerCard {
/* align */
width: 250px;
max-width: 100%;
height: 320px;
/* style */
background-color: rgba(0, 0, 0, 0.4);
border-radius: 10px;
/* content */
padding: 1rem;
display: flex;
flex-direction: column;
justify-content: space-between;
align-items: center;
}

.portrait {
/* align */
height: 180px;
width: 180px;
/* style */
object-fit: cover;
border-radius: 5px;
}

.officerCard h3 {
/* style */
font-size: 1.3rem;
font-weight: 700;
text-align: center;
}

.officerCard h4 {
/* style */
font-size: 1.25rem;
font-weight: 700;
text-align: center;
}

.socialLinks {
/* content */
display: flex;
gap: 1rem;
}

/*
The website logo is slightly smaller than the other logos.
This scales it up a tiny bit.
This is where the logo comes from:
https://fonts.google.com/icons?selected=Material%20Symbols%20Outlined%3Alanguage%3AFILL%401%3Bwght%40400%3BGRAD%400%3Bopsz%4024
Copy link
Member

Choose a reason for hiding this comment

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

[Comment]: Good callout & use of comments. May be worth looking into later, but let's accept this limitation for now. I'll sync with you later and see if we should open a backlog ticket to dig into this more.

*/
.socialLinks img[data-name="Website"] {
/* style */
transform: scale(1.2);
}

.socialLinks a:hover {
/* style */
transform: scale(1.1);
}
56 changes: 56 additions & 0 deletions src/app/about/officer-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import Image from "next/image";
import styles from "./OfficerCard.module.css";

type SocialLink = {
WillBAnders marked this conversation as resolved.
Show resolved Hide resolved
name: string;
url: string;
icon: string;
};

export type OfficerCardProperties = {
WillBAnders marked this conversation as resolved.
Show resolved Hide resolved
position: string;
image: string;
alt: string;
Copy link
Member

Choose a reason for hiding this comment

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

[Comment/Suggestion]: It's unusual to see an alt text property for a generic component like this (especially given it's set to an empty string when used below). I would suggest computing something like Profile picture of ${name}. instead, I think that's good enough for our needs.

name: string;
socialLinksData: SocialLink[];
};

export default function OfficerCard({
position,
image,
alt,
name,
socialLinksData,
}: OfficerCardProperties) {
const socialLinks = socialLinksData.map((link) => (
<a
href={link.url}
key={link.name}
target="_blank"
rel="noopener noreferrer"
>
<Image
src={link.icon}
alt={link.name}
height={30}
width={30}
data-name={link.name}
/>
</a>
));

return (
<div className={styles.officerCard}>
<h3>{position}</h3>
<Image
className={styles.portrait}
src={image}
alt={alt}
height={180}
width={180}
/>
<h4>{name}</h4>
<div className={styles.socialLinks}>{socialLinks}</div>
</div>
);
}
126 changes: 119 additions & 7 deletions src/app/about/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,111 @@
import Image from "next/image";
import styles from "./About.module.css";

import OfficerCard, { OfficerCardProperties } from "./officer-card";

const officerCardData: OfficerCardProperties[] = [
Copy link
Member

Choose a reason for hiding this comment

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

[Comment]: This may be a good candidate to have in a separate file, that way it can just be referenced by this page. I'd expect this constant to get a lot messier once we start adding previous officers.

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 agree. I like to put data like this in separate files. In previous projects, I've stored data in .ts files in a separate data/ directory. We could do that too, or put data in the src/app/about/ directory. I'd like to know what you think (and if this should maybe be saved for a separate issue).

Copy link
Member

Choose a reason for hiding this comment

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

I'm generally inclined to always abstract by usage - in other words, a simple officers.ts file (or officer-data.ts, if preferred) in the about directory is probably the best option for now. I'm not particularly a fan of diving projects up into categories like /components, /forms, /types, etc. I think it forces more abstraction than is often needed (or conceptually present) and grouping by programmatic concern is less valuable than grouping by business concern.

{
position: "President",
image: "public/officers/Yonas_Bahre.jpg",
alt: "",
name: "Yonas Bahre",
socialLinksData: [
{
name: "LinkedIn",
url: "https://www.linkedin.com/in/yonasbahre/",
icon: "/linkedin_logo.png",
},
],
},
{
position: "Vice President",
image: "public/officers/Michael_Hayworth.jpg",
alt: "",
name: "Michael Hayworth",
socialLinksData: [
{
name: "LinkedIn",
url: "https://www.linkedin.com/in/michaeldhayworth/",
icon: "/linkedin_logo.png",
},
],
},
{
position: "Treasurer",
image: "public/officers/Stephen_Coomes.jpg",
alt: "",
name: "Stephen Coomes",
socialLinksData: [
{
name: "LinkedIn",
url: "https://www.linkedin.com/in/stephen-coomes-8a4885221/",
icon: "/linkedin_logo.png",
},
],
},
{
position: "Outreach Officer",
image: "public/officers/Angel_Lopez.png",
alt: "",
name: "Angel Lopez",
socialLinksData: [
{
name: "GitHub",
url: "https://github.com/angel1254mc",
icon: "/github_logo.svg",
},
{
name: "LinkedIn",
url: "https://www.linkedin.com/in/angel1254/",
icon: "/linkedin_logo.png",
},
{
name: "Website",
url: "https://www.angel1254.com/",
icon: "/web_logo.svg",
},
],
},
{
position: "Program Officer",
image: "public/officers/Param_Gupta.jpg",
alt: "",
name: "Param Gupta",
socialLinksData: [
{
name: "LinkedIn",
url: "https://www.linkedin.com/in/paramg/",
icon: "/linkedin_logo.png",
},
],
},
{
position: "Involvement Officer",
image: "public/officers/CJ_Weir.jpg",
alt: "",
name: "CJ Weir",
socialLinksData: [
{
name: "LinkedIn",
url: "https://www.linkedin.com/in/cj-weir/",
icon: "/linkedin_logo.png",
},
],
},
];

export default function About() {
const officerCards = officerCardData.map((cardData) => (
<OfficerCard
position={cardData.position}
image={cardData.image}
alt={cardData.alt}
name={cardData.name}
socialLinksData={cardData.socialLinksData}
Copy link
Member

Choose a reason for hiding this comment

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

[Suggestion]: You can use {...cardData} to spread props into the component, save a bit of work/duplication here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't change this due to eslint react/jsx-props-no-spreading. I've decided to keep it as is rather than disable the rule. Let me know if you insist this be changed.

Copy link
Member

Choose a reason for hiding this comment

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

That's fine, no strong opinion here. I'd guess that the use of TypeScript also helps catch many of the common bugs here.

key={cardData.name}
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
/>
));

return (
<main className={styles.content}>
<h1>About</h1>
Expand All @@ -16,13 +120,21 @@ export default function About() {
careers in software development.
</p>
</div>
<Image
className={styles.image}
src="fall_2022_planning_meeting.jpg"
alt="SSD members (about 30) at our Fall 2022 planning meeting in CISE."
height={600}
width={600}
/>
<div className={styles.imageContainer}>
<Image
className={styles.image}
src="fall_2022_planning_meeting.jpg"
alt="SSD members (about 30) at our Fall 2022 planning meeting in CISE."
height={600}
width={600}
/>
</div>
<section className={styles.officers}>
<div className={styles.container}>
Copy link
Member

Choose a reason for hiding this comment

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

[Comment]: Will probably have to abstract this down the line when we add past officers. Don't change anything here; just making note and musing for the future.

<h2>Officers</h2>
Brian-Magnuson marked this conversation as resolved.
Show resolved Hide resolved
<div className={styles.cardContainer}>{officerCards}</div>
</div>
</section>
</main>
);
}
Loading