Fetch upcoming events and recent news on home page #162

Merged
a258wang merged 13 commits from feat/upcoming-events-recent-news into main 2 years ago
Owner
  • Home page displays the 2 soonest upcoming events.
    • If there are no upcoming events, displays a short message.
    • If there are 1-2 upcoming events, displays the events.
    • If there are more than 2 upcoming events, displays the events as well as a link to the Events page. (though I haven't implemented the actual link yet)
  • Home page displays the most recent news.
    • If there is no news, displays a short message.
    • Might we want to display more than just 1 news item, and/or display news from the past X months?

Closes #149

- Home page displays the 2 soonest upcoming events. - If there are no upcoming events, displays a short message. - If there are 1-2 upcoming events, displays the events. - If there are more than 2 upcoming events, displays the events as well as a link to the Events page. (though I haven't implemented the actual link yet) - Home page displays the most recent news. - If there is no news, displays a short message. - Might we want to display more than just 1 news item, and/or display news from the past X months? Closes #149
a258wang added 3 commits 2 years ago
a3thakra reviewed 2 years ago
pages/index.tsx Outdated
export const getStaticProps: GetStaticProps<Props, Params> = async () => {
const events = await getUpcomingEvents();
const news = await getRecentNews();
return { props: { events, news } };
Collaborator

You should slice these arrays here, if you don't that will make the HTML pages gigantic, as all the events and news will be serialized in HTML.

You should slice these arrays here, if you don't that will make the HTML pages gigantic, as all the events and news will be serialized in HTML.
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
pages/index.tsx Outdated
type Params = ParsedUrlQuery;
export const getStaticProps: GetStaticProps<Props, Params> = async () => {
Collaborator

Just GetStaticProps<Props> is fine, you can omit Params here.

Just `GetStaticProps<Props>` is fine, you can omit Params here.
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
pages/index.tsx Outdated
];
interface Props {
events: Event[];
news: News[];
Collaborator

We only need the first news, so we should only pass 1 news item, instead of an array.

We only need the first news, so we should only pass 1 news item, instead of an array.
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
pages/index.tsx Outdated
<div className={styles.cardsBackground}>
<div className={styles.cards}>
{/* TODO: add links to past events and past news */}
{/* TODO: add links to past/upcoming events and past news */}
Collaborator

links:

  • upcoming events: /events
  • past events: /events/archive
  • past news: /news/archive

note that we will never have upcoming news.

links: - upcoming events: `/events` - past events: `/events/archive` - past news: `/news/archive` note that we will never have upcoming news.
a3thakra marked this conversation as resolved
Collaborator

LGTM after comments are addressed!

LGTM after comments are addressed!
a258wang added 1 commit 2 years ago
314272fe1e Clean up props and add links based on PR comments
a3thakra reviewed 2 years ago
lib/events.ts Outdated
const eventsInTerm = await getEventsByTerm(year, TERMS[term]);
return await Promise.all(
eventsInTerm.map(
async (slug) => await getEventBySlug(year, TERMS[term], slug)
Collaborator

no need to use async and await if the thing you're returning from your function is a promise.

so:

async (slug) => await getEventBySlug(year, TERMS[term], slug)

is the same as

(slug) => getEventBySlug(year, TERMS[term], slug)

no need to use async and await if the thing you're returning from your function is a promise. so: `async (slug) => await getEventBySlug(year, TERMS[term], slug)` is the same as `(slug) => getEventBySlug(year, TERMS[term], slug)`
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
pages/index.tsx Outdated
{ Content: AltTab, metadata: altTabEventMetadata },
];
interface Props {
numberOfEvents: number; // total number of upcoming events
Collaborator

this seems redundant, why can't we just use events.length?

this seems redundant, why can't we just use events.length?
Poster
Owner

Currently, we're displaying a link to the upcoming events page iff there are more than 2 upcoming events, however we are only passing up to 2 events in the events array (that is, 0 <= events.length <= 2).

Currently, we're displaying a link to the upcoming events page iff there are more than 2 upcoming events, however we are only passing up to 2 events in the events array (that is, 0 <= events.length <= 2).
Collaborator

oh, I see. In that case, you should pass in a boolean prop (eg moreUpcomingEvents) instead of numberOfEvents.

oh, I see. In that case, you should pass in a boolean prop (eg `moreUpcomingEvents`) instead of `numberOfEvents`.
a258wang marked this conversation as resolved
a3thakra reviewed 2 years ago
pages/index.tsx Outdated
</Link>
</p>
<hr className={styles.cardsDividingLine} />
{props.numberOfEvents == 0 ? (
Collaborator

Use ===. Only use == for null checks.

Use `===`. Only use `==` for null checks.
a3thakra marked this conversation as resolved
a258wang added 1 commit 2 years ago
fc3aec8667 Fix small nitpicks
a258wang added 6 commits 2 years ago
a258wang added 1 commit 2 years ago
bebfe00c5c Change numberOfEvents to moreEvents
a258wang added 1 commit 2 years ago
a3thakra approved these changes 2 years ago
a3thakra left a comment
Collaborator

🚢

🚢
a258wang merged commit 490ec7660b into main 2 years ago
a3thakra deleted branch feat/upcoming-events-recent-news 2 years ago

Reviewers

a3thakra approved these changes 2 years ago
continuous-integration/drone/push Build is passing
The pull request has been merged as 490ec7660b.
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#162
Loading…
There is no content yet.