www
/
www-new
12
2
Fork 3
Code Issues 56 Pull Requests 29 Projects Releases Wiki Activity
Labels Milestones

Redesign and update 'Meet the Team' data #452

Merged
j285he merged 17 commits from j285he-meet-the-team-s22 into main 9 months ago
Conversation 4 Commits 17 Files Changed 63
j285he commented 10 months ago
Collaborator

Closes #450. Closes #436.

Closes #450. Closes #436.
j285he added 4 commits 10 months ago
continuous-integration/drone/push Build is passing
Details
69451c36c7 Change team names
continuous-integration/drone/push Build is passing
Details
0f5707edd3 Delete old members images
j285he added 1 commit 10 months ago
continuous-integration/drone/push Build is passing
Details
5d2b718b23 Fix teammembercard
j285he changed title from Redesign and update 'Meet the Team' data to WIP: Redesign and update 'Meet the Team' data 10 months ago
j285he added 1 commit 10 months ago
continuous-integration/drone/push Build is passing
Details
425b05c6a6 Do not pass in metadata as props
j285he added 1 commit 10 months ago
continuous-integration/drone/push Build is passing
Details
789f57c0b3 Delete old images
j285he changed title from WIP: Redesign and update 'Meet the Team' data to Redesign and update 'Meet the Team' data 10 months ago
j285he changed title from Redesign and update 'Meet the Team' data to WIP: Redesign and update 'Meet the Team' data 10 months ago
j285he added 2 commits 10 months ago
continuous-integration/drone/push Build is failing
Details
ad224d3eed Add new members
j285he changed title from WIP: Redesign and update 'Meet the Team' data to Redesign and update 'Meet the Team' data 10 months ago
j285he commented 10 months ago
Poster
Collaborator
There is no content yet.
1.png
174 KiB
2.png
259 KiB
3.png
177 KiB
4.png
133 KiB
5.png
206 KiB
j285he commented 10 months ago
Poster
Collaborator
There is no content yet.
6.png
158 KiB
j285he requested review from n3parikh 10 months ago
j285he requested review from a258wang 10 months ago
a3thakra reviewed 10 months ago
components/TeamMember.tsx Outdated
import React from "react";
import { Metadata } from "@/lib/team";
a3thakra commented 10 months ago
Collaborator

Components should be isolated and self contained. They should not be importing things from outside the /components folder.

Components should be isolated and self contained. They should not be importing things from outside the /components folder.
a3thakra commented 10 months ago
Collaborator

This is why we duplicated an interface here to TeamMemberProps. This makes the component independent of whatever the Metadata is. If in the future the metadata changes, the props of this component would also change (or the other way around) - which we may not want.

This is why we duplicated an interface here to `TeamMemberProps`. This makes the component independent of whatever the Metadata is. If in the future the metadata changes, the props of this component would also change (or the other way around) - which we may not want.
👍 1
j285he marked this conversation as resolved
a3thakra reviewed 10 months ago
pages/about/team.tsx Outdated
photography = sortTeam(photography);
website = sortTeam(website);
systems = sortTeam(systems);
terminal = sortTeam(terminal);
a3thakra commented 10 months ago
Collaborator

This is getting a little out of hand. With these many teams, can you move them into a 2d array?

