www
/
www-new
12
3
Fork 5
Code Issues 58 Pull Requests 42 Projects Releases Wiki Activity

Fetch upcoming events and recent news on home page #162

Merged
a258wang merged 13 commits from feat/upcoming-events-recent-news into main 2021-08-23 14:56:12 -04:00
Conversation 1 Commits 13 Files Changed 4 +136 -30
a258wang commented 2021-08-19 01:53:45 -04:00
Owner
Copy Link
  • 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 2021-08-19 01:53:46 -04:00
continuous-integration/drone/push Build is passing Details
80890eef2a Add getRecentEvents()
a3thakra reviewed 2021-08-22 05:10:09 -04:00
pages/index.tsx Outdated
@ -95,0 +114,4 @@
export const getStaticProps: GetStaticProps<Props, Params> = async () => {
const events = await getUpcomingEvents();
const news = await getRecentNews();
return { props: { events, news } };
a3thakra commented 2021-08-22 05:10:09 -04:00
Owner
Copy Link

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 2021-08-22 05:10:35 -04:00
pages/index.tsx Outdated
@ -95,0 +111,4 @@
type Params = ParsedUrlQuery;
export const getStaticProps: GetStaticProps<Props, Params> = async () => {
a3thakra commented 2021-08-22 05:10:35 -04:00
Owner
Copy Link

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 2021-08-22 05:11:40 -04:00
pages/index.tsx Outdated
@ -29,1 +19,3 @@
];
interface Props {
events: Event[];
news: News[];
a3thakra commented 2021-08-22 05:11:40 -04:00
Owner
Copy Link

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 2021-08-22 05:13:05 -04:00
pages/index.tsx Outdated
@ -55,3 +49,3 @@
<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 */}
a3thakra commented 2021-08-22 05:13:05 -04:00
Owner
Copy Link

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
a3thakra commented 2021-08-22 05:15:48 -04:00
Owner
Copy Link

LGTM after comments are addressed!

LGTM after comments are addressed!
a258wang added 1 commit 2021-08-22 17:18:00 -04:00
continuous-integration/drone/push Build is passing Details
314272fe1e Clean up props and add links based on PR comments
a3thakra reviewed 2021-08-22 21:03:00 -04:00
lib/events.ts Outdated
@ -65,0 +80,4 @@
const eventsInTerm = await getEventsByTerm(year, TERMS[term]);
return await Promise.all(
eventsInTerm.map(
async (slug) => await getEventBySlug(year, TERMS[term], slug)
a3thakra commented 2021-08-22 21:03:00 -04:00
Owner
Copy Link

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 2021-08-22 21:03:49 -04:00
pages/index.tsx Outdated
@ -28,2 +18,2 @@
{ Content: AltTab, metadata: altTabEventMetadata },
];
interface Props {
numberOfEvents: number; // total number of upcoming events
a3thakra commented 2021-08-22 21:03:49 -04:00
Owner
Copy Link

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

this seems redundant, why can't we just use events.length?
a258wang commented 2021-08-23 12:08:29 -04:00
Author
Owner
Copy Link

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).
a3thakra commented 2021-08-23 13:59:48 -04:00
Owner
Copy Link

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 2021-08-22 21:04:32 -04:00
pages/index.tsx Outdated
@ -61,1 +57,4 @@
</Link>
</p>
<hr className={styles.cardsDividingLine} />
{props.numberOfEvents == 0 ? (
a3thakra commented 2021-08-22 21:04:32 -04:00
Owner
Copy Link

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

Use `===`. Only use `==` for null checks.
a3thakra marked this conversation as resolved
a258wang added 1 commit 2021-08-23 12:12:35 -04:00
continuous-integration/drone/push Build is passing Details
fc3aec8667 Fix small nitpicks
a258wang added 6 commits 2021-08-23 12:15:35 -04:00
a258wang added 1 commit 2021-08-23 14:47:46 -04:00
continuous-integration/drone/push Build is passing Details
bebfe00c5c Change numberOfEvents to moreEvents
a258wang added 1 commit 2021-08-23 14:52:30 -04:00
continuous-integration/drone/push Build is passing Details
7f8c88074d Merge branch 'main' into feat/upcoming-events-recent-news
a3thakra approved these changes 2021-08-23 14:53:15 -04:00
a3thakra left a comment
Owner
Copy Link

🚢

🚢
a258wang merged commit 490ec7660b into main 2021-08-23 14:56:12 -04:00
a3thakra deleted branch feat/upcoming-events-recent-news 2021-08-23 14:57:14 -04:00
Sign in to join this conversation.
Reviewers
No reviewers
a3thakra
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
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#162
No description provided.
Delete Branch "feat/upcoming-events-recent-news"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?