This is getting a little out of hand. With these many teams, can you move them into a 2d array?
a3thakra reviewed 10 months ago
pages/about/team.tsx Outdated
return {
props: {
execs,
coordinators,
a3thakra commented 10 months ago
Collaborator

this way you can make the props: execs (a 1d array) and teams: (a 2d array)

this way you can make the props: execs (a 1d array) and teams: (a 2d array)
j285he commented 10 months ago
Poster
Collaborator

What about the teams array on line 64? Won't we need to change each members property to be (for example) members: teams[0]?

What about the `teams` array on line 64? Won't we need to change each `members` property to be (for example) `members: teams[0]`?
j285he commented 10 months ago
Poster
Collaborator

I was also thinking perhaps teams could go in a contents .json file but let me know what you think is best

I was also thinking perhaps `teams` could go in a contents .json file but let me know what you think is best
a3thakra commented 10 months ago
Collaborator

Yeah if we move it into an array here, then we won't need the array on line 64, and a lot of hardcoding would go away.

Perhaps putting all the teams in one place would be a better idea.

Eg - instead of /contents/team/team-name.json we can put all the json data in an array in /contents/team/non-execs.json which could follow the following format:

[
  {
    id: "website",
    name: "Web Committee",
    members: [
      {
        ...
      },
      ...
    ]
  },
  ...
]

But don't block this PR on this change. Let's get the content updated first and you can file an issue to get back to this :)

Yeah if we move it into an array here, then we won't need the array on line 64, and a lot of hardcoding would go away. Perhaps putting all the teams in one place would be a better idea. Eg - instead of `/contents/team/team-name.json` we can put all the json data in an array in `/contents/team/non-execs.json` which could follow the following format: ``` [ { id: "website", name: "Web Committee", members: [ { ... }, ... ] }, ... ] ``` But don't block this PR on this change. Let's get the content updated first and you can file an issue to get back to this :)
a3thakra reviewed 10 months ago
pages/about/team.tsx Outdated
const teamMembers: Metadata[] = [];
const teamOthers: Metadata[] = [];
for (const member of team) {
if (!Object.prototype.hasOwnProperty.call(member, "role")) {
a3thakra commented 10 months ago
Collaborator

if (member.hasOwnProperty("role")) {

`if (member.hasOwnProperty("role")) {`
a3thakra commented 10 months ago
Collaborator

or if (member["role"] != null)

or `if (member["role"] != null)`
j285he marked this conversation as resolved
a3thakra reviewed 10 months ago
pages/about/team.tsx Outdated
}
teamLeads.sort((a, b) => a.name.localeCompare(b.name));
teamMembers.sort((a, b) => a.name.localeCompare(b.name));
teamOthers.sort((a, b) => a.name.localeCompare(b.name));
a3thakra commented 10 months ago
Collaborator

nit: Instead of using that for loop, I would suggest using a more declarative approach.

const leads = team.filter(({role}) => role === "Team Lead").sort(memberComparer);
const general = team.filter(({role}) => role == null || role === "").sort(memberComparer);
const others = team.filter(({role}) => role != null && role !== "" && role !== "Team Lead").sort(memberComparer);

return [...leads, ...general, ...others]

function memberComparer(a: Metadata, b: Metadata) {
	return a.name.localeCompare(b.name)
}
nit: Instead of using that for loop, I would suggest using a more declarative approach. ```typescript const leads = team.filter(({role}) => role === "Team Lead").sort(memberComparer); const general = team.filter(({role}) => role == null || role === "").sort(memberComparer); const others = team.filter(({role}) => role != null && role !== "" && role !== "Team Lead").sort(memberComparer); return [...leads, ...general, ...others] function memberComparer(a: Metadata, b: Metadata) { return a.name.localeCompare(b.name) } ```
j285he marked this conversation as resolved
a3thakra commented 10 months ago
Collaborator

yay all the old pictures are gone!

Left comments for code nits, but looks good overall :) Thanks @j285he!!

yay all the old pictures are gone! Left comments for code nits, but looks good overall :) Thanks @j285he!!
a258wang reviewed 10 months ago
content/team/systems-committee.json Outdated
"name": "Andrew Wang"
},
{
"name": "Anthony Brennan",
a258wang commented 10 months ago
Owner

@j285he I suspect we want to keep all these alumni-labelled peeps, but let's let @r389li confirm that. Sorry for not being more clear with expectations 😅

@j285he I suspect we want to keep all these alumni-labelled peeps, but let's let @r389li confirm that. Sorry for not being more clear with expectations 😅
j285he marked this conversation as resolved
r389li commented 10 months ago
Owner

@a258wang Correct, we should keep them. Syscom membership doesn't usually change. This term, however, Max graduated, so he is now Alumni, and I misidentified Edwin as alumni when he's in fact a current student. Also if someone could link syscom and termcom in my bio to the relevant sections on the page that would be appreciated!

@a258wang Correct, we should keep them. Syscom membership doesn't usually change. This term, however, Max graduated, so he is now Alumni, and I misidentified Edwin as alumni when he's in fact a current student. Also if someone could link syscom and termcom in my bio to the relevant sections on the page that would be appreciated!
j285he added 3 commits 10 months ago
continuous-integration/drone/push Build is failing
Details
cad441fab6 Fix TeamMember
j285he added 1 commit 10 months ago
continuous-integration/drone/push Build is passing
Details
ddf6ac4b9b use declarative apporach
j285he added 2 commits 9 months ago
continuous-integration/drone/push Build is passing
Details
e8f7a8e31c Change Raymond's blurb
a258wang reviewed 9 months ago
pages/about/team.tsx
.filter(({ role }) => role === "Team Lead")
.sort(memberComparer);
const general = team
.filter(({ role }) => role == null || role === "")
a258wang commented 9 months ago
Owner

Would role == null || role === "" be equivalent to !role here? (and also a couple lines below)

Would `role == null || role === ""` be equivalent to `!role` here? (and also a couple lines below)
j285he marked this conversation as resolved
a258wang approved these changes 9 months ago
a258wang left a comment
Owner

Just one tiny (non-blocking) nitpick, but otherwise this looks great! 🚢

Once this is merged, could you please create a ticket to clean up how we store/process the team data, as discussed in the comments of this PR? Thanks!

Just one tiny (non-blocking) nitpick, but otherwise this looks great! 🚢 Once this is merged, could you please create a ticket to clean up how we store/process the team data, as discussed in the comments of this PR? Thanks!
j285he added 2 commits 9 months ago
continuous-integration/drone/push Build is passing
Details
65eaaa12f0 Change !role
j285he merged commit e1af564621 into main 9 months ago
j285he deleted branch j285he-meet-the-team-s22 9 months ago

Reviewers

n3parikh was requested for review 10 months ago
a258wang approved these changes 9 months ago
continuous-integration/drone/push Build is passing
Details
The pull request has been merged as e1af564621.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
n3parikh
a258wang
Labels
Apply labels
Clear labels
a11y Backlog Blocked Bug Content Dependencies Design Feature Request Good First Issue In Progress Performance Priority - High Priority - Low Priority - Medium Untriaged
No Label a11y Backlog Blocked Bug Content Dependencies Design Feature Request Good First Issue In Progress Performance Priority - High Priority - Low Priority - Medium Untriaged
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#452
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'j285he-meet-the-team-s22'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